[Top][All Lists]
[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