[Top][All Lists]
[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/
- Sketch for in-notes. (issue 5293053), mtsolo, 2011/10/20
- Re: Sketch for in-notes. (issue 5293053), mtsolo, 2011/10/20
- Re: Sketch for in-notes. (issue 5293053), mtsolo, 2011/10/20
- Re: Sketch for in-notes. (issue 5293053),
bordage . bertrand <=
- Re: Sketch for in-notes. (issue 5293053), mtsolo, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), bordage . bertrand, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), bordage . bertrand, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), percival . music . ca, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), n . puttock, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), dak, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), bordage . bertrand, 2011/10/21
- Re: Sketch for in-notes. (issue 5293053), dak, 2011/10/21