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: mtsolo
Subject: Re: Sketch for in-notes. (issue 5293053)
Date: Fri, 21 Oct 2011 12:21:57 +0000

I'm not sure how to deal with some of the "d├ęBordage" of line lengths -
you're right that they're ugly, but I dont' see how to chop them down &
stay within good code style.

Thanks for the review!
~Mike


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#newcode79
lily/page-layout-problem.cc:79: footnotes_added = g->get_property
("footnote-stencil") != SCM_EOL;
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
footnotes_added = scm_is_pair (g->get_property ("footnote-stencil"));

Would this work?  footnote-stencil's value is a stencil (not a list), so
would it come up true in scm_is_pair ?

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;
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
Shouldn't it be has_in_notes_ (with an underscore at the end)?
has_footnotes_ should be defined here too.

Nein, as this is internal to the function (because in_notes only matter
on a line to line basis) whereas has_footnotes_ is valid for the whole
page.

http://codereview.appspot.com/5293053/diff/12001/lily/page-spacing.cc#newcode79
lily/page-spacing.cc:79: in_note_height += (has_in_notes
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
What is the default value of an unset boolean?  I mean: the first
time, is it
adding 0.0 or breaker_->in_note_padding()?

unset_boolean = stupid_programmer

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)
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
Ugh...  A boolean to tell if we're in a footnote or in an "in-note"...

Why the ugh?

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")))
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
Line length.  Plus, I don't really understand what's the use of this.

It keeps either footnotes or in-notes depending on what we want.

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?")
On 2011/10/21 11:52:25, Bertrand Bordage wrote:
Again, this is really ugly.

Perhaps a 'style property instead?  For now, I have it as a boolean
because it could only be one of two things.

http://codereview.appspot.com/5293053/

reply via email to

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