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/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
