Issue 2445: Add measure counter to LilyPond

Issue 2445: Add measure counter to LilyPond

This patch allows the user to number groups of successive measures, which is useful as an aid to the performer in keeping track of repetitions.  There is no requirement that the counted measures be repeated, however.  The number is centered on the measure (discounting prefatory material), above the staff by default.

In the case of a measure split across a line break, all segments are numbered.  The first number has its normal appearance, while the following numbers are enclosed in parentheses.

A new grob, a spanner called `MeasureCounter,' is defined.  Each element of a count is an independent instance of this grob.  A new event-class, `measure-counter-event,' and a new interface, `measure-counter-interface,' are also defined.

The engraver, `Measure_counter_engraver,' must be added to the relevant context (probably Staff) in the .ly file.  This engraver has been written in Scheme, following the precedent of `Span_stem_engraver.'

The user frames the measures to count with the commands `\startMeasureCount' and `\stopMeasureCount.'  The initial number is read from a new grob-property, 'count-from, which may be overridden.

[david.nalesnik, 2012-10-17 13:34:45]

Please review.

Thanks!

-David

[janek, 2012-10-24 10:37:08]

I found some ambiguities in descriptions.

Janek

http://codereview.appspot.com/6730044/diff/10001/input/regression/measure-cou...
File input/regression/measure-counter-broken.ly (right):

http://codereview.appspot.com/6730044/diff/10001/input/regression/measure-cou...
input/regression/measure-counter-broken.ly:6: enclosed in parentheses."
Do you mean that the number before the break is not parenthesized, and the
number of the part after the break is parenthesized?
It may be worth rewording this sentence.

http://codereview.appspot.com/6730044/diff/10001/scm/define-event-classes.scm
File scm/define-event-classes.scm (right):

http://codereview.appspot.com/6730044/diff/10001/scm/define-event-classes.scm...
scm/define-event-classes.scm:49: tremolo-span-event tuplet-span-event))
i think there's something wrong with the indentation here, but considering how
nitpicking about indentation didn't do us any good in the past i'm leaning
towards not caring about it.

http://codereview.appspot.com/6730044/diff/10001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/6730044/diff/10001/scm/define-grob-properties.s...
scm/define-grob-properties.scm:204: (count-from ,integer? "The number beginning
a measure count.")
I suppose you mean "In a measure count, don't print numbers lower than this
one"?
I mean, count-from could work either like this
{
  \override MeasureCount count-from = 3
  \startMeasureCount 
  a1 % no number 
  b1 % no number
  a1 % 3
  b1 % 4
} 
or like this 
{
  \override MeasureCount count-from = 3
  \startMeasureCount 
  a1 % 3 
  b1 % 4
  a1 % 5
  b1 % 6
}
It's important to make it clear in which way count-from works.

http://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm
File scm/scheme-engravers.scm (right):

http://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#new...
scm/scheme-engravers.scm:27: spanners.  Each spanner is bounded by the first
command column of successive
what is a command column?

http://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#new...
scm/scheme-engravers.scm:61: ; if this is not done, _all_ command columns will
be numbered
so, sometimes all command columns will be nubmered?

or do you mean that the code needs to make sure that we're in a new bar because
if it didn't, all command columns would be numbered and that's something we
don't want?

In other words, maybe "_all_ command columns would be numbered" would be a
better wording?

http://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#new...
scm/scheme-engravers.scm:67: ; first column of measure, even if grace notes
appear?
umm, i don't understand.  Is it a question about what this code actually does? 
Or some kind of rhethoric-question-comment? ;)

[david.nalesnik, 2012-10-24 12:19:54]

Thanks for your review, Janek!

https://codereview.appspot.com/6730044/diff/10001/input/regression/measure-co...
File input/regression/measure-counter-broken.ly (right):

https://codereview.appspot.com/6730044/diff/10001/input/regression/measure-co...
input/regression/measure-counter-broken.ly:6: enclosed in parentheses."
On 2012/10/24 10:37:08, janek wrote:
> Do you mean that the number before the break is not parenthesized, and the
> number of the part after the break is parenthesized?
> It may be worth rewording this sentence.

Done.

https://codereview.appspot.com/6730044/diff/10001/scm/define-event-classes.scm
File scm/define-event-classes.scm (right):

https://codereview.appspot.com/6730044/diff/10001/scm/define-event-classes.sc...
scm/define-event-classes.scm:49: tremolo-span-event tuplet-span-event))
On 2012/10/24 10:37:08, janek wrote:
> i think there's something wrong with the indentation here, but considering how
> nitpicking about indentation didn't do us any good in the past i'm leaning
> towards not caring about it.

Yes, it looks pretty bad, and the original doesn't appear to be right either.  I
made this consistent with `rhythmic-event' below.

https://codereview.appspot.com/6730044/diff/10001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

https://codereview.appspot.com/6730044/diff/10001/scm/define-grob-properties....
scm/define-grob-properties.scm:204: (count-from ,integer? "The number beginning
a measure count.")
On 2012/10/24 10:37:08, janek wrote:

> I mean, count-from could work either like this
> {
>   \override MeasureCount count-from = 3
>   \startMeasureCount 
>   a1 % no number 
>   b1 % no number
>   a1 % 3
>   b1 % 4
> } 
> or like this 
> {
>   \override MeasureCount count-from = 3
>   \startMeasureCount 
>   a1 % 3 
>   b1 % 4
>   a1 % 5
>   b1 % 6
> }

The second example above is the way it works.  Hopefully my revisions make this
a bit clearer!

https://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm
File scm/scheme-engravers.scm (right):

https://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#ne...
scm/scheme-engravers.scm:27: spanners.  Each spanner is bounded by the first
command column of successive
On 2012/10/24 10:37:08, janek wrote:
> what is a command column?

In the IR we find:

currentCommandColumn (graphical (layout) object)
Grob that is X-parent to all current breakable (clef, key signature, etc.)
items.

I changed the capitalization and enclosed it in @code{ } elsewhere.  Hope this
makes it clearer (or a search more fruitful).

https://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#ne...
scm/scheme-engravers.scm:61: ; if this is not done, _all_ command columns will
be numbered
On 2012/10/24 10:37:08, janek wrote:
> so, sometimes all command columns will be nubmered?
> 
> or do you mean that the code needs to make sure that we're in a new bar
because
> if it didn't, all command columns would be numbered and that's something we
> don't want?

Yes, exactly.  If this check weren't in place, you would get many many numbers.
> 
> In other words, maybe "_all_ command columns would be numbered" would be a
> better wording?
> 	

Yes, it would.  I made some more substantial changes here.

https://codereview.appspot.com/6730044/diff/10001/scm/scheme-engravers.scm#ne...
scm/scheme-engravers.scm:67: ; first column of measure, even if grace notes
appear?
On 2012/10/24 10:37:08, janek wrote:
> umm, i don't understand.  Is it a question about what this code actually does?

> Or some kind of rhethoric-question-comment? ;)

Oh, I was just mimicking the variable name with its question mark.  The comment
refers to the need to check for the first column of the measure.  I made this a
little less chatty :)

[janek, 2012-10-24 12:47:15]

much clearer now, thanks!
LGTM
Janek

[pkx166h, 2012-10-24 14:05:35]

I've opened

http://code.google.com/p/lilypond/issues/detail?id=2924

for the Documentation in the NR

[pkx166h, 2012-10-24 14:10:03]

Also, should we include

Measure_counter_engraver

in the Staff context by default?

(it'd make documenting it simpler in the @lilypond if nothing else :) )

[janek, 2012-10-24 20:28:36]

On Wed, Oct 24, 2012 at 4:10 PM,  <pkx166h@gmail.com> wrote:
> Also, should we include
>
> Measure_counter_engraver
>
> in the Staff context by default?
>
> (it'd make documenting it simpler in the @lilypond if nothing else :) )

+1 from me :)
Janek

[david.nalesnik, 2012-10-25 11:51:04]

On 2012/10/24 20:28:36, janek wrote:
> On Wed, Oct 24, 2012 at 4:10 PM,  <mailto:pkx166h@gmail.com> wrote:
> > Also, should we include
> >
> > Measure_counter_engraver
> >
> > in the Staff context by default?
> >
> > (it'd make documenting it simpler in the @lilypond if nothing else :) )
> 
> +1 from me :)
> Janek

OK--done!

[dak, 2012-10-31 05:32:15]

Please make the indicated convert-ly-like changes.  Thanks!

http://codereview.appspot.com/6730044/diff/32001/input/regression/measure-cou...
File input/regression/measure-counter.ly (right):

http://codereview.appspot.com/6730044/diff/32001/input/regression/measure-cou...
input/regression/measure-counter.ly:20: \override Staff.MeasureCounter
#'count-from = #2
This should be
  \override Staff.MeasureCounter.count-from = #2
to simulate convert-ly for 2.17.6 being run (sorry).

http://codereview.appspot.com/6730044/diff/32001/input/regression/measure-cou...
input/regression/measure-counter.ly:26: \revert Staff.MeasureCounter
#'count-from
Should now be

  \revert Staff.MeasureCounter.count-from

[dak, 2012-10-31 05:35:53]

http://codereview.appspot.com/6730044/diff/32001/input/regression/measure-cou...
File input/regression/measure-counter.ly (right):

http://codereview.appspot.com/6730044/diff/32001/input/regression/measure-cou...
input/regression/measure-counter.ly:1: \version "2.17.5"
Oops: also change version header, or just run convert-ly -ed for doing all
combined changes before committing.

[david.nalesnik, 2012-10-31 11:41:19]

David,

I don't mind making the changes, but I'm running into some problems.  I used git
pull -r before making a patch set and uploading to rietveld, but looking over
the patch set there I see that other changes have found their way in--including
your check-grob-path function.

I'm probably making some silly error here, and I appreciate any advice you can
give me for this.  (Unfortunately, I probably won't be able to get this until
some hours from now.)

[dak, 2012-10-31 11:47:37]

On 2012/10/31 11:41:19, david.nalesnik wrote:
> David,
> 
> I don't mind making the changes, but I'm running into some problems.  I used
git
> pull -r before making a patch set and uploading to rietveld, but looking over
> the patch set there I see that other changes have found their way
in--including
> your check-grob-path function.

check-grob-path is in current master.  Rietveld is not all too clever about
changing material underneath a patch, but you don't really need a separate
review for those changes, as they are just a consequence of running convert-ly,
and your patch already passed countdown.

If you are worried, you can send me the patch and I'll do what is needed.

[david.nalesnik, 2012-10-31 11:55:12]

Thank you--I am a bit worried!

Speaking of "patch," when I run git format-patch I get six separate patches.  Is
there any way to compress them into one so this is more convenient for you?

-David

[dak, 2012-10-31 11:58:42]

http://codereview.appspot.com/6730044/diff/40002/ly/engraver-init.ly
File ly/engraver-init.ly (right):

http://codereview.appspot.com/6730044/diff/40002/ly/engraver-init.ly#newcode88
ly/engraver-init.ly:88: \consists "Footnote_engraver"
Any idea where this line is from?  It just appears in the last patch set.

And it should have been removed with
commit 5801d8b438473ad68ae74f7dc742bbc8eea887fa

So the question is why it resurfaces here.

[dak, 2012-10-31 12:01:15]

On 2012/10/31 11:55:12, david.nalesnik wrote:
> Thank you--I am a bit worried!
> 
> Speaking of "patch," when I run git format-patch I get six separate patches. 
Is
> there any way to compress them into one so this is more convenient for you?
> 
> -David

Try if you can git send-email to work for you.  That still results in 6
different messages, but applying them with git am is easy.