lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: New format for autobeaming rules


From: Carl Sorensen
Subject: Re: New format for autobeaming rules
Date: Sun, 12 Jul 2009 16:34:01 -0600



On 7/12/09 11:13 AM, "address@hidden" <address@hidden> wrote:

> 
> 
> http://codereview.appspot.com/88155/diff/43/1044
> File Documentation/user/rhythms.itely (right):
> 
> http://codereview.appspot.com/88155/diff/43/1044#newcode1662
> Line 1662: of the beam, e.g. @code{#'(1 . 16)}, or @code{#'*} to
> indicate a
> This could be confusing to users unfamiliar with Scheme, since it
> implies the hash and quote are required inside the alist.

Good catch.  This was written in an earlier incarnation, when I didn't have
an alist, and they needed the hash for each argument.  Thanks!
> 
> http://codereview.appspot.com/88155/diff/43/1045
> File
> input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly
> (right):
> 
> http://codereview.appspot.com/88155/diff/43/1045#newcode1
> Line 1: %% Do not edit this file; it is auto-generated from input/new
> If you're keeping the revised rule for 4/4, this file's obsolete (it's
> already been unticked in LSR for docs).
> 
> The changes you've made contradict the original intent of this snippet
> (and the title's incorrect), so it would be better to have a new snippet
> for this.
> 
> Frankly, I'd much prefer returning to the old settings (also for 3/4);
> not only are they more in keeping with LilyPond's declared aims with
> regard to typesetting style, they also produce better looking examples
> in the documentation.

I have no problem with returning to the old settings.  I tried to make this
patch rule-neutral (but I made a mistake in 4 8, which you pointed out
earlier).

IIRC, Trevor Daniels took charge of cleaning up the old (2.12) autobeaming
stuff once beatGrouping started working.

I guess there's no sense in trying to recreate the history; let's just fix
it.  What do you think the default beam grouping for 4 4 and 3 4 should be?

for 4 4: (* . (2 2))  ?

and for 3 4: (* . (3)) ?



> 
> http://codereview.appspot.com/88155/diff/43/1045#newcode53
> Line 53: } % begin verbatim
> There should only be one of these inserted automatically (as above, at
> the closing brace for the header).
> 

Thanks, I'll fix that for all the snippets.

> 
> http://codereview.appspot.com/88155/diff/43/1060
> File lily/measure-grouping-engraver.cc (right):
> 
> http://codereview.appspot.com/88155/diff/43/1060#newcode65
> Line 65: SCM time_signature_fraction =
> get_property("timeSignatureFraction");
> get_property (

Oh, yes, thanks.
> 
> http://codereview.appspot.com/88155/diff/43/1060#newcode69
> Line 69:
> remove blank line

Done.
> 
> http://codereview.appspot.com/88155/diff/43/1060#newcode74
> Line 74: scm_from_locale_string ("end"))),
> ly_symbol2scm ("end")

OK, thanks.  I remember seeing that function elsewhere, but I couldn't find
it when I was coding.

I wish we had a way of telling people where in the source tree the scheme
utility functions are.  Maybe in the CG...?

> 
> http://codereview.appspot.com/88155/diff/43/1060#newcode77
> Line 77: scm_from_locale_string ("*")),
> ly_symbol2scm ("*")

Thanks.  Fixed
> 
> http://codereview.appspot.com/88155/diff/43/1067
> File scm/c++.scm (right):
> 
> http://codereview.appspot.com/88155/diff/43/1067#newcode34
> Line 34: (or (number? x) (symbol? x)))
> unused?

Yep, detritus from an earlier incarnation when '* or (1 . 16) were
stand-alone arguments, rather than part of an alist.

Removed.

Thanks,

Carl





reply via email to

[Prev in Thread] Current Thread [Next in Thread]