Re: Revised autobeam settings patch -- cleaned up debug comments (issue1667044)

[Trevor Daniels]

Carl.D.Sorensen wrote Tuesday, June 15, 2010 11:27 PM

> Description:
> Revised autobeam settings patch -- cleaned up debug comments
> in code and eliminated the irrelevant changes in
> Documentation/snippets just due to running makelsr.py
>
> Please review this at http://codereview.appspot.com/1667044/show

I've run a few examples through this new code and so
far it all works extremely well.  

One or two of the default beam settings might be 
improved (while you're changing the beaming, that is - I 
think the behaviour below is probably as in the current 
releases, not introduced in this patch).  The most 
important is illustrated by

\relative c' {
 \time 3/4
 % In 3/4 time never beam an odd number of 8th notes or two
 % 8th notes in different beats
 f8 f f f f f
 f16 f f f f f f f f f f f
 f32 f f f f f f f f f f f
 f f f f f f f f f f f f
 f4 r8 f f f     % incorrect!
 f8 f~f f f f    % incorrect!
 d'4. c8 b8. a16  % incorrect!
}

I think here it would be better to break quaver beams
every beat (unless you have a trick up your sleeve
to recognise these incorrect cases).

Also in 4/2 I think it would be better to remove the
32nds beam rule so they are grouped as 16ths are.

It might also be good to add triplet beaming to a few
more common time signatures, as is done for 4/4.

The documentation looks much improved.  I would only
suggest saying how to disable autobeaming so the 
beaming follows beatLength somewhere early, and maybe
including an example showing how to use \set beamSettings
to create settings for more than one beam type (getting
the Scheme brackets right is tricky for everyone who is
unfamiliar with Scheme!)

Congratulations on a fine piece of work!  It's a credit
to your persistence in persevering with this issue!

Trevor