add \shape

add \shape
add \shape as a method of altering bezier curves by offsetting their control-points.

[david.nalesnik, 2012-05-28 18:41:23]

Add \shape as a method of modifying the control points of bezier curves.

[janek, 2012-05-28 20:23:37]

generally LGTM.

What about coloring the \shaped curves?  This would make checking regtests more
obvious to an ordinary person who may be not familiar with this stuff.

thanks, David!

http://codereview.appspot.com/6255056/diff/1/input/regression/shape-other-cur...
File input/regression/shape-other-curves.ly (right):

http://codereview.appspot.com/6255056/diff/1/input/regression/shape-other-cur...
input/regression/shape-other-curves.ly:19: \bar "||"
Instead of adding double barlines, i would separate different types of curves by
placing them in separate systems.

[david.nalesnik, 2012-05-29 00:02:20]

On 2012/05/28 20:23:37, janek wrote:
> generally LGTM.
> 
> What about coloring the \shaped curves?  This would make checking regtests
more
> obvious to an ordinary person who may be not familiar with this stuff.

Good idea!  I did this for both regression tests.

>
http://codereview.appspot.com/6255056/diff/1/input/regression/shape-other-cur...
> File input/regression/shape-other-curves.ly (right):
> 
>
http://codereview.appspot.com/6255056/diff/1/input/regression/shape-other-cur...
> input/regression/shape-other-curves.ly:19: \bar "||"
> Instead of adding double barlines, i would separate different types of curves
by
> placing them in separate systems.

Done.

[janek, 2012-05-30 05:35:34]

it's good!

[Graham Percival, 2012-05-30 16:57:31]

LGTM, and I like the regtests.

[Neil Puttock, 2012-05-31 16:22:53]

http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs.ly
File input/regression/shape-slurs.ly (right):

http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs....
input/regression/shape-slurs.ly:5: @code{\\shape}. The blue slurs are modified
from the default slurs shown first."
two spaces after full-stop/period

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1041: (_i "Offset control-points of @var{grob} by
@var{offsets}. The argument
two spaces follow full stop/period

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1042: is a Scheme alist or list of alists.  Each
element of an alist represents
`offsets' isn't an alist: there's no (key . value) relationship in the list of
number pairs

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1046: (siblings (if (and (ly:grob? orig)
ly:grob-original always returns a grob, so this is unnecessary

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1048: (ly:spanner-broken-into orig) '() ))
remove spurious whitespace

http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
ly/music-functions-init.ly:1051: (reverse (ly:grob-basic-properties grob))))
fix indent (aligns with 'control-points)

[david.nalesnik, 2012-05-31 17:40:40]

Thank you for reviewing this, Neil!

On 2012/05/31 16:22:53, Neil Puttock wrote:
>
http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs.ly
> File input/regression/shape-slurs.ly (right):
> 
>
http://codereview.appspot.com/6255056/diff/5001/input/regression/shape-slurs....
> input/regression/shape-slurs.ly:5: @code{\\shape}. The blue slurs are modified
> from the default slurs shown first."
> two spaces after full-stop/period

Done.

 
> http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly
> File ly/music-functions-init.ly (right):
> 
>
http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:1041: (_i "Offset control-points of @var{grob} by
> @var{offsets}. The argument
> two spaces follow full stop/period

Done.  (This is actually what I do instinctively.  Here I was fighting the
impulse--with inconsistent results!)
 
>
http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:1042: is a Scheme alist or list of alists.  Each
> element of an alist represents
> `offsets' isn't an alist: there's no (key . value) relationship in the list of
> number pairs
 
Done.  This certainly makes more sense.

>
http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:1046: (siblings (if (and (ly:grob? orig)
> ly:grob-original always returns a grob, so this is unnecessary
 
Done.

>
http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:1048: (ly:spanner-broken-into orig) '() ))
> remove spurious whitespace

Done.

>
http://codereview.appspot.com/6255056/diff/5001/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:1051: (reverse (ly:grob-basic-properties grob))))
> fix indent (aligns with 'control-points)

Done.

[Neil Puttock, 2012-05-31 21:02:44]

On 31 May 2012 18:40,  <david.nalesnik@gmail.com> wrote:
> Thank you for reviewing this, Neil!

You're welcome. :)

LGTM.

[janek, 2012-06-03 19:39:35]

Hi David,

please close this Rietveld issue (crossed circle next to its name).

cheers,
Janek

[david.nalesnik, 2012-06-03 20:16:25]

On 2012/06/03 19:39:35, janek wrote:
> Hi David,
> 
> please close this Rietveld issue (crossed circle next to its name).

OK, done.

Thanks!
David