Hi Carl, looks good to me! I just have minor questions wrt coding style, but these are mainly because of my own ignorance :)
http://codereview.appspot.com/3100041/diff/2001/input/regression/zero_staff_space.ly File input/regression/zero_staff_space.ly (right): http://codereview.appspot.com/3100041/diff/2001/input/regression/zero_staff_space.ly#newcode12 input/regression/zero_staff_space.ly:12: \override StaffSymbol #'staff-space = #0.0 Just a question: why did you chose to use "0.0" instead of plain "0" here? http://codereview.appspot.com/3100041/diff/2001/lily/staff-symbol-referencer.cc File lily/staff-symbol-referencer.cc (right): http://codereview.appspot.com/3100041/diff/2001/lily/staff-symbol-referencer.cc#newcode87 lily/staff-symbol-referencer.cc:87: Real denom = Staff_symbol::staff_space (st); Is there a reason why you're using "denom" instead of "denominator"? From what I can see Lily's source code doesn't use this abbrev. anywhere else... http://codereview.appspot.com/3100041/diff/2001/lily/staff-symbol-referencer.cc#newcode90 lily/staff-symbol-referencer.cc:90: st->warning (_ ("staff-space is 0, setting staff-position to 0")); I may be completely deluded here, but wouldn't it be cleaner to put property names as %d variables in the warning string? This could make it easier (for localization, etc.) if we were to change the property names in the future. http://codereview.appspot.com/3100041/ _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
