lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

fret diagram comments


From: Han-Wen Nienhuys
Subject: fret diagram comments
Date: Sun, 22 Jun 2008 00:16:42 +0200

hi Carl,

here some quick notes on the fret diagram code.  I am not familiar
with  frets, so I have little comment on the code itself, but it seems
a little sloppy with lots of random formatting changes all over the
place.

- for the properties inside the fret0-diagram-details, would it make
sense to drop the fret- prefix?

          (xo-list (cdr (assoc 'xo-list parameters)))

what if xo-list is not in the list? Use something that has a default,
so it won't crash on (cdr '())

 ;            (barre-vertical-offset (chain-assoc-get
'barre-vertical-offset props 0.5))

make sure there is no commented out code left after you finish. If you
want to preserve a snippet of code, add a explaining comment.  I think
there is a finger-code comented out somewhere.

+(define (merge-markup-override x alist-list . default)
+  "Return ALIST-LIST entries for  key X, in one combined alist.

rename x to key


+  "Return ALIST-LIST entries for  key X, in one combined alist.
+  There can be two ALIST-LIST entries for a given key.  The first
+  comes from the override-markup function, the second comes
+  from property settings during a regular override.
+  Return DEFAULT (optional, else #f) if not
+found."
+

maybe an example would clarify

+(define (assoc-default key alist default)

I think we have a ly:assoc-get that does this already

+(define (merge-markup-override x alist-list . default)

the code has nothing to do with markup. Rename?

    fix documentation files for new fret-diagram-details

    change Fretboard documentation to replace finger-code
    with fret-diagram-details.

this commit reindents all of define-grobs.scm.  Is that necessary?
Maybe this is a tab/space issue, but please make sure you don't touch
code without reason?

I see you fixed this in further commits. Maybe you can use cherry
picking to construct a list of commits which does not touch the file
unnecessarily?

     (if (= fret-count 1)
-       fret-line
+    fret-line

you seem to have indent issues with your editor.  Which one are you
using?  I also saw some lines where you were adding/removing trailing
whitespace.  Did I leave trailing whitespace, or did you add it? (we
shouldn't have any)


+(define (draw-frets  fret-range string-count th size orientation)

2 spaces after func name

      ((equal?   number-type  'roman-lower) (fancy-format #f "~(~:@r~)" ba

double spaces

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen




reply via email to

[Prev in Thread] Current Thread [Next in Thread]