Let Merge_rests_engraver deal with dotted rests

Let Merge_rests_engraver deal with dotted rests

Compare simple rests by their duration-length, duration-log does
not take possible dots into account.
Superfluous dots are killed with ly:grob-suicide!

[thomasmorley651, 2017-08-28 14:26:51]

Please review.

Does this engraver needs an entry via ly:register-translator like the
Measure_counter_engraver in the same file (scheme-engravers.scm)?

[dak, 2017-08-28 15:18:06]

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

https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newc...
scm/scheme-engravers.scm:121: (define-public (Merge_rests_engraver context)
Stupid question: is there a reason Merge_rests_engraver is unregistered?  That
keeps it from appearing in the IR and precludes its use with

\consists Merge_rests_engraver

like other engravers.

[thomasmorley651, 2017-08-28 16:01:36]

On 2017/08/28 15:18:06, dak wrote:
> https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm
> File scm/scheme-engravers.scm (right):
> 
>
https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newc...
> scm/scheme-engravers.scm:121: (define-public (Merge_rests_engraver context)
> Stupid question: is there a reason Merge_rests_engraver is unregistered?  

How to do so? With ly:register-translator?
That was my question in the announcement-massage on devel.

[dak, 2017-08-28 16:29:52]

On 2017/08/28 16:01:36, thomasmorley651 wrote:
> On 2017/08/28 15:18:06, dak wrote:
> > https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm
> > File scm/scheme-engravers.scm (right):
> > 
> >
>
https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newc...
> > scm/scheme-engravers.scm:121: (define-public (Merge_rests_engraver context)
> > Stupid question: is there a reason Merge_rests_engraver is unregistered?  
> 
> How to do so? With ly:register-translator?

Yes.

> That was my question in the announcement-massage on devel.

Well, the other two engravers are registered right before the new definition. 
Must have missed that message.

[thomasmorley651, 2017-08-28 20:42:33]

register the engraver

[thomasmorley651, 2017-08-29 07:24:45]

improve coding, change docs, regtest to use \consists \...

[dak, 2017-08-29 07:31:20]

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
File Documentation/notation/simultaneous.itely (right):

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
Documentation/notation/simultaneous.itely:10: @c \version "2.19.29"
Version warrants changing to the version where the engraver got register.  At
least once you actually make use of the registration...

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
Documentation/notation/simultaneous.itely:933: \consists \Merge_rests_engraver
Uh, what?

You could have written _this_ without registering the engraver.  The point of
registering the engraver is that you can write

    \consists Merge_rests_engraver

or (I think that's the convention we usually use)

    \consists "Merge_rests_engraver"

like with C++-defined engravers.

[thomasmorley651, 2017-08-29 07:39:45]

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
File Documentation/notation/simultaneous.itely (right):

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
Documentation/notation/simultaneous.itely:10: @c \version "2.19.29"
On 2017/08/29 07:31:20, dak wrote:
> Version warrants changing to the version where the engraver got register.  At
> least once you actually make use of the registration...

Not sure to which version I should change it, 2.21.0?

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
Documentation/notation/simultaneous.itely:933: \consists \Merge_rests_engraver
On 2017/08/29 07:31:20, dak wrote:
> Uh, what?
> 
> You could have written _this_ without registering the engraver.  The point of
> registering the engraver is that you can write
> 
>     \consists Merge_rests_engraver
> 
> or (I think that's the convention we usually use)
> 
>     \consists "Merge_rests_engraver"
> 
> like with C++-defined engravers.

Silly me.
Will change, in the reg-test as well

[dak, 2017-08-29 07:55:06]

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
File Documentation/notation/simultaneous.itely (right):

https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
Documentation/notation/simultaneous.itely:10: @c \version "2.19.29"
On 2017/08/29 07:39:45, thomasmorley651 wrote:
> On 2017/08/29 07:31:20, dak wrote:
> > Version warrants changing to the version where the engraver got register. 
At
> > least once you actually make use of the registration...
> 
> Not sure to which version I should change it, 2.21.0?

Excellent question, actually.  I think that the dot merge functionality is too
new and untested for 2.20.0 but the Merge_rest_engraver itself has been around
unregistered for months and that seems like a bad idea for 2.20.  So I'd propose
that you split the registration (and its use in docs and old(!) regtest) into a
separate commit and use version 2.20.0 on that.  I can cherry-pick that into the
stable branch then.

The rest is for 2.21 then.  It's either that or back out (namely revert) all of
the Merge_rest_engraver for 2.20.  But that would necessitate tampering with
both documentation and translation a lot more, so it's not my preferred option.

Holler if you need Git assistance.

[thomasmorley651, 2017-08-29 08:47:37]

splitting into two patches: registering and dot-merging

[thomasmorley651, 2017-08-29 08:51:54]

On 2017/08/29 07:55:06, dak wrote:
>
https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
> File Documentation/notation/simultaneous.itely (right):
> 
>
https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/si...
> Documentation/notation/simultaneous.itely:10: @c \version "2.19.29"
> On 2017/08/29 07:39:45, thomasmorley651 wrote:
> > On 2017/08/29 07:31:20, dak wrote:
> > > Version warrants changing to the version where the engraver got register. 
> At
> > > least once you actually make use of the registration...
> > 
> > Not sure to which version I should change it, 2.21.0?
> 
> Excellent question, actually.  I think that the dot merge functionality is too
> new and untested for 2.20.0 but the Merge_rest_engraver itself has been around
> unregistered for months and that seems like a bad idea for 2.20.  So I'd
propose
> that you split the registration (and its use in docs and old(!) regtest) into
a
> separate commit and use version 2.20.0 on that.  I can cherry-pick that into
the
> stable branch then.
> 
> The rest is for 2.21 then.  It's either that or back out (namely revert) all
of
> the Merge_rest_engraver for 2.20.  But that would necessitate tampering with
> both documentation and translation a lot more, so it's not my preferred
option.
> 
> Holler if you need Git assistance.

I now splitted the patch.
Obviously Rietveld merges the two patches for review, though.
Is it possible to see them separated somewhere?