lilypond-devel
[Top][All Lists]
Advanced

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

Re: Issue 3330: redo much of the stencil stacking/spacing/empty-check (i


From: janek . lilypond
Subject: Re: Issue 3330: redo much of the stencil stacking/spacing/empty-check (issue 8869044)
Date: Sun, 28 Apr 2013 21:59:37 +0000

changes i see generally LGTM (but i may be missing something, in
particular Keith seems to make a good point).

Let Stencil::is_empty use Box::is_empty [...]

I think the desription of this commit could use some more detail, for
example that we need (1 . -1) stencils for backspacing.

Let Stencil::add_at_edge skip offsets
for half-empty stencils [...]

I think "half-empty" is confusing (i at least was confused).  I think
"empty along only one axis" would be clearer.

Let Stencil::align_to check axis-specific
emptiness before punting

could you use a more odrinary word?  I had to check 'punt'
in a dictionary, and i consider my English to be fairly good.


https://codereview.appspot.com/8869044/diff/9001/lily/box.cc
File lily/box.cc (right):

https://codereview.appspot.com/8869044/diff/9001/lily/box.cc#newcode66
lily/box.cc:66: return interval_a_[a][LEFT] == empty[LEFT]
Do we need to define 'empty'?  I.e. why not just write
interval_a_[a][LEFT] == infinity_f ?
Are we expecting that the definition of empty extent will change
further?

https://codereview.appspot.com/8869044/diff/9001/lily/stencil.cc
File lily/stencil.cc (right):

https://codereview.appspot.com/8869044/diff/9001/lily/stencil.cc#newcode253
lily/stencil.cc:253:
whitespace error

https://codereview.appspot.com/8869044/diff/9001/lily/stencil.cc#newcode259
lily/stencil.cc:259: offset += d * padding;
I think this is worth a comment with an example.  It's not obvious why
sometimes we want padding and sometimes not.

https://codereview.appspot.com/8869044/diff/9001/scm/markup.scm
File scm/markup.scm (right):

https://codereview.appspot.com/8869044/diff/9001/scm/markup.scm#newcode67
scm/markup.scm:67: between the end of each stencil and the reference
point of the following
Hmm, shouldn't this read "between the end of the stencil and the
*beginning* of the following stencil"?

https://codereview.appspot.com/8869044/



reply via email to

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