Make \shiftOff an assertive \override, not a \revert

Docs: convert-ly by hand shiftOff; issue 4242
  
   Documentation already described new behavior,
   so version-bump only
   Earlier commit a1d73a332fc4d1cb99bace19a5ec2c35262429e7
   missed the translations
    
convert-ly: catch all bflat -> b-flat; issue 4372

[dak, 2015-05-10 12:06:25]

https://codereview.appspot.com/190500043/diff/1/python/convertrules.py
File python/convertrules.py (right):

https://codereview.appspot.com/190500043/diff/1/python/convertrules.py#newcod...
python/convertrules.py:3731: str = re.sub (r'\\shiftOff\b', r'\\undo\\shiftOn',
str)
I'm currently trying to apply a convert-ly rule and this rule completely messes
up the translations.  I have failed to notice this during review.

WTF?

It is apparently such a bad idea that this patch bumps the respective version
numbers for the documentation manually in order to avoid the triggering of this
rule.

If it is expected that people seriously worked with the previous semantics of
\shiftOff as contrasted to after the change, we should not have changed its
behavior without replacement in the first place.

I am also less than enthused that the responsible commit does not refer to the
issue number, the commit title does not match the issue title, and the commit id
30a6581b78cd49b6600d43e8aa7b6d9e35cabb31 is not recorded in the issue report.

Part of the reason is that this issue was marked "Verified" before the bug squad
could check that all the respective information for successful verification is
in the issue report.

This makes it hard to find the issue.

In addition, the following commit
commit a1d73a332fc4d1cb99bace19a5ec2c35262429e7
Author: Keith OHara <k-ohara5a5a@oco.net>
Date:   Tue Dec 30 23:30:27 2014 -0800

    Update by hand instead of convert-ly
    
    The documentation already described the new behavior of \shiftOff

did not bother updating the version numbers in the translations.  In
consequence, running scripts/auxiliar/update-with-convert-ly.sh now will
overwrite the German, Spanish, Catalan, French, Hungarian, Italian, Japanese and
other translations with inappropriate changes.

This blocks issue 3229 from getting applied/pushed as it requires doing a
convert-ly update.

I am opening a new issue for removing this rule.

[Keith, 2015-05-10 15:44:17]

On Sun, 10 May 2015 05:06:24 -0700, <dak@gnu.org> wrote:

> In addition, the following commit
> commit a1d73a332fc4d1cb99bace19a5ec2c35262429e7
> Author: Keith OHara <k-ohara5a5a@oco.net>
> Date:   Tue Dec 30 23:30:27 2014 -0800
>
>      Update by hand instead of convert-ly
>
>      The documentation already described the new behavior of \shiftOff
>
> did not bother updating the version numbers in the translations.  In
> consequence, running scripts/auxiliar/update-with-convert-ly.sh now will
> overwrite the German, Spanish, Catalan, French, Hungarian, Italian,
> Japanese and other translations with inappropriate changes.
>

Oops.  Yep, I had intended to bump version numbers across translations.
I'll clean that up in an follow-up to issue 44242

[dak, 2015-05-10 15:48:49]

On 2015/05/10 15:44:17, Keith wrote:
> On Sun, 10 May 2015 05:06:24 -0700, <mailto:dak@gnu.org> wrote:
> 
> > In addition, the following commit
> > commit a1d73a332fc4d1cb99bace19a5ec2c35262429e7
> > Author: Keith OHara <mailto:k-ohara5a5a@oco.net>
> > Date:   Tue Dec 30 23:30:27 2014 -0800
> >
> >      Update by hand instead of convert-ly
> >
> >      The documentation already described the new behavior of \shiftOff
> >
> > did not bother updating the version numbers in the translations.  In
> > consequence, running scripts/auxiliar/update-with-convert-ly.sh now will
> > overwrite the German, Spanish, Catalan, French, Hungarian, Italian,
> > Japanese and other translations with inappropriate changes.
> >
> 
> Oops.  Yep, I had intended to bump version numbers across translations.
> I'll clean that up in an follow-up to issue 44242

Please check out issue 4384 as well: there are some updated findings regarding
the current impact on our documentation.

[dak, 2015-05-10 18:02:21]

It's not clear why you are now mixing in variants of conversion rules for issue
4372.  There already is a more thorough conversion rule for issue 4372 in review
and I don't see any comment of yours on it.  While there may have been some
overlap in effort, it does not make sense to mix that issue with this one.

But at any rate I think you need to explain why you think that you want a
convert-ly rule to retain behavior that you consider
a) to not correspond to the documentation or expected behavior
b) to not deserve a command of its own
c) you do not consider worth documenting separately
d) which requires explicit protection of the documentation from conversion

Adding a convert-ly rule here makes no sense in connection with the rationale
with which the change was done in the first place.  All the cleanup required to
let LilyPond itself escape unscathed from the conversion should be a red flag
that it was not a good idea to start with.

[Keith, 2015-05-10 21:00:26]

On Sun, 10 May 2015 11:02:21 -0700, <dak@gnu.org> wrote:

> It's not clear why you are now mixing in variants of conversion rules
> for issue 4372.

I noticed that my rule made the same \b mistake as yours, and they are right
next to each other, and I thought it was an easy fix, so I tried fixing both at
once.

> But at any rate I think you need to explain why you think that you want
> a convert-ly rule to retain behavior that you consider
> a) to not correspond to the documentation or expected behavior
> b) to not deserve a command of its own
> c) you do not consider worth documenting separately
> d) which requires explicit protection of the documentation from
> conversion

It was hard to decide whether to have the convert-ly rule or not for \shiftOff.

For existing music that used the old shiftOff = \revert
NoteColumn.horizontal-shift,
the replacement \undo\shiftOn  has the same expansion.

I searched for uses of \shiftOff and found that Urs Metzger has a few pieces on
mutopiaproject that use \shiftOn \shiftOff pairs as if they were a set/revert
pair.  The convert-ly rule doesn't help for those examples, though, because they
have hand-adjusted shifts in the other voices.

The \revert version generated "too many clashing notecolumns" warnings, which is
now "need a \shiftXxx", and I thought it a bit safer to keep a warning.

The manuals did not use \shiftOff in LilyPond code where it had any effect, but
only talked about its desired effect in the text.  Thus the manuals should not
get any convert-ly rule.


If you revert the convert-ly rule, it would be more tidy if you also reverse its
changes in
'Documentation/learning/fundamental.itely' and
'Documentation/fr/learning/fundamental.itely'
Only three lines each file.   :%s/\\undo\\shiftOn/\\shiftOff/g

[dak, 2015-05-10 22:50:36]

On 2015/05/10 21:00:26, Keith wrote:
> On Sun, 10 May 2015 11:02:21 -0700, <mailto:dak@gnu.org> wrote:
> 
> > It's not clear why you are now mixing in variants of conversion rules
> > for issue 4372.
> 
> I noticed that my rule made the same \b mistake as yours, and they are right
> next to each other, and I thought it was an easy fix, so I tried fixing both
at
> once.

Well, I guess I got the more thorough fix.  This is a rather nasty pattern.

At any rate, there is a different issue for it.

> > But at any rate I think you need to explain why you think that you want
> > a convert-ly rule to retain behavior that you consider
> > a) to not correspond to the documentation or expected behavior
> > b) to not deserve a command of its own
> > c) you do not consider worth documenting separately
> > d) which requires explicit protection of the documentation from
> > conversion
> 
> It was hard to decide whether to have the convert-ly rule or not for
\shiftOff.
> 
> For existing music that used the old shiftOff = \revert
> NoteColumn.horizontal-shift,
> the replacement \undo\shiftOn  has the same expansion.
> 
> I searched for uses of \shiftOff and found that Urs Metzger has a few pieces
on
> mutopiaproject that use \shiftOn \shiftOff pairs as if they were a set/revert
> pair.  The convert-ly rule doesn't help for those examples, though, because
they
> have hand-adjusted shifts in the other voices.
> 
> The \revert version generated "too many clashing notecolumns" warnings, which
is
> now "need a \shiftXxx", and I thought it a bit safer to keep a warning.

What's safer here?  For old code where people did not bother heeding the warning
anyway (possibly because the output was ok for them) now the warning is no
longer there.  For new code the convert-ly rule is not relevant.

> If you revert the convert-ly rule, it would be more tidy if you also reverse
its
> changes in
> 'Documentation/learning/fundamental.itely' and
> 'Documentation/fr/learning/fundamental.itely'
> Only three lines each file.   :%s/\\undo\\shiftOn/\\shiftOff/g

I mentioned the pending cleanup in issue 4372.  I'd lean towards rewinding the
version numbers as well but would check that they are not warranted for other
changes.

[Keith, 2015-05-11 04:19:56]

On 2015/05/10 22:50:36, dak wrote:
 
>> The \revert version generated "too many clashing notecolumns" warnings, which
>> is now "need a \shiftXxx", and I thought it a bit safer to keep a warning.
>
> What's safer here?  For old code where people did not bother heeding the
warning
> anyway (possibly because the output was ok for them) 

Well, I think the newer warning is much more helpful,
and the updater of the code is more experienced than he was when he first wrote
the code.

There is also the (remote) possibility that someone wrote a function depending
on \shiftOff being a \revert -- maybe using \shiftOff as part of restoring the
former state, or something.

I can't find an example where convert-ly's replacing \shiftOff, with code to
keep the old behavior, is actually helpful.  So, I'll just reverse the unwanted
changes in the docs.

[dak, 2015-05-11 14:29:06]

https://codereview.appspot.com/190500043/diff/80001/Documentation/fr/learning...
File Documentation/fr/learning/fundamental.itely (left):

https://codereview.appspot.com/190500043/diff/80001/Documentation/fr/learning...
Documentation/fr/learning/fundamental.itely:1165: @funindex \undo\shiftOff
This one is actually not just a convert-ly effect but additionally a manual
translation/transcription error.  The convert-ly rule rather produced
\undo\shiftOn in the untranslated document.