On Nov 28, 2011, at 2:24 AM, carl.d.soren...@gmail.com wrote: > Looks pretty good. Thanks for hitting this so quickly. Just a couple > of stylistic comments. > > Thanks, > > Carl > > > > http://codereview.appspot.com/5438060/diff/1/lily/include/system.hh > File lily/include/system.hh (right): > > http://codereview.appspot.com/5438060/diff/1/lily/include/system.hh#newcode45 > lily/include/system.hh:45: Grob * get_neighboring_staff (Direction dir, > Grob *vag); > *vag should be *vertical_axis_group for clarity in the header file, IMO. > > In the engraver, the code that defines vag shows it to be a > vertical_axis_group, so I'm ok with it there. > > http://codereview.appspot.com/5438060/diff/1/lily/system.cc > File lily/system.cc (right): > > http://codereview.appspot.com/5438060/diff/1/lily/system.cc#newcode759 > lily/system.cc:759: System::get_neighboring_staff (Direction dir, Grob > *vag) > Here, *vag should be *vertical_axis_group because there is no context > for understanding what it means. > > http://codereview.appspot.com/5438060/diff/1/scm/define-grob-properties.scm > File scm/define-grob-properties.scm (right): > > http://codereview.appspot.com/5438060/diff/1/scm/define-grob-properties.scm#newcode1047 > scm/define-grob-properties.scm:1047: (has-span-bar ,pair? "A pair of > span bars indicating whether a a span bar > "A pair of grobs containing the span bars to be drawn above and below > the staff. If no span bar is in a position, the respective element is > set to @code{#f}." > > Is this a correct statement? If so, I think it's clearer than the > current wording. > > http://codereview.appspot.com/5438060/
I've incorporated all these suggestions into a new patch & posted it. Thank you! Cheers, MS _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
