#5251 document restNumberThreshold

[Malte Meyn]

As it’s fairly common, recommended by Gould, and—depending on house style for longer measures—sometimes necessary to have single measure MMRs marked with a “1” in orchestral parts, it should be documented how this can be achieved:

\set restNumberThreshold = 0

The default seems to be “unset” but in multi-measure-rest-engraver.cc it is hardcoded to 1; you won’t find this value in engraver-init.ly.

[Malte Meyn]

Issue 5251/1: set default restNumberThreshold = 1
Issue 5251/2: add snippet for doc
Issue 5251/3: add snippet to NR

http://codereview.appspot.com/353850043

[Anonymous]

Passes make, make check and a full make doc.

[Thomas Morley]

Likely unrelated to this issue and worth another ticket...
Below returns unexpected result, at least for me

{
  \compressFullBarRests
  \set restNumberThreshold = 0
  R1 R1
  \set restNumberThreshold = 1
  R1 R1
}

[David Kastrup]

"Thomas Morley" thomas-morley@users.sourceforge.net writes:

Likely unrelated to this issue and worth another ticket...
Below returns unexpected result, at least for me
~~~
{
\compressFullBarRests
\set restNumberThreshold = 0
R1 R1
\set restNumberThreshold = 1
R1 R1
}
~~~

Adding another R1 to each group makes clearer what happens:
restNumberThreshold is consulted at the moment the rest ends, not when
it starts. Recording it at the time of the rest start would be
additional work. There is a reason many settings like these are
actually grob properties rather than context properties: that way they
are recorded at grob creation time. For a setting that decides whether
or not to even create a grob, that is a bit tricky but it could be
considered a property of the rest itself rather than of the rest number.

--
David Kastrup

[Malte Meyn]

fix bug mentioned at SF, add regtest

http://codereview.appspot.com/353850043

[Malte Meyn]

run makelsr

http://codereview.appspot.com/353850043

[Anonymous]

I thought I asked not to run makelsr?

I am going to test patch version 2 not 3 (without the makelsr).

Running makelsr for patch review is silly - look how many files have changed! How can anyone know what is 'real' and what is just the makelsr?

Please don't commit the patch (when it eventually goes through) with makelsr included as it makes it inconvenient if we have to revert anything or cherry pick the real fix (because the makelesr is effectively merged into the real patch).

So ... if patch version 3 contains anything other than the just makelsr output you will need to re-submit again.

[Malte Meyn]

I thought I asked not to run makelsr?

Yes, you did. But harm asked me to do so so that he could review. To make everyone happy I made those two patch sets.

I am going to test patch version 2 not 3 (without the makelsr).

That was what I intended. I’m sorry that I didn’t mention that.

Please don't commit the patch (when it eventually goes through) with makelsr included as it makes it inconvenient if we have to revert anything or cherry pick the real fix (because the makelesr is effectively merged into the real patch).

I won’t commit the makelsr run in the same commit as the other changes. I thought of something like

1st commit: Issue 5251/1: set default restNumberThreshold and fix a bug
2nd commit: Issue 5251/2: add snippet to NR (not the snippet file itself but only the change to rhythms.itely)
3rd commit: Issue 5251/3: add regtest
4th commit: Issue 5251/4: run makelsr

Would that be ok? Or should I leave out the makelsr run completely? Then the NR won’t build correctly, will it?

So ... if patch version 3 contains anything other than the just makelsr output you will need to re-submit again.

It doesn’t contain anything else.

[Thomas Morley]

I thought I asked not to run makelsr?

Running makelsr for patch review is silly - look how many files have changed! How can anyone know what is 'real' and what is just the makelsr?

Hi James,

he did because I asked to do so.
I think we need some guidelines, let's discuss on devel.

Cheers,
Harm

[Anonymous]

This fails make, the second patch no longer included the .../snippet/new/,,.. file which is still referrenced in the NR.

[Thomas Morley]

Yep.
Though, said snippet compiles with 2.18. Thus there's no need to put it in snippet/new, if it is present in /Documentation/snippets.
So the need to run makelsr.

To come out of the dilemma, would it be reasonable to commit a run of makelsr as a single patch and delay Malte's patch?

[Malte Meyn]

clean patchset without makelsr based on makelsr-ed master

http://codereview.appspot.com/353850043

[Anonymous]

Passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for January 5th

[Anonymous]

Patch counted down - please push

[Anonymous]

Patch counted down - please push

[Malte Meyn]

I’ll do as soon I manage to compile LilyPond again (see devel list) so that I can test the changes suggested by Harm (see Rietveld). They’re minor and IMHO won’t need review but I don’t want to push them untested.

[Anonymous]

Patch counted down - please push - you may need to rebase by now though Malte.