Looks pretty good. I have a few comments about things that seem to be unnecessarily specific to harmonics.
Thanks, Carl http://codereview.appspot.com/1669041/diff/26001/27003 File scm/define-grob-interfaces.scm (right): http://codereview.appspot.com/1669041/diff/26001/27003#newcode91 scm/define-grob-interfaces.scm:91: 'harmonic-parentheses-interface Calling this "parenthesis-interface" would allow its use for other applications of parentheses and would be a good idea, in my opinion. http://codereview.appspot.com/1669041/diff/26001/27004 File scm/define-grob-properties.scm (right): http://codereview.appspot.com/1669041/diff/26001/27004#newcode56 scm/define-grob-properties.scm:56: (angularity ,number? "Angularity of a bracket.") "angle bracket" instead of "bracket"? http://codereview.appspot.com/1669041/diff/26001/27004#newcode130 scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of the harmonic angle bracket.") Why do we need bracket-width? Why can't we just use the pre-existing width property? http://codereview.appspot.com/1669041/diff/26001/27004#newcode850 scm/define-grob-properties.scm:850: (white-padding ,number? "Amount of padding surrounding a harmonic Why make this specific for harmonics? Why not just "Amount of white space boundary in a whiteout" or something similar? http://codereview.appspot.com/1669041/diff/26001/27007 File scm/tablature.scm (right): http://codereview.appspot.com/1669041/diff/26001/27007#newcode155 scm/tablature.scm:155: (define (draw-harmonic-stencil dir grob) Why not use parenthesize-stencil (see scm/stencil.scm)? This seems like duplicated code, and we should probably avoid that if possible. http://codereview.appspot.com/1669041/show _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
