\tuplet and \tupletSpan to replace \times and tupletSpannerDuration

\tuplet and \tupletSpan to replace \times and tupletSpannerDuration


Commits in reverse:

Run scripts/auxiliar/update-with-convert-ly.sh


define-music-display-methods.scm: use \tuplet rather than \times for TimeScaledMusic


convertrules.py: substitute \times with \tuplet where appropriate

Also convert set/unset of tupletSpannerDuration to \tupletSpan

\tuplet and \tupletSpan to replace \times and tupletSpannerDuration

\tuplet:
Scale the given @var{music} to tuplets.  @var{ratio} is a
fraction that specifies how many notes are played in place of the
nominal value: it will be @samp{3/2} for triplets, namely three notes
being played in place of two.  If the optional @var{duration} is
specified, it is used instead of @code{tupletSpannerDuration} for
grouping the tuplets.
For example,
@example
\tuplet 3/2 4 @{ c8 c c c c c @}
@end example
will result in two groups of three tuplets, each group lasting for a
quarter note.

\tupletSpan:
Set @code{tupletSpannerDuration}, the length into which
tuplets are being subdivided.  To unset, use a zero duration like
@samp{1*0}.

If TimeScaledMusic carries a duration, let it override tupletSpannerDuration

[dak, 2013-01-11 12:27:54]

No need for extensive protection while music is protected by iterator

[Ian Hulin (gmail), 2013-01-12 12:54:25]

This LGTM, but with one really big question: do we really still need \tupletSpan
at all?

The start point for this work was to make "easy triplets and tuplets", i.e. make
handling of all the triplets and *uplets less daunting and a bit more readable
by humans.

The original \times command had a companion \tupletSpannerDuration command to
allow a different length of spanner bracket from the total length of music
"tupleted" music.  \tupletSpannerDuration was implemented as a property for
\times. 

Here is the NR example for tupletSpannerDuration and \times:
> 
> {
>   \set tupletSpannerDuration = #(ly:make-moment 1 4)
>   \times 2/3 { c8 c c c c c }
> }
> 
I think using this property was working round the restriction we had in the
language at the time in that we couldn't use optional parameters to music
functions.

I really think that unless someone can come up with a really strong case that
\tupletSpannerDuration is needed for backwards-compatibility we don't need to
provide \tupletSpan.  It was my attempt at covering all the bases in the
doc-string for \tupletSpan that convinced me. (See below).

Cheers,
Ian Hulin

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly#n...
ly/music-functions-init.ly:1365: tuplets are being subdivided.  To unset, use a
zero duration like
tuplets specified by the current \tuplet command are to be subdivided.  To
unset, use a zero duration like @samp{1*0}. Doing this will ensure the spanner
for the music on the current \tuplet command spans all the music, rather than
the value set to any prior \tupletSpan call.

[dak, 2013-01-12 14:51:40]

On 2013/01/12 12:54:25, Ian Hulin (gmail) wrote:
> This LGTM, but with one really big question: do we really still need
\tupletSpan
> at all?

It is for setting a default.  If you have a back-and-forth between tuplets and
full notes (which is essentially the case for which there has been substantial
clamor for \triplet as a "shorthand" for \tuplet 3/2), then it might save
typing.  I am not sure that the saved typing is worth having a non-local
side-effect in the form of a context property, but \triplet did not strike me as
all that necessary either.

> The start point for this work was to make "easy triplets and tuplets", i.e.
make
> handling of all the triplets and *uplets less daunting and a bit more readable
> by humans.

> Here is the NR example for tupletSpannerDuration and \times:
> > 
> > {
> >   \set tupletSpannerDuration = #(ly:make-moment 1 4)
> >   \times 2/3 { c8 c c c c c }
> > }
> >

Ugly.

> I think using this property was working round the restriction we had in the
> language at the time in that we couldn't use optional parameters to music
> functions.

At that time one could not use durations as a "native" parameter to music
functions at all.  But that is more or less a red herring: at that time, a _lot_
of functionality was just implemented in the parser instead of using music
functions, and if there had been sufficient pressure for putting an optional
duration with a \times command, that syntax would have ended up in the parser,
anyway.

Come to think of it, \times _was_ already hardwired in the parser because of the
2/3 parsing and was only turned into a music function in version 2.15.17, when I
made LilyPond fractions have a Scheme representation.

> I really think that unless someone can come up with a really strong case that
> \tupletSpannerDuration is needed for backwards-compatibility we don't need to
> provide \tupletSpan.  It was my attempt at covering all the bases in the
> doc-string for \tupletSpan that convinced me. (See below).

Well, it might have been stupid of me to make a user-understandable syntax and
then special-case a geeky value like 1*0 into it.  I can offer two cleaner
syntactic alternatives:

\tupletSpan \default

\tupletSpanOff

Since unsetting the property will let any setting in a parenting context shine
through, \tupletSpan \default is not unintuitive.  However, it is longer to
write.

> https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly
> File ly/music-functions-init.ly (right):
> 
>
https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly#n...
> ly/music-functions-init.ly:1365: tuplets are being subdivided.  To unset, use
a
> zero duration like
> tuplets specified by the current \tuplet command are to be subdivided.  To
> unset, use a zero duration like @samp{1*0}. Doing this will ensure the spanner
> for the music on the current \tuplet command spans all the music, rather than
> the value set to any prior \tupletSpan call.

I think we can agree that we'd rather want a different command or the \default
solution to avoid this technobabble.  That was bad thinking on my part.

Either that, or scrap the property altogether.  But there would not be a good
convert-ly rule for that course, and the feature is quite likely to have seen
use.

Of course, we can also just leave the property in place and provide no
user-level command for setting it.  But I think even when discouraging its use,
a separate command with a readable presentation will lead to more readable
upconverted scores.

[Ian Hulin (gmail), 2013-01-12 15:49:56]

OK, let's go with keeping \tupletSpan with a statement that it's not the
preferred way of doing things, and use \tupletSpan \default as the unset.  I
know \tupletSpanOff may be less typing, but the name kind of implies it accepts
#t/#f values.

I've only commented on the doc-text for the \tupletSpan bit, but otherwise, if
you fix up the \default thing, It'll LGTM.

Cheers,
Ian

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly#n...
ly/music-functions-init.ly:1365: tuplets are being subdivided.  To unset, use a
zero duration like
On 2013/01/12 12:54:25, Ian Hulin (gmail) wrote:
> tuplets specified by the current \tuplet command are to be subdivided.  To
> unset, use a zero duration like @samp{1*0}. Doing this will ensure the spanner
> for the music on the current \tuplet command spans all the music, rather than
> the value set to any prior \tupletSpan call.

tuplets specified by the current \tuplet command are to be subdivided.  To
unset, use @code{\tupletSpan \default}. Doing this will ensure the spanner for
the music on the current \tuplet command spans all the music, rather than
the value set to any prior \tupletSpan call. In general, you are recommended to
use the optional @var{duration} parameter to the @code{\tuplet} command rather
than @code{\tupletSpan}.

[dak, 2013-01-12 16:05:25]

Incorporate results of discussion with Ian

[Trevor Daniels, 2013-01-12 16:16:55]

On 2013/01/12 15:49:56, Ian Hulin (gmail) wrote:
>
> tuplets specified by the current \tuplet command are to be subdivided.  To
> unset, use @code{\tupletSpan \default}. Doing this will ensure the spanner for
> the music on the current \tuplet command spans all the music, rather than
> the value set to any prior \tupletSpan call. In general, you are recommended
to
> use the optional @var{duration} parameter to the @code{\tuplet} command rather
> than @code{\tupletSpan}.

I prefer \tupletSpan \default too, but I don't think we should
advise users not to use \tupletSpan.  That would be justified only 
if we intend to deprecate it, and there is no suggestion of that.
If many tuplets are to be the same length the use of \tupletSpan
is preferable to entering multiple identical optional durations.

Otherwise LGTM.

[dak, 2013-01-12 16:25:22]

Fix embarrassingly half-baked renaming of duration to tuplet-span

[dak, 2013-01-12 16:35:17]

On 2013/01/12 16:16:55, Trevor Daniels wrote:

> I prefer \tupletSpan \default too, but I don't think we should
> advise users not to use \tupletSpan.

The disadvantage of \tupletSpan is that it makes a non-local change.  That's not
copy&paste friendly and leads to non-self-contained code pieces.

> That would be justified only 
> if we intend to deprecate it, and there is no suggestion of that.
> If many tuplets are to be the same length the use of \tupletSpan
> is preferable to entering multiple identical optional durations.

Well, entering a duration is an easy feat.  There is also the consideration that
if you are having nested \tuplet commands, specifying \tupletSpan becomes
non-trivial as it has to be specified as "absolute" timing whereas the duration
given to the \tuplet argument is scaled together with its contents (to
facilitate its scaling under music_compress, I had to use "duration" as the name
of the respective music property).  So
\tupletSpan 4 \tuplet 3/2 { c4 c c c c c c c c c c c }
does not yield anything useful when placed inside of
\tuplet 4/3
whereas
\tuplet 3/2 4 { c4 c c c c c c c c c c c }
works as expected.

So \tupletSpan is only a minor help, and it does not really save significant
typing, either.

The user-level documentation for this has not even been started, but I would
propose not putting too much stress on \tupletSpan.  The optional argument just
works more straightforwardly.

> Otherwise LGTM.

[Ian Hulin (gmail), 2013-01-12 16:43:42]

Thanks Trevor, good catch. Have re-drafted the doc-string accordingly.

Cheers,

Ian

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/2001/ly/music-functions-init.ly#n...
ly/music-functions-init.ly:1365: tuplets are being subdivided.  To unset, use a
zero duration like
On 2013/01/12 15:49:56, Ian Hulin (gmail) wrote:
> On 2013/01/12 12:54:25, Ian Hulin (gmail) wrote:
> > tuplets specified by the current \tuplet command are to be subdivided.  To
> > unset, use a zero duration like @samp{1*0}. Doing this will ensure the
spanner
> > for the music on the current \tuplet command spans all the music, rather
than
> > the value set to any prior \tupletSpan call.
> 
> tuplets specified by the current \tuplet command are to be subdivided.  To
> unset, use @code{\tupletSpan \default}. Doing this will ensure the spanner for
> the music on the current \tuplet command spans all the music, rather than
> the value set to any prior \tupletSpan call. In general, you are recommended
to
> use the optional @var{duration} parameter to the @code{\tuplet} command rather
> than @code{\tupletSpan}. 

tuplets specified by the current \tuplet command are to be subdivided.  To unset
it, use @code{\tupletSpan \default}. Doing this will ensure the spanner for the
music on the current \tuplet command spans all the music, rather than the value
set to any prior \tupletSpan call. The @code{\tupletSpan} command is provided as
a convenience for cases where several @code{\tuplet} commands are coded and they
all need to use the same alternative spanner length.

[Trevor Daniels, 2013-01-12 16:59:48]

On 2013/01/12 16:35:17, dak wrote:
> On 2013/01/12 16:16:55, Trevor Daniels wrote:
> 
> > I prefer \tupletSpan \default too, but I don't think we should
> > advise users not to use \tupletSpan.
> 
> The disadvantage of \tupletSpan is that it makes a non-local change.  That's
not
> copy&paste friendly and leads to non-self-contained code pieces.

True, but don't all context properties do that?  Many pieces
will use the same value of tuplet span throughout.
 
> There is also the consideration that
> if you are having nested \tuplet commands, specifying \tupletSpan becomes
> non-trivial as it has to be specified as "absolute" timing whereas the
duration
> given to the \tuplet argument is scaled together with its contents 

That's more significant.  The user-level docs will need to
warn about this.

[dak, 2013-01-12 17:56:16]

On 2013/01/12 16:43:42, Ian Hulin (gmail) wrote:

> tuplets specified by the current \tuplet command are to be subdivided.  To
unset
> it, use @code{\tupletSpan \default}. Doing this will ensure the spanner for
the
> music on the current \tuplet command spans all the music, rather than the
value
> set to any prior \tupletSpan call. The @code{\tupletSpan} command is provided
as
> a convenience for cases where several @code{\tuplet} commands are coded and
they
> all need to use the same alternative spanner length.

I had already rephrased both DOC strings to make them clearer.  Could you check
whether you still consider your proposal here better in all respects?  Or is
this text rather intended for the manual rather than the DOC string?

[Ian Hulin (gmail), 2013-01-12 23:22:36]

LGTM

https://codereview.appspot.com/7094044/diff/4003/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/4003/ly/music-functions-init.ly#n...
ly/music-functions-init.ly:1372: ")
LGTM

[dak, 2013-01-13 12:56:14]

Deal with 2.17.10 release by updating version numbers

[Keith, 2013-01-15 05:09:45]

https://codereview.appspot.com/7094044/diff/11001/lily/tuplet-iterator.cc
File lily/tuplet-iterator.cc (right):

https://codereview.appspot.com/7094044/diff/11001/lily/tuplet-iterator.cc#new...
lily/tuplet-iterator.cc:139: }
lgtm

https://codereview.appspot.com/7094044/diff/11001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/11001/ly/music-functions-init.ly#...
ly/music-functions-init.ly:1376: 
Perfect

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py
File python/convertrules.py (right):

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py#newc...
python/convertrules.py:3490: if (den & (den - 1)) != 0 :
 # if den is a power of two
A bit too clever to put in with no comment.  Somebody somewhere will have his
time needlessly wasted figuring this out... me, for example.

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py#newc...
python/convertrules.py:3502: return (r"\tupletSpan 1*%d/%d" % (num, den))
It would have been fine to handle all cases this way.

https://codereview.appspot.com/7094044/diff/11001/scm/define-music-display-me...
File scm/define-music-display-methods.scm (right):

https://codereview.appspot.com/7094044/diff/11001/scm/define-music-display-me...
scm/define-music-display-methods.scm:681: " "
total of two spaces ?  \tuplet 2/3  4{...}

https://codereview.appspot.com/7094044/diff/11001/scm/define-music-display-me...
scm/define-music-display-methods.scm:683: "")
I guess you wanted a space  " ")
so you get \tuplet 2/3 4 {...}
but I don't understand why you used string-append at all.

[dak, 2013-01-15 12:08:57]

Address Keith's comments, add display test, fix display expression

[dak, 2013-01-15 12:09:41]

https://codereview.appspot.com/7094044/diff/11001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/7094044/diff/11001/ly/music-functions-init.ly#...
ly/music-functions-init.ly:1376: 
On 2013/01/15 05:09:46, Keith wrote:
> Perfect

Comment or implementation?  I actually like how using \default has impacted code
complexity.

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py
File python/convertrules.py (right):

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py#newc...
python/convertrules.py:3490: if (den & (den - 1)) != 0 :
On 2013/01/15 05:09:46, Keith wrote:
>  # if den is a power of two
> A bit too clever to put in with no comment.  Somebody somewhere will have his
> time needlessly wasted figuring this out... me, for example.

I was not actually thinking anybody would ever look at the convert rules ever. 
They are not really active code, and fairly inconsistent overall.  At any rate,
added a comment.

https://codereview.appspot.com/7094044/diff/11001/python/convertrules.py#newc...
python/convertrules.py:3502: return (r"\tupletSpan 1*%d/%d" % (num, den))
On 2013/01/15 05:09:46, Keith wrote:
> It would have been fine to handle all cases this way.

Disagree.  Naturally occuring durations should be handled as such.  1*x/y is a
fallback.

https://codereview.appspot.com/7094044/diff/11001/scm/define-music-display-me...
File scm/define-music-display-methods.scm (right):

https://codereview.appspot.com/7094044/diff/11001/scm/define-music-display-me...
scm/define-music-display-methods.scm:683: "")
On 2013/01/15 05:09:46, Keith wrote:
> I guess you wanted a space  " ")
Here?  No.  The "" is not inside of the string-append at all.  It is the
else-branch of (if span ...

> so you get \tuplet 2/3 4 {...}
> but I don't understand why you used string-append at all.

The space is wrongly prepended rather than appended.  I did it via the format
string instead now.  Turns out that formatting here (after changing
*time-scale*) was the wrong thing to do anyway.