I can't compile because of valgrind. Bertrand
http://codereview.appspot.com/5293053/diff/12001/lily/include/page-layout-problem.hh File lily/include/page-layout-problem.hh (right): http://codereview.appspot.com/5293053/diff/12001/lily/include/page-layout-problem.hh#newcode100 lily/include/page-layout-problem.hh:100: Direction in_note_direction_; Trailing whitespace. http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc File lily/page-breaking.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc#newcode189 lily/page-breaking.cc:189: old.in_note_heights_.begin (), old.in_note_heights_.end ()); Check the length of these lines of code. http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc#newcode268 lily/page-breaking.cc:268: in_note_padding_ = robust_scm2double (pb->paper_->c_variable ("in-note-padding"), 0.0); Same comment as before. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode74 lily/page-layout-problem.cc:74: if (lines == SCM_EOL) if (!scm_is_pair (lines)) See the new chapter "Scheme->C interface" in CG/Programming work. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode76 lily/page-layout-problem.cc:76: Trailing whitespace. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g->get_property ("footnote-stencil") != SCM_EOL; footnotes_added = scm_is_pair (g->get_property ("footnote-stencil")); http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode81 lily/page-layout-problem.cc:81: footnotes_added = p->get_property ("footnote-stencil") != SCM_EOL; Same as before. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode251 lily/page-layout-problem.cc:251: in_note_mol.add_at_edge (Y_AXIS, DOWN, *footnote_stencil, padding); Check the line length. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode346 lily/page-layout-problem.cc:346: #include <valgrind/valgrind.h> ? http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode413 lily/page-layout-problem.cc:413: in_note_padding_ = robust_scm2double (paper->c_variable ("in-note-padding"), 0.5); Line length. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode416 lily/page-layout-problem.cc:416: in_note_direction_ = UP; You could use robust_scm2dir instead of these three lines. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode525 lily/page-layout-problem.cc:525: Stencil *in_note_stencil = unsmob_stencil (sys->get_property ("in-note-stencil")); Line length. http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode532 lily/page-layout-problem.cc:532: sky->set_minimum_height (sky->max_height () + in_note_direction_ * (in_note_padding_ + in_note_stencil->extent (Y_AXIS).length ())); Line length. http://codereview.appspot.com/5293053/diff/12001/lily/page-spacing.cc File lily/page-spacing.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-spacing.cc#newcode76 lily/page-spacing.cc:76: bool has_in_notes; Shouldn't it be has_in_notes_ (with an underscore at the end)? has_footnotes_ should be defined here too. http://codereview.appspot.com/5293053/diff/12001/lily/page-spacing.cc#newcode79 lily/page-spacing.cc:79: in_note_height += (has_in_notes What is the default value of an unset boolean? I mean: the first time, is it adding 0.0 or breaker_->in_note_padding()? http://codereview.appspot.com/5293053/diff/12001/lily/page-spacing.cc#newcode81 lily/page-spacing.cc:81: : breaker_->in_note_padding ()); Trailing whitespace. http://codereview.appspot.com/5293053/diff/12001/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/system.cc#newcode298 lily/system.cc:298: System::internal_get_note_heights_in_range (vsize start, vsize end, bool foot) Ugh... A boolean to tell if we're in a footnote or in an "in-note"... http://codereview.appspot.com/5293053/diff/12001/lily/system.cc#newcode305 lily/system.cc:305: if (foot ? !to_boolean (footnote_grobs[i]->get_property ("footnote")) : to_boolean (footnote_grobs[i]->get_property ("footnote"))) Line length. Plus, I don't really understand what's the use of this. http://codereview.appspot.com/5293053/diff/12001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/5293053/diff/12001/scm/define-grob-properties.scm#newcode299 scm/define-grob-properties.scm:299: (footnote ,boolean? "Should this be a footnote or in-note?") Again, this is really ugly. http://codereview.appspot.com/5293053/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
