Updates to fret-diagrams

[joeneeman]

Reviewers: Carl.D.Sorensen,


http://codereview.appspot.com/11857/diff/1/2
File input/regression/fret-diagrams.ly (right):

http://codereview.appspot.com/11857/diff/1/2#newcode1
Line 1: \version "2.12.0"
This regtest is getting quite large. Is there a logical way to split it
up (eg. fret-diagrams-landscape, fret-diagrams-string-count, etc)?

http://codereview.appspot.com/11857/diff/1/4
File scm/fret-diagrams.scm (right):

http://codereview.appspot.com/11857/diff/1/4#newcode1
Line 1: ;;;; fret-diagrams.scm --
I don't know enough about fret diagrams to really understand what's
going on, but I have one fairly general comment: the code is sprinkled
with
(cond ((eq? orientation 'landscape)
       foo)
      ((eq? orientation 'opposing-landscape)
       bar)
      (else
       baz))
where foo, bar and baz are almost the same except that they have
differences in signs and the X,Y are swapped around. It would be great
if this cond could be isolated to a few functions. For example:

(define (real-coordinate string-coordinate fret-coordinate orientation)
  (cond
   ((eq? orientation 'landscape)
    (cons fret-coordinate string-coordinate))
   ((eq? orientation 'opposing-landscape)
    (cons (-fret-coordinate string-coordinate))
   (else
    (cons string-coordinate fret-coordinate))))

and then make-straight-barre-stencil (for example) becomes

(define (make-straight-barre-stencil
         size half-thickness fret-coordinate
         start-string-coordinate end-string-coordinate
         orientation)
  (let ((one-point (real-coordinate start-string-coordinate
fret-coordinate orientation))
        (other-point (real-coordinate end-string-coordinate
fret-coordinate orientation)))
   (ly:make-line-stencil
     half-thickness
     (car one-point) (cdr one-point)
     (car other-point) (cdr other-point)))

If you're in a hurry to get the functionality in, don't let this hold
you up.

http://codereview.appspot.com/11857/diff/1/4#newcode91
Line 91: (cond
Here again you have cond leading to duplicate code. If you had
(define (combine-stencils a b c d) (ly:stencil-combine-at-edge a (car b)
(cdr b) c d))
then you could move the cond inside:

(combine-stencils string-stencil
  (cond
    ((eq? orientation 'landscape) (cons Y UP))
    ((eq? orientation 'opposing-landscape) (cons Y DOWN))
    (else (cons X RIGHT)))
  (draw-strings (- string-count 1) fret-range th size orientation)
  gap))

Description:
Updates to fret-diagrams
Add new orientation opposing-landscape as requested by user
Revise orientation code so 'normal is always a default (in
    the else clause of a cond)
Adjust the origin of the fret diagram so that the top fret
  on the diagram is always at the origin
Clean up calls for fret-diagram-details so that merge-details is always
  used.
Clean up text stencil alignments
Fix bug in thick top fret
Revise input/regression/fret-diagrams.ly so that it tests all
fret-diagram code functionality.

Please review this at http://codereview.appspot.com/11857

Affected files:
  M input/regression/fret-diagrams.ly
  M scm/define-grob-properties.scm
  M scm/fret-diagrams.scm