Issue 5251/1: set default restNumberThreshold = 1 (issue 353850043 by address@hidden)

[thomasmorley65]

Hi Malte,

some concerns:



https://codereview.appspot.com/353850043/diff/1/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):

https://codereview.appspot.com/353850043/diff/1/Documentation/notation/rhythms.itely#newcode966
Documentation/notation/rhythms.itely:966:
{numbering-single-measure-rests.ly}
I tried to test your patch, but 'make' failed with
lilypond-book.py: error: file not found:
numbering-single-measure-rests.ly
I had to exclude this lines to finish 'make'

Is there the _need_ to import lsr first?
If so, I think it should be part of the current patch in an own commit.
Imho, it's good practise to have every issue's patch-set being able to
stand alone.

https://codereview.appspot.com/353850043/diff/1/Documentation/snippets/new/numbering-single-measure-rests.ly
File Documentation/snippets/new/numbering-single-measure-rests.ly
(right):

https://codereview.appspot.com/353850043/diff/1/Documentation/snippets/new/numbering-single-measure-rests.ly#newcode14
Documentation/snippets/new/numbering-single-measure-rests.ly:14:
\relative {
no need for \relative here

Meanwhile I've approved this snippet in LSR (deleting \relative). So I
don't think there is any need to put it in Documentation/snippets/new as
well.
It will be available after next lsr-import anyway.

https://codereview.appspot.com/353850043/diff/1/lily/multi-measure-rest-engraver.cc
File lily/multi-measure-rest-engraver.cc (right):

https://codereview.appspot.com/353850043/diff/1/lily/multi-measure-rest-engraver.cc#newcode186
lily/multi-measure-rest-engraver.cc:186: int t = scm_to_int (thres);
I don't think this is the correct way.

We want a fall-back-value if restNumberThreshold is unset. Only setting
it in engraver-init.ly is not sufficient, imho, because setting
context-properties will not create a reversible stack. Thus this example
fails:
\relative {
  \compressFullBarRests
  \set restNumberThreshold = 0
  R1 R1*2
  \unset restNumberThreshold
  R1 R1*2
}
with:
ERROR: Wrong type (expecting exact integer): ()
Leaving the user perplexed what he might have done.

Not sure if the old coding is the best way o provide such a fall-back,
my C++-knowledge is close to zero, so I can't come up with a better
proposal.

https://codereview.appspot.com/353850043/