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

[n . puttock]

On 2010/10/08 05:32:32, Patrick McCarty wrote:

> Looks great!

Thanks.


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.

Oops, standards are slipping. ;)


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.  :)

Ah, I just obediently followed the example of `resetrotation', while
thinking it slightly silly to send unused params.


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.

OK, I'll sort out a revised patch with all the suggested changes.

Cheers,
Neil



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