[Top][All Lists]
[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
- fret diagram comments,
Han-Wen Nienhuys <=
RE: fret diagram comments, Carl D. Sorensen, 2008/06/24