changes.tely: deal with \transposition and instrumentTransposition changes

changes.tely: deal with \transposition and instrumentTransposition changes

Those were introduced by issue 754 and issue 3168, respectively.

[Trevor Daniels, 2013-02-24 13:01:28]

LGTM apart from the suggested change.

https://codereview.appspot.com/7404046/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/7404046/diff/1/Documentation/changes.tely#newc...
Documentation/changes.tely:83: would have been the other way round.  This and
the previous change
previous?
Perhaps invert the order of these two items.

[dak, 2013-02-24 23:59:18]

Fix Texinfo error, swap items like Trevor suggested

[dak, 2013-02-25 00:01:50]

Hope I interpreted Trevor's comment correctly.  The roposed "new" order did not
occur to me since it violates causation: the (now) first change is dependent on
the second one.  However, that is on an implementation level and probably not
interesting to the user.

https://codereview.appspot.com/7404046/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/7404046/diff/1/Documentation/changes.tely#newc...
Documentation/changes.tely:83: would have been the other way round.  This and
the previous change
On 2013/02/24 13:01:28, Trevor Daniels wrote:
> previous?
> Perhaps invert the order of these two items.

Done.

[Trevor Daniels, 2013-02-25 16:02:41]

On 2013/02/25 00:01:50, dak wrote:
> Hope I interpreted Trevor's comment correctly.  

Not quite, but it is hardly a point worth labouring
over.  Changes are listed with the most recent at the
top, and 'previously' means 'earlier in time', so it
ought to refer to items lower in the list.  Perhaps
it would be better to use a term than has no connotation
of time, like 'above' or 'below'.  I don't mind in which
order the two items appear, as long as the direction of
the reference corresponds.

Whether changed or not, LGTM.

Apologies for nitpicking,
Trevor

[dak, 2013-02-25 16:15:53]

On 2013/02/25 16:02:41, Trevor Daniels wrote:

> Not quite, but it is hardly a point worth labouring
> over.  Changes are listed with the most recent at the
> top, and 'previously' means 'earlier in time', so it
> ought to refer to items lower in the list.

Our changes list is not really ordered in strict reverse chronology for the
reader.  That's the way it is accumulating (unless several changes to one area
are getting conflated, in which case changes may appear well below the top).

I don't see "following" as indicating a time order, and "below" is not really
specific.  I'd have no qualms doing a one-word change without rerunning reviews,
but I don't think that the one-word change "below" carries the full information,
and "next change below" is not much better.  So unless I get a wording that
fares better in terms of being understandable, I'd lean towards sticking with
the current text.

I think people can be expected to read from top to bottom, and I find specifying
things in terms of reading order most natural.

[Keith, 2013-02-27 05:58:58]

LGTM

https://codereview.appspot.com/7404046/diff/4001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/7404046/diff/4001/Documentation/changes.tely#n...
Documentation/changes.tely:71: would have been the other way round.  This and
the following change
Maybe "With the related change to @code{\transpose}, this makes dealing with
..."
But I would guess people will understand 'following' to mean following in
reading order, as you intended.

https://codereview.appspot.com/7404046/diff/4001/Documentation/changes.tely#n...
Documentation/changes.tely:84: was equivalent to @code{\transposition f'}.  Now
it stays
(and @code{\whateverItWasCalled f'} is available in case an old score depended
on the old behavior).

[Keith, 2013-02-27 07:40:59]

On 2013/02/27 05:58:58, Keith wrote:

> (and @code{\whateverItWasCalled f'} is available

Oops.  It was \oldTransposition  but it was not put into LilyPond.

On the one hand, any score that used \transposition p  in music that goes
through transpose will change its behavior.

On the other hand, I /think/ that the only cases where the old behavior was
useful would have one \transposition at the start of the section of music, so
transforming to the new LilyPond is simple---rather, relatively simple in the
world of writing for transposing instruments.

Back to the first hand, search/replace to use \oldTransposition  is easier to
explain to the impatient, than how to write for transposing instruments.

[dak, 2013-02-27 08:47:21]

On 2013/02/27 07:40:59, Keith wrote:
> On 2013/02/27 05:58:58, Keith wrote:
> 
> > (and @code{\whateverItWasCalled f'} is available
> 
> Oops.  It was \oldTransposition  but it was not put into LilyPond.

Since the sign of instrumentTransposition has been inverted, it would require
serious trickery or a separate music event type to implement \oldTransposition
with the previous semantics.  Getting the previous semantics for instrument
definitions seems even worse.

> On the one hand, any score that used \transposition p  in music that goes
> through transpose will change its behavior.

Yes.  I think the most likely candidates would be
\transpose c' bes { \transposition c' ...
where "\transposition c'" is used in the meaning "I don't care what you do with
\transpose, the following is concert pitch".

It might make some sense to complain about this special case and/or the pattern
\transpose [letters, spaces, octaves]*{[space]\transposition
in convert-ly rules.

> On the other hand, I /think/ that the only cases where the old behavior was
> useful would have one \transposition at the start of the section of music, so
> transforming to the new LilyPond is simple---rather, relatively simple in the
> world of writing for transposing instruments.

> Back to the first hand, search/replace to use \oldTransposition  is easier to
> explain to the impatient, than how to write for transposing instruments.

The problem I have with supporting \oldTransposition behavior in the standard
code is that we won't ever get rid it again.  We still have an "old-relative"
option to revert to LilyPond 1.8 behavior for \relative, and the associated code
paths.  Probably nobody has a clue what this actually does, or whether it
actually still does what it was intended to do once.

[Graham Percival, 2013-02-28 23:48:27]

LGTM

[Keith, 2013-03-03 07:58:54]

On Wed, 27 Feb 2013 00:47:21 -0800, <dak@gnu.org> wrote:

> On 2013/02/27 07:40:59, Keith wrote:
>
>> Oops.  It was \oldTransposition  but it was not put into LilyPond.
>
> Since the sign of instrumentTransposition has been inverted, it would
> require serious trickery or a separate music event type to implement
> \oldTransposition with the previous semantics.

The \oldTransposition you suggested in <https://codereview.appspot.com/7303057>
has the required trickery, and I just re-checked that it works fine.

If I have time I'll point to it in changes.tely, just in case someone used the
old method for something too complicated to easily convert by hand.

> Getting the previous
> semantics for instrument definitions seems even worse.

Probably no-one has used \addInstrumentDefinition for transposition, but if
someone has, the instrumentTransposition assignments already use ly:make-pitch,
so inserting ly:pitch-negate is not too much work.

[dak, 2013-03-03 08:18:27]

"Keith OHara" <k-ohara5a5a@oco.net> writes:

> On Wed, 27 Feb 2013 00:47:21 -0800, <dak@gnu.org> wrote:
>
>> On 2013/02/27 07:40:59, Keith wrote:
>>
>>> Oops.  It was \oldTransposition  but it was not put into LilyPond.
>>
>> Since the sign of instrumentTransposition has been inverted, it would
>> require serious trickery or a separate music event type to implement
>> \oldTransposition with the previous semantics.
>
> The \oldTransposition you suggested in
> <https://codereview.appspot.com/7303057> has the required trickery,
> and I just re-checked that it works fine.

Would you believe that I had already forgotten about this contraption?
It did not exactly get a favorable review from you, either.

Ok, but we'd still need to figure out how to deal with this abomination.
I'd prefer not giving it a name of its own, instead load a Scheme file
on-demand that redefines \transposition (of course, that does not work
using the given definition with #{ \transposition ... #} inside but that
is easily fixed).  That way, our proper code base steers clear of this
kind of cruft.

The question is what convert-ly should try doing about it.  Just issue a
diagnostic when seeing \transposition?  Load the compatibility file
unconditionally?  Load/warn only given certain patterns?

> Probably no-one has used \addInstrumentDefinition for transposition,
> but if someone has, the instrumentTransposition assignments already
> use ly:make-pitch, so inserting ly:pitch-negate is not too much work.

Patterns for that will not exactly be easy to make since
instrumentTransposition is usually an alist.

-- 
David Kastrup

[Keith, 2013-03-04 04:15:12]

On Sun, 03 Mar 2013 00:18:09 -0800, David Kastrup <dak@gnu.org> wrote:

> "Keith OHara" <k-ohara5a5a@oco.net> writes:
>
>> The \oldTransposition you suggested in
>> <https://codereview.appspot.com/7303057> has the required trickery,
>> and I just re-checked that it works fine.

> I'd prefer not giving it a name of its own, instead load a Scheme file
> on-demand that redefines \transposition ...

The backward-compatible \oldTransposition is *much* more useful if it can be
used alongside \transposition.  If you were unfortunate enough to depend on the
misfeature, you will probably want to change only the few instances of
\transpose to \oldTransposition necessary to make cue notes and/or MIDI come out
correctly under 2.17.11, and them move or adjust the \oldTranpositions to the
correct \transposition, checking the output as you go.

> Just issue a diagnostic when seeing \transposition?

Yes, just one message per file that contains "\transposition ".  No automatic
conversion because most uses will need no change, and we cannot tell which uses
might be passed through a \transpose operation.

> Patterns for that will not exactly be easy to make since
> instrumentTransposition is usually an alist.

No convert-ly rule at all is the most appropriate here, because every known
setting of instrumentTransposition /tried/ to use the /new/ behavior (and thus
would not have worked previously).

For example in <http://www.mutopiaproject.org/cgibin/piece-info.cgi?id=119> for
version 2.9, you find
     \tag #'horn-A {
         \key c \major
         \set Staff . instrumentTransposition = #(ly:make-pitch -1 5 0)
         \clef treble }
There are no cues, so no problems in the printed output, but the MIDI is rather
unharmonious.