lilypond-devel
[Top][All Lists]
Advanced

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

Re: Adds dimension stencil command to correct with-dimension (issue 1295


From: dak
Subject: Re: Adds dimension stencil command to correct with-dimension (issue 12957047)
Date: Sat, 24 Aug 2013 16:19:27 +0000


https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc
File lily/paper-system.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/paper-system.cc#newcode55
lily/paper-system.cc:55: if (head == ly_symbol2scm ("skyline-stencil"))
It seems awkward and error-prone to go through a number of stencil types
here just for fishing out footnotes.  Of course, this is just another
layer of ugliness on top of existing ugliness, but it would seem that we
actually want a separate _backend_ for fishing out footnotes, so that
all the default interpretations of stencils are done correctly.

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/12957047/diff/46001/lily/stencil-integral.cc#newcode915
lily/stencil-integral.cc:915: SCM skyline_stencil = scm_cadr (expr);
Why would this traverse the skyline_stencil rather than the real one?

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands.scm#newcode730
scm/define-markup-commands.scm:730: (make-box-skyline-stencil
Stupid question: _iff_ you store a _whole_ replacement skyline anyway,
shouldn't pad-around just shift the original left/right/up/down skylines
left/right/up/down by the given amount (don't ask me what to do about
the corners, though)?

Of course this would be incompatible with previous behavior, but more
likely matching the expectations?

I am not sure that replacement skylines are the right thing anyway, but
it seems sort of pointless _if_ you propose they are to not use them
here.

https://codereview.appspot.com/12957047/diff/46001/scm/define-markup-commands.scm#newcode1997
scm/define-markup-commands.scm:1997: (make-box-skyline-stencil m x-wide
y-wide)))
See pad-markup (which presumably does the same).  Should one of them use
boxes and one of them skylines?

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm
File scm/stencil.scm (right):

https://codereview.appspot.com/12957047/diff/46001/scm/stencil.scm#newcode689
scm/stencil.scm:689: (let ((sur (make-filled-box-stencil x y)))
A filled-box stencil seems like serious overkill here when all you want
is setting the dimensions.  A stencil operator for just setting
dimensions seems less awkward.  There are two possibilities: have it
produce an empty stencil expression but with dimensions, and then use
make-skyline-stencil on it.  Or have it contain an inner expression
anyway.

In that case, make-box-skyline-stencil (what an awkward name) does not
need to call either make-filled-box-stencil nor contain skyline-stencil
anyway.

It means that one needs one more stencil primitive _if_ one wants to
support make-skyline-stencil.  But it avoids juggling with an obscure
combination of stencil operations if one doesn't.

https://codereview.appspot.com/12957047/



reply via email to

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