#4384 Patch: Remove convert-ly rule \shiftOff -> \undo\shiftOn

[Anonymous]

Originally created by: *anonymous

Originally created by: dak@gnu.org
Originally owned by: dak@gnu.org

Remove convert-ly rule \shiftOff -> \undo\shiftOn

For the rationale, see
<URL:https://codereview.appspot.com/190500043/#msg1>

The presence of the rule will cause similarly surprising effects to
users as it would to the current state of documentation translations.

Even if it were properly mentioned in the diagnostic description of
the conversion rule, people would rarely look closely enough to
notice.

http://codereview.appspot.com/233290043

[Google Importer]

Originally posted by: dak@gnu.org

(No comment was entered for this change.)

Owner: dak@gnu.org

[Google Importer]

Originally posted by: dak@gnu.org

(No comment was entered for this change.)

Blocking: lilypond:3229

[Google Importer]

Originally posted by: dak@gnu.org

The corresponding Google issue of the problematic change is issue 4242.

Labels: -Type-Enhancement Type-Maintainability

[Google Importer]

Originally posted by: dak@gnu.org

Aaand the rule has botched up the English manuals in the translation branch as well via
commit [rca016880c383a964f3d3542cfba64ada0e1d41cd]
Author: Trevor Daniels <t.daniels@treda.co.uk>
Date:   Wed Apr 29 16:19:05 2015 +0100

    Issue 3687 (part 0): run convert-ly
   
      Run convert-ly on the doc files to be changed to bring them
      up to date.

and has then been "translated" into
commit [reb64473ed20845c5872250192c680f53ef7ee251]
Author: Jean-Charles Malahieude <lilyfan@orange.fr>
Date:   Fri May 8 18:20:07 2015 +0200

    Doc-fr: updates LM and NR

This is going to need a lot of cleanup in addition to removing the rule.

Cc: k-ohara5...@oco.net

[Google Importer]

Originally posted by: dak@gnu.org

Patchy the autobot says: passes tests.  Not terribly surprising since a convert-ly rule change is nothing Patchy would cover

Labels: -Patch-new Patch-review

[Google Importer]

Originally posted by: lily...@orange.fr

I would have warned if I'd noticed anything in the logs, but nothing just after a merge of master into translation, nor after a full build from scratch before re-merging into staging.

[Google Importer]

Originally posted by: dak@gnu.org

The convert-ly rule replaced valid code by valid code, so there is little to warn about from Patchy.

The problem is just that as a rule, the replaced code is not wanted.  Particularly not in the documentation, but quite likely not elsewhere either.

[Google Importer]

Originally posted by: pkx1... (code.google.com)@gmail.com

If it helps I manually tested this patch with a full make doc and it passed.

[Google Importer]

Originally posted by: dak@gnu.org

Well, there is really nothing worthwhile for automatic testing since the Patch just removes one rule action for convert-ly and that rule is very well separated from anything else.  So there is really nothing at all to test here, only to discuss.

The reversal of the rule should be accompanied with a cleanup of the affected documentation files.  In fact, the documentation files should be cleaned up even when keeping the rule, the only difference being that with the rule kept it would be unsafe to also rewind the version numbers.

That cleanup will be too late for the currently rolling release I think.  So we still have a two-week time window to get that under wraps.  The cleanup would warrant Patchy testing once it is prepared: while it is simple to do, it will be mostly manual and manual changes have the potential for accidents.

So at any rate: thanks for testing this, and sorry for not having made clearer that this effort would likely have been better spent on a different phase of cleaning up.

[Google Importer]

Originally posted by: dak@gnu.org

Clean up documentation as well

http://codereview.appspot.com/233290043

Labels: -Patch-review Patch-new

[Google Importer]

Originally posted by: pkx1... (code.google.com)@gmail.com

Patchy the autobot says: passes tests.  includes a full make doc

Labels: -Patch-new Patch-review

[Google Importer]

Originally posted by: pkx1... (code.google.com)@gmail.com

Patch on countdown for May 19th

Labels: -Patch-review Patch-countdown

[Google Importer]

Originally posted by: dak@gnu.org

I'll be applying this before the end of countdown once the other issues on "Patch-push" have been pushed in order to commit issue 3229.  So holler soon if this should be split into separate patches or you have other considerations.

[Google Importer]

Originally posted by: dak@gnu.org

Pushed to staging as
commit [r6d2a63ff019f548037a513176dc09cead6e53408]
Author: David Kastrup <dak@gnu.org>
Date:   Sun May 17 19:19:07 2015 +0200

    Run scripts/auxiliar/update-with-convert-ly.sh
   
    Also some whitespace fixes.

commit [r7e9e338479796af1ef2ef66fca98240ff6da273a]
Author: David Kastrup <dak@gnu.org>
Date:   Mon May 11 16:15:54 2015 +0200

    Issue 4384/2: Undo effects of \shiftOff convert-ly rule
   
    Includes the effects of several manual operations in consequence

commit [rc1793a8d3c9943293ebcd5725dcbfa26bb2d99ad]
Author: David Kastrup <dak@gnu.org>
Date:   Sun May 10 14:17:48 2015 +0200

    Issue 4384: Remove convert-ly rule \shiftOff -> \undo\shiftOn
   
    For the rationale, see
    <URL:https://codereview.appspot.com/190500043/#msg1>
   
    The presence of the rule will cause similarly surprising effects to
    users as it would to the current state of documentation translations.
   
    Even if it were properly mentioned in the diagnostic description of
    the conversion rule, people would rarely look closely enough to
    notice.

Labels: -Patch-countdown Fixed_2_19_21
Status: Fixed

[Google Importer]

Originally posted by: dak@gnu.org

(No comment was entered for this change.)

Blocking: -lilypond:3229

[Google Importer]

Originally posted by: PhilEHol...@googlemail.com

Verified from git

Status: Verified