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