[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fix #888: Add ly:stencil-scale. (issue2275042)
From: |
pnorcks |
Subject: |
Re: Fix #888: Add ly:stencil-scale. (issue2275042) |
Date: |
Fri, 08 Oct 2010 05:32:32 +0000 |
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/
- Fix #888: Add ly:stencil-scale. (issue2275042), v . villenave, 2010/10/04
- Re: Fix #888: Add ly:stencil-scale. (issue2275042),
pnorcks <=
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), n . puttock, 2010/10/24
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), n . puttock, 2010/10/24
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), pnorcks, 2010/10/25
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), lemzwerg, 2010/10/25
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), n . puttock, 2010/10/25
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), markpolesky, 2010/10/26
- Re: Fix #888: Add ly:stencil-scale. (issue2275042), Carl . D . Sorensen, 2010/10/27