Correct convert-ly of page spacing

Several page spacing variables, formerly expressed in mm,
are now expressed in staff-spaces;
patch convert-ly to convert to the new units.

Correct text of a NOT_SMART

Add a NOT_SMART to catch cases the script cannot update automatically, and explanation in changes.tely using the same terminology as the NOT_SMART.

[Keith, 2011-01-05 00:07:37]

I wrote a conversion, inspired by Neil, a month ago, but have been unable to
make the regex match the variety of expressions that could have set the
old-style variable,
while behaving correctly in the case of a single-line-commented-out variable
setting.
% between-staff-padding = 0.5 \cm

I need your opinions. I would happily fall back to a NOT_SMART "this variable
must now be expressed in units of staff spaces"

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py
File python/convertrules.py (left):

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#oldcode2967
python/convertrules.py:2967: str = re.sub (r"page-top-space\s*=\s*#([0-9.]+)",
The # in the match text skipped the form
  page-top-space = 2\cm
any other reason for it?

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2967
python/convertrules.py:2967: str = re.sub
(r"\n[^%]*(page-top-space)\s*=[^\n}]*",
Please comment on use of \n in search string, versus flag=MULTILINE.  I tested
on Win and Linux, but not Mac.
Limitations: does not find
#(define page-top-space 3)
page-top-space \n = 3

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2968
python/convertrules.py:2968: r"\g<0> top-system-spacing #'space = #(/ \1
staff-space)",
The old variable assignment is retained, followed by
  new_variable = #(/ old_variable staff-space)
Assumes there is a syntactically valid location for the new_variable assignment
after the newline.

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2984
python/convertrules.py:2984: stderr_write(NOT_SMART % _("minimum-Y-extent;
vertical spacing no longer depends on the Y-extent of a VerticalAxisGroup.\n"))
Responding to discussion on issue 1298

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2991
python/convertrules.py:2991: str)
Responding to issue 1311

[Graham Percival (old account), 2011-01-05 05:35:51]

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2957
python/convertrules.py:2957: @rule ((2, 13, 10),
you're changing the rules for the 2.13.10 conversion?  hmm.  We don't normally
do this, but I guess it makes sense in this case.

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2990
python/convertrules.py:2990: r"% in the Notation Reference regarding
'staff-affinity = ##f.\n\g<0>",
I would rather go with our normal text in UPDATE_MANUALLY.  If you'd like a
longer discussion of what to change, I'd encourage you to do it in changes.tely.

[Keith, 2011-01-06 04:17:43]

My current plan is to avoid my concerns about line endings with a new patch
(tomorrow) that does not involve newlines.

Simple assignments 
 oldvar = 2 \in
would be converted into the longer line
 obsolete-oldvar = 2 \in  newvar #'newkey = #(/ obsolete-oldvar staff-space)

Assignments involving Scheme (more complicated than x = #3) would remain
untouched, but trigger a NOT_SMART.

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/3793046/diff/1/python/convertrules.py#newcode2990
python/convertrules.py:2990: r"% in the Notation Reference regarding
'staff-affinity = ##f.\n\g<0>",
On 2011/01/05 05:35:51, Graham Percival wrote:
> ... I'd encourage you to do it in changes.tely.

That's a better way. I'll show a changes.tely addition in the next patch.

[Keith, 2011-01-07 09:14:55]

New patch set is up.

> 
> you're changing the rules for the 2.13.10 conversion?  
> ...  but I guess it makes sense in this case.
>
so that older files which have not yet been through convert-ly get the affected
variables converted to staff-spaces.

A couple reg-tests already got convert-ly-marked >2.13.10, so I update them by
hand in this patch.

[Graham Percival (old account), 2011-01-08 05:47:06]

Looks fine, although I haven't actually tested it, but since it's not in the
core program I'm less concerned.  Could you email me the git patch and I'll push
it?

[Neil Puttock, 2011-01-08 17:53:54]

On 8 January 2011 05:47,  <percival.music.ca@gmail.com> wrote:
> Looks fine, although I haven't actually tested it, but since it's not in
> the core program I'm less concerned.  Could you email me the git patch
> and I'll push it?

Do you mind holding fire until I've had chance to look at it?

Cheers,
Neil

[Graham Percival, 2011-01-08 18:42:16]

On Sat, Jan 08, 2011 at 05:53:51PM +0000, Neil Puttock wrote:
> On 8 January 2011 05:47,  <percival.music.ca@gmail.com> wrote:
> > Looks fine, although I haven't actually tested it, but since it's not in
> > the core program I'm less concerned.  Could you email me the git patch
> > and I'll push it?
> 
> Do you mind holding fire until I've had chance to look at it?

k, no problem.

Cheers,
- Graham

[Keith, 2011-01-08 21:55:30]

On 2011/01/08 17:53:54, Neil Puttock wrote:
> Do you mind holding fire until I've had chance to look at it?
> 
Holding fire.
Testing might require a (trivial) merge with Carl's "Change stringTunings ..."
commit. I have a merged patch but hesitate to upload because I haven't yet seen
how Rietveld displays such patches.  If I get bold, but make a mess with Set 3,
just look at Patch Set 2.

[Neil Puttock, 2011-01-08 22:02:40]

On 8 January 2011 21:55,  <k-ohara5a5a@oco.net> wrote:

> Testing might require a (trivial) merge with Carl's "Change
> stringTunings ..." commit. I have a merged patch but hesitate to upload
> because I haven't yet seen how Rietveld displays such patches.  If I get
> bold, but make a mess with Set 3, just look at Patch Set 2.

Don't worry, I've already rebased the second patchset and will post a
few comments soon.

Cheers,
Neil

[Neil Puttock, 2011-01-08 22:32:46]

LGTM.

http://codereview.appspot.com/3793046/diff/6001/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/3793046/diff/6001/python/convertrules.py#newcod...
python/convertrules.py:2970: r"  top-system-spacing #'space = #(/ obsolete-\1
staff-space)",
newlines looks a bit tidier here (even if the indentation might not match users'
code)

http://codereview.appspot.com/3793046/diff/6001/python/convertrules.py#newcod...
python/convertrules.py:3144: stderr_write (NOT_SMART % "between-systems-space,
-padding.  " + UPDATE_MANUALLY)
between-system-space

[c_sorensen, 2011-01-08 22:41:00]

On 1/8/11 2:55 PM, "k-ohara5a5a@oco.net" <k-ohara5a5a@oco.net> wrote:

> On 2011/01/08 17:53:54, Neil Puttock wrote:
>> Do you mind holding fire until I've had chance to look at it?
> 
> Holding fire.
> Testing might require a (trivial) merge with Carl's "Change
> stringTunings ..." commit. I have a merged patch but hesitate to upload
> because I haven't yet seen how Rietveld displays such patches.  If I get
> bold, but make a mess with Set 3, just look at Patch Set 2.
> 
> http://codereview.appspot.com/3793046/

Just do git pull origin master

git rebase master

git-cl upload master

and everything should work well.

Thanks,

Carl

[Keith, 2011-01-08 23:23:15]

Patch set relative to current head, 
and one typo fixed in an output message.
Email patch to follow shortly to Graham.

Carl wrote> ... git-cl upload master
Thanks, Carl.  
I committed locally first, so relative to my local this gave an empty diff. 
git-cl upload origin/master 

So I see that Rietveld compares against the appropriate base for each patch set.
 Very nice.

http://codereview.appspot.com/3793046/diff/6001/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/3793046/diff/6001/python/convertrules.py#newcod...
python/convertrules.py:2970: r"  top-system-spacing #'space = #(/ obsolete-\1
staff-space)",
On 2011/01/08 22:32:46, Neil Puttock wrote:
> newlines looks a bit tidier here 

But %-commented-out page-spacing variables
 %  oldvar = 3\in
seem particularly common (on mutopia).  In such cases my assignment to newvar on
the next line would give an error.  When I tested that method and looked at the
error, I decided it might confuse reasonable users.

[c_sorensen, 2011-01-08 23:26:56]

On 1/8/11 4:23 PM, "k-ohara5a5a@oco.net" <k-ohara5a5a@oco.net> wrote:

> Patch set relative to current head,
> and one typo fixed in an output message.
> Email patch to follow shortly to Graham.
> 
> Carl wrote> ... git-cl upload master
> Thanks, Carl.
> I committed locally first, so relative to my local this gave an empty
> diff.  git-cl upload origin/master
> 
> So I see that Rietveld compares against the appropriate base for each
> patch set.  Very nice.

You'll want to develop the habit of using different branches for each patch
set.  Otherwise, all of your patches will want to go to the same Rietveld
issue.

HTH,

Carl