Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
Reviewers: david.nalesnik, janek,
Message:
On 2012/10/03 05:04:11, janek wrote:
i've skimmed over the discussion in
http://code.google.com/p/lilypond/issues/detail?id=2858 and i'm
confused.
Do we want to change
\shape GrobName #'offsets
into
\shape #'offsets GrobName
or
\shape #'offsets
(used as a tweak, with GrobName being guessed)?
Yes. It is quite valuable to be able to use this function as a tweak
(for example, if you want to meddle with tied chords which LilyPond is
not spectacularly good at), and multiple tweaks to the same music stack
reasonably well syntactically only if the tweaked music is at the end.
So the somewhat more natural argument order when used as an override is
sacrificed in order to make \shape more versatile.
When used as a tweak, the grob name is not guessed but only grobs
created directly from the tweaked music event are affected, whether or
not they share the same grob name. That is the usual behavior of
\tweak.
Description:
Allow \shape to tweak music, swap its arguments
Also does
Run scripts/auxiliar/update-with-convert-ly.sh
Add string-or-music? predicate
Please review this at http://codereview.appspot.com/6585052/
Affected files:
M input/regression/shape-other-curves.ly
M input/regression/shape-slurs.ly
M ly/music-functions-init.ly
M python/convertrules.py
M scm/c++.scm
M scm/lily.scm
Index: input/regression/shape-other-curves.ly
diff --git a/input/regression/shape-other-curves.ly
b/input/regression/shape-other-curves.ly
index
6e8bda777443df88ea62d2e680ecb4b49dc77c0a..d3cce657fa0101338ab6b61e450759915b164290
100644
--- a/input/regression/shape-other-curves.ly
+++ b/input/regression/shape-other-curves.ly
@@ -1,4 +1,4 @@
-\version 2.16.0
+\version 2.17.4
\header {
texidoc = In addition to @code{Slur}, the music function @code{\\shape}
works
@@ -16,7 +16,7 @@ function.
% PhrasingSlur
d4\( d' b g g,8 f' e d c2\)
\override PhrasingSlur #'color = #blue
- \shape PhrasingSlur #'((0 . -2) (-1 . 3.5) (0.5 . 0.5) (0 . -2.5))
+ \shape #'((0 . -2) (-1 . 3.5) (0.5 . 0.5) (0 . -2.5)) PhrasingSlur
d4\( d' b g g,8 f' e d c2\)
\break
@@ -25,7 +25,7 @@ function.
\break
cis
\override Tie #'color = #blue
- \shape Tie #'(() ((0 . -0.9) (0 . -0.5) (0 . -0.5) (0 . -0.9)))
+ \shape #'(() ((0 . -0.9) (0 . -0.5) (0 . -0.5) (0 . -0.9))) Tie
cis~
\break
cis
@@ -34,13 +34,13 @@ function.
% LaissezVibrerTie
c\laissezVibrer
\override LaissezVibrerTie #'color = #blue
- \shape LaissezVibrerTie #'((0 . 0) (0.5 . 0.2) (1.5 . 0.2) (2 . 0))
+ \shape #'((0 . 0) (0.5 . 0.2) (1.5 . 0.2) (2 . 0)) LaissezVibrerTie
c\laissezVibrer
\break
% RepeatTie
c\repeatTie
\override RepeatTie #'color = #blue
- \shape RepeatTie #'((-1 . 0) (-0.7 . 0) (-0.3 . 0) (0 . 0))
+ \shape #'((-1 . 0) (-0.7 . 0) (-0.3 . 0) (0 . 0)) RepeatTie
c\repeatTie
}
Index: input/regression/shape-slurs.ly
diff --git a/input/regression/shape-slurs.ly
b/input/regression/shape-slurs.ly
index
8b4ad53a6d683008c0dc0769a3c644a12fde3aaf..729ddad2aedf4e5b3f7b27e7e5721fa0e98f55bb
100644
--- a/input/regression/shape-slurs.ly
+++ b/input/regression/shape-slurs.ly
@@ -1,4 +1,4 @@
-\version 2.16.0
+\version 2.17.4
\header {
texidoc = The control points of a broken or unbroken slur may be offset
by
@@ -22,12 +22,12 @@
% modified
\relative c'' {
\override Slur #'color = #blue
- \shape Slur #'((0 . -2) (-1 . 3.5) (0.5 . 0.5) (0 . -2.5))
+ \shape #'((0 . -2) (-1 . 3.5) (0.5 . 0.5) (0 . -2.5)) Slur
d4( d' b g g,8 f' e d c2)
- \shape Slur #'(
+ \shape #'(
((0 . -2.5) (0 . 1.5) (0 . 1) (0 . -0.5))
((1 . 2.5) (0 . 1.5) (0 . 1) (0 . 0))
- )
+ ) Slur
d4( d' b g
\break
g,8 f' e d c2)
Index: ly/music-functions-init.ly
diff --git a/ly/music-functions-init.ly b/ly/music-functions-init.ly
index
26db17c5ede8ce07e9b72f78fa7d05c0f82a7cbf..ab24bdf4f24d5b3d7a891eecb192ec98cd228ea3
100644
--- a/ly/music-functions-init.ly
+++ b/ly/music-functions-init.ly
@@ -1095,12 +1095,16 @@ a context modification duplicating their effect.)
mods))
shape =
-#(define-music-function (parser location grob offsets)
- (string? list?)
- (_i Offset control-points of @var{grob} by @var{offsets}. The argument
-is a list of number pairs or list of such lists. Each element of a pair
-represents an offset to one of the coordinates of a control-point.)
- (define ((shape-curve offsets) grob)
+#(define-music-function (parser location offsets item)
+ (list? string-or-music?)
+ (_i Offset control-points of @var{item} by @var{offsets}. The
+argument is a list of number pairs or list of such lists. Each
+element of a pair represents an offset to one of the coordinates of a
+control-point. If @var{item} is a string, the result is
+@code{\\once\\override} for the specified grob type. If @var{item} is
+a music expression, the result is the same music expression with an
+appropriate tweak applied.)
+ (define (shape-curve grob)
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On 2012/10/02 23:39:05, david.nalesnik wrote: LGTM [...] I wonder if it would be helpful to alter one or two of the following applications of the function as override to the tweak form. [...] What about adding a demonstration here of the ability to modify curves beginning at the same timestep? So possibly insert here something like the following (paired with the alteration I've added below): To those suggestions let me answer with the famous answer to the question Mr Gandhi, what do you think of Western civilization?, namely with I would consider it a good idea. Had Gandhi been a programmer, his answer might have been Patches welcome. The long and the short answer is that I would not know where to start. I have not actually considered the documentation, and I don't even know whether doing so would not interfere with issue 2858 URL:http://code.google.com/p/lilypond/issues/detail?id=2858 which is currently on hold due to this pending issue. So the best I think I can do at the moment is to crosslink your suggestion to issue 2858 (which is about documenting \shape) and see how and when and where Trevor thinks it should be incorporated. http://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On 2012/10/03 13:57:25, dak wrote: [...] To those suggestions let me answer with the famous answer to the question Mr Gandhi, what do you think of Western civilization?, namely with I would consider it a good idea. Oh, for just one zinger like that... So the best I think I can do at the moment is to crosslink your suggestion to issue 2858 (which is about documenting \shape) and see how and when and where Trevor thinks it should be incorporated. OK, thanks for doing this. Here I was simply thinking that the reg tests should cover as many situations as possible (within a modest length), but I suppose that the changes I proposed aren't testing anything that isn't covered elsewhere (Tweaks work!) https://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On 2012/10/03 15:08:30, david.nalesnik wrote: On 2012/10/03 13:57:25, dak wrote: [...] To those suggestions let me answer with the famous answer to the question Mr Gandhi, what do you think of Western civilization?, namely with I would consider it a good idea. Oh, for just one zinger like that... So the best I think I can do at the moment is to crosslink your suggestion to issue 2858 (which is about documenting \shape) and see how and when and where Trevor thinks it should be incorporated. OK, thanks for doing this. Here I was simply thinking that the reg tests should cover as many situations as possible (within a modest length), but I suppose that the changes I proposed aren't testing anything that isn't covered elsewhere (Tweaks work!) David, I honestly have not really looked into this at all and did not even realize you were talking about regtests rather than documentation (I have not touched either myself but rather let the convert-ly rule deal with swapping argument order). So if you consider regression test additions orthogonal to Trevor's documentation work, you are certainly more acquainted with the code, and I am currently immersed in parser work to make the Context.GrobName thing fly. So any actual git-format-patch proposals to be folded into this issue or as a separate add-on issue would be more than welcome. And it is never wrong to have the regtests for one feature test all aspects of that feature, and not rely on something else to prove that. https://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On 2012/10/03 15:17:26, dak wrote: [...] and I am currently immersed in parser work to make the Context.GrobName thing fly. Thank you very much for working on this! I am looking forward to being able to fix the mess at the beginning of \alterBroken (and a general offsetting function I hope to put up for comments at some point). So any actual git-format-patch proposals to be folded into this issue or as a separate add-on issue would be more than welcome. And it is never wrong to have the regtests for one feature test all aspects of that feature, and not rely on something else to prove that. OK, I think I'll wait till this patch and the Trevor's documentation patch is through, and then I'll create a new issue for the regtests. I may simply leave these two as they are, and add a short test along the lines of, \shape works independently on curves beginning at the same timestep. https://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On Wed, Oct 3, 2012 at 3:46 PM, d...@gnu.org wrote: On 2012/10/03 05:04:11, janek wrote: i've skimmed over the discussion in http://code.google.com/p/lilypond/issues/detail?id=2858 and i'm confused. Do we want to change \shape GrobName #'offsets into \shape #'offsets GrobName or \shape #'offsets (used as a tweak, with GrobName being guessed)? Yes. uh, i ususally find that it's a bad idea to answer yes to a question that goes like do we want X or Y? :) It is quite valuable to be able to use this function as a tweak ...but from this sentence i deduce that we're trying to do the second thing, i.e. \shape will have one #'offsets argument and no argument specifying grob name, which will be deduced from the music that follows the command. Right? In that case, LGTM. cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
david.nales...@gmail.com OK, I think I'll wait till this patch and the Trevor's documentation patch is through, and then I'll create a new issue for the regtests. I may simply leave these two as they are, and add a short test along the lines of, \shape works independently on curves beginning at the same timestep. Sounds good! Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On 2012/10/03 16:38:45, janek wrote: On Wed, Oct 3, 2012 at 3:46 PM, mailto:d...@gnu.org wrote: On 2012/10/03 05:04:11, janek wrote: i've skimmed over the discussion in http://code.google.com/p/lilypond/issues/detail?id=2858 and i'm confused. Do we want to change \shape GrobName #'offsets into \shape #'offsets GrobName or \shape #'offsets (used as a tweak, with GrobName being guessed)? Yes. uh, i ususally find that it's a bad idea to answer yes to a question that goes like do we want X or Y? :) The question do we want to change X into Y or Z, and the answer to that was yes because you can now _either_ write Y or Z, and _either_ will work. \shape will figure out from the _type_ of the last argument whether it is supposed to override or tweak. And since tweaks stack awfully when the tweaked argument is not last, we put the tweaked argument last in _either_ case. Even if the tweaked argument happens to be a grob name (resulting in an override) instead of some music (resulting in a tweak). It is quite valuable to be able to use this function as a tweak ...but from this sentence i deduce that we're trying to do the second thing, i.e. \shape will have one #'offsets argument and no argument specifying grob name, which will be deduced from the music that follows the command. Right? Either will work. That's the nice thing. In that case, LGTM. And in this case? https://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
On Wed, Oct 3, 2012 at 7:36 PM, d...@gnu.org wrote: ...but from this sentence i deduce that we're trying to do the second thing, i.e. \shape will have one #'offsets argument and no argument specifying grob name, which will be deduced from the music that follows the command. Right? Either will work. That's the nice thing. Wow! I'm definitely underestimating Lily possibilities here! Looks like another improvement of yours that i overlooked ;) In that case, LGTM. And in this case? in this case i can only say __ ___ __ | / _ | |\ /| |_, \_| | | \/ | Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Allow \shape to tweak music, swap its arguments (issue 6585052)
LGTM https://codereview.appspot.com/6585052/diff/1/input/regression/shape-other-curves.ly File input/regression/shape-other-curves.ly (right): https://codereview.appspot.com/6585052/diff/1/input/regression/shape-other-curves.ly#newcode18 input/regression/shape-other-curves.ly:18: \override PhrasingSlur #'color = #blue I wonder if it would be helpful to alter one or two of the following applications of the function as override to the tweak form. https://codereview.appspot.com/6585052/diff/1/input/regression/shape-slurs.ly File input/regression/shape-slurs.ly (right): https://codereview.appspot.com/6585052/diff/1/input/regression/shape-slurs.ly#newcode19 input/regression/shape-slurs.ly:19: g,8 f' e d c2) What about adding a demonstration here of the ability to modify curves beginning at the same timestep? So possibly insert here something like the following (paired with the alteration I've added below): \break \acciaccatura b8^( cis4 cis' fis, gis) https://codereview.appspot.com/6585052/diff/1/input/regression/shape-slurs.ly#newcode33 input/regression/shape-slurs.ly:33: g,8 f' e d c2) \break \shape #'((0 . 0) (-0.1 . -0.25) (0.1 . -0.25) (0 . -0.25)) Slur \acciaccatura b8 -\shape #'((-0.5 . 0.5) (0 . 0.5) (0 . 0) (0 . 0)) ^( cis4 cis' fis, gis) https://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow \shape to tweak music, swap its arguments (issue 6585052)
i've skimmed over the discussion in http://code.google.com/p/lilypond/issues/detail?id=2858 and i'm confused. Do we want to change \shape GrobName #'offsets into \shape #'offsets GrobName or \shape #'offsets (used as a tweak, with GrobName being guessed)? Janek http://codereview.appspot.com/6585052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
