lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Sketch for in-notes. (issue 5293053)


From: bordage . bertrand
Subject: Re: Sketch for in-notes. (issue 5293053)
Date: Fri, 21 Oct 2011 11:52:25 +0000

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/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]