Issue 7404046: changes.tely: deal with \transposition and instrumentTransposition changes (Closed)
|
Created:
11 years, 2 months ago by dak Modified:
11 years, 2 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
Descriptionchanges.tely: deal with \transposition and instrumentTransposition changes
Those were introduced by issue 754 and issue 3168, respectively.
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix Texinfo error, swap items like Trevor suggested #
Total comments: 2
MessagesTotal messages: 12
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.
Sign in to reply to this message.
Fix Texinfo error, swap items like Trevor suggested
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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).
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.
Sign in to reply to this message.
Message was sent while issue was closed.
LGTM
Sign in to reply to this message.
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.
Sign in to reply to this message.
"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
Sign in to reply to this message.
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.
Sign in to reply to this message.
|
