Carl, Thanks for the comments! I've integrated all but one of them - when you say "Also add to TabStaff," what do you mean? I was under the impression that TabStaff would inherit this engraver because it inherits from Staff.
Cheers, MS On Jan 8, 2011, at 1:38 PM, [email protected] wrote: > Reviewers: MikeSol, > > Message: > Looks great, Mike! > > You have some code indentation issues that don't match our style. Other > than that, I think it's good to go. > > Of course, we do need a regression test as well. > > Thanks, > > Carl > > > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc > File lily/beam-collision-engraver.cc (right): > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc#newcode4 > lily/beam-collision-engraver.cc:4: Copyright (C) 1997--2010 Han-Wen > Nienhuys <[email protected]> > Copyright should be 2011 Mike Solomon. > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc#newcode48 > lily/beam-collision-engraver.cc:48: { > brace not needed > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc#newcode55 > lily/beam-collision-engraver.cc:55: { > remove { > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc#newcode57 > lily/beam-collision-engraver.cc:57: { > remove { > > http://codereview.appspot.com/3928041/diff/1/lily/beam-collision-engraver.cc#newcode83 > lily/beam-collision-engraver.cc:83: { > remove { > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc > File lily/beam.cc (right): > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode944 > lily/beam.cc:944: { > indent { 2 spaces > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode962 > lily/beam.cc:962: { > indent 2 spaces > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode967 > lily/beam.cc:967: magic = -2.0; > { on separate line, indented 2 spaces > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode972 > lily/beam.cc:972: { > indent > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode992 > lily/beam.cc:992: { > indentation > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode997 > lily/beam.cc:997: } else { > indentation > > http://codereview.appspot.com/3928041/diff/1/lily/beam.cc#newcode1008 > lily/beam.cc:1008: } else > else on its own line, same indentation as if > > http://codereview.appspot.com/3928041/diff/1/ly/engraver-init.ly > File ly/engraver-init.ly (right): > > http://codereview.appspot.com/3928041/diff/1/ly/engraver-init.ly#newcode68 > ly/engraver-init.ly:68: \consists "Beam_collision_engraver" > Also add to TabStaff > > Description: > Fixing issue 37 with extra position callback > Adds beam collision engraver > > Goodbye whitespace > > Please review this at http://codereview.appspot.com/3928041/ > > Affected files: > A lily/beam-collision-engraver.cc > M lily/beam.cc > M lily/include/beam.hh > M ly/engraver-init.ly > M scm/define-grobs.scm > > > > _______________________________________________ > lilypond-devel mailing list > [email protected] > http://lists.gnu.org/mailman/listinfo/lilypond-devel _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
