Re: Fix #888: Add ly:stencil-scale. (issue2275042)

[pnorcks]

Hi Neil,

Looks great!

I just have a couple of comments for you (below).

Thanks,
Patrick


http://codereview.appspot.com/2275042/diff/2001/input/regression/stencil-scale.ly
File input/regression/stencil-scale.ly (right):

http://codereview.appspot.com/2275042/diff/2001/input/regression/stencil-scale.ly#newcode21
input/regression/stencil-scale.ly:21: (ly:stencil-scale
(ly:text-interface::print grob) 2 1.6))
There is trailing whitespace on this line.

http://codereview.appspot.com/2275042/diff/2001/lily/stencil-interpret.cc
File lily/stencil-interpret.cc (right):

http://codereview.appspot.com/2275042/diff/2001/lily/stencil-interpret.cc#newcode104
lily/stencil-interpret.cc:104: y_scale));
"resetscale" doesn't really need any formal parameters, since they are
unused in the backends.

Interestingly, "resetrotation" also has (unused) formal parameters, but
"resetcolor" does not.  :)

http://codereview.appspot.com/2275042/diff/2001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

http://codereview.appspot.com/2275042/diff/2001/scm/define-markup-commands.scm#newcode3375
scm/define-markup-commands.scm:3375: (number? number? markup?)
On 2010/10/04 10:36:47, Valentin Villenave wrote:
> Hi Neil, your patch looks awesome! Minor comment:
> Wouldn't a number-pair be more consistent with existing markup
commands such as
> translate-scaled?

I have a slight preference for a number-pair instead of two separate
numbers.

http://codereview.appspot.com/2275042/