New format for autobeaming rules

New format for autobeaming rules
Change autobeaming rules so they are all set in one place and they
remain if the time signature is changed.

Eliminates override-auto-beam-settings and revert-auto-beam-settings

autoBeamSettings property changed to beamSettings property

beatGrouping property eliminated (default autobeam rule is now
    used for beatGrouping)

[Neil Puttock, 2009-07-12 17:13:53]

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.

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.

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).

http://codereview.appspot.com/88155/diff/43/1046
File input/lsr/beam-endings-in-score-context.ly (right):

http://codereview.appspot.com/88155/diff/43/1046#newcode57
Line 57: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/43/1047
File input/lsr/beam-grouping-in-7-8-time.ly (right):

http://codereview.appspot.com/88155/diff/43/1047#newcode57
Line 57: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/43/1049
File input/lsr/compound-time-signatures.ly (right):

http://codereview.appspot.com/88155/diff/43/1049#newcode75
Line 75: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/43/1050
File input/lsr/conducting-signs,-measure-grouping-signs.ly (right):

http://codereview.appspot.com/88155/diff/43/1050#newcode88
Line 88: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/43/1051
File input/lsr/grouping-beats.ly (right):

http://codereview.appspot.com/88155/diff/43/1051#newcode49
Line 49: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/43/1055
File input/lsr/reverting-default-beam-endings.ly (right):

http://codereview.appspot.com/88155/diff/43/1055#newcode59
Line 59: } % begin verbatim
rogue %begin verbatim

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 (

http://codereview.appspot.com/88155/diff/43/1060#newcode69
Line 69: 
remove blank line

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

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

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?

[Neil Puttock, 2009-07-14 21:57:40]

Carl, I haven't commenting on them directly, but there are quite a few
indentation errors in the .scm files.

http://codereview.appspot.com/88155/diff/95/1147
File Documentation/topdocs/NEWS.tely (right):

http://codereview.appspot.com/88155/diff/95/1147#newcode69
Line 69: section 1.2.4 Beams, for more information.
Is it possible to use @ruser{} here?

http://codereview.appspot.com/88155/diff/95/1149
File Documentation/user/rhythms.itely (right):

http://codereview.appspot.com/88155/diff/95/1149#newcode1743
Line 1743: Beam settings can be reverted to get back to default behavior.  This
This looks like it should be an input/new snippet.

http://codereview.appspot.com/88155/diff/95/1155
File input/lsr/conducting-signs,-measure-grouping-signs.ly (right):

http://codereview.appspot.com/88155/diff/95/1155#newcode77
Line 77: }
missing %begin verbatim

http://codereview.appspot.com/88155/diff/95/1155#newcode88
Line 88: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/95/1170
File lily/measure-grouping-engraver.cc (right):

http://codereview.appspot.com/88155/diff/95/1170#newcode71
Line 71: scm_list_2 (time_signature_fraction,
same line as ly_assoc_get (

http://codereview.appspot.com/88155/diff/95/1170#newcode72
Line 72: ly_symbol2scm ("end")),
indentation

http://codereview.appspot.com/88155/diff/95/1170#newcode75
Line 75: grouping_rules, SCM_EOL);
indentation

http://codereview.appspot.com/88155/diff/95/1177
File scm/c++.scm (right):

http://codereview.appspot.com/88155/diff/95/1177#newcode30
Line 30: (define-public (pair-or-symbol? x)
also unused

[Neil Puttock, 2009-07-15 21:45:37]

http://codereview.appspot.com/88155/diff/2005/3086
File scm/music-functions.scm (right):

http://codereview.appspot.com/88155/diff/2005/3086#newcode519
Line 519: (make-simultaneous-music output)
This breaks all the Festival regression tests which use \time
(song-associated-voice.ly, song-basic.ly, song-breathe.ly, song-melisma.ly,
song-stanzas.ly & song-tempo.ly): 

/usr/local/share/lilypond/2.13.4/scm/song.scm:707:52: In procedure last in
expression (last lst-1):
/usr/local/share/lilypond/2.13.4/scm/song.scm:707:52: Wrong type argument in
position 1 (expecting pair): ()

[joeneeman, 2009-07-21 18:24:35]

very nice!

http://codereview.appspot.com/88155/diff/3092/2039
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3092/2039#newcode699
Line 699: (make-music 'SequentialMusic 'void #t))
I'd feel better if this were finished. At least add FIXME so it can be easily
grepped for.

http://codereview.appspot.com/88155/diff/3092/2047
File scm/music-functions.scm (right):

http://codereview.appspot.com/88155/diff/3092/2047#newcode546
Line 546: (define (make-default-beaming-rule context)
this doesn't seem to be used...

[Carl, 2009-07-22 19:25:06]

OK, I've responded to Joe's comments.

I've created a new file lily/beam-scheme.cc, and put c++ and scheme callable
routines in it.  Then I've used those calls from auto-beam.scm,
measure-grouping-engraver.cc, and beaming-pattern.cc.  I also added a new file
lily/include/beam-grouping.hh to provide those functions to c++.

I've never done this much mucking in the c++ code before, so please make sure
I've done things right.

Thanks,

Carl

http://codereview.appspot.com/88155/diff/3092/2039
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3092/2039#newcode699
Line 699: (make-music 'SequentialMusic 'void #t))
On 2009/07/21 18:24:35, joeneeman wrote:
> I'd feel better if this were finished. At least add FIXME so it can be easily
> grepped for.

OK -- fixed in new patch set.

http://codereview.appspot.com/88155/diff/3092/2047
File scm/music-functions.scm (right):

http://codereview.appspot.com/88155/diff/3092/2047#newcode546
Line 546: (define (make-default-beaming-rule context)
On 2009/07/21 18:24:35, joeneeman wrote:
> this doesn't seem to be used...
Thanks for the catch -- removed

[joeneeman, 2009-07-22 22:15:24]

I think this is pretty much ready to commit

http://codereview.appspot.com/88155/diff/3101/4032
File lily/beam-scheme.cc (right):

http://codereview.appspot.com/88155/diff/3101/4032#newcode2
Line 2: beam-scheme.cc -- Retrieving beam settings
could you call this beam-grouping-scheme.cc or something like that? beam-scheme
sounds like it contains routines for manipulating Beam grobs.

http://codereview.appspot.com/88155/diff/3101/4032#newcode12
Line 12: LY_DEFINE (ly_beam_settings, "ly:beam-settings",
is this function really necessary?

http://codereview.appspot.com/88155/diff/3101/4032#newcode49
Line 49: ly_grouping_rules(settings,time_signature,rule_type),
formatting

http://codereview.appspot.com/88155/diff/3101/4032#newcode61
Line 61: SCM settings = ly_beam_settings(context);
formatting

[Neil Puttock, 2009-07-22 23:23:04]

http://codereview.appspot.com/88155/diff/3101/4027
File input/new/grouping-beats.ly (right):

http://codereview.appspot.com/88155/diff/3101/4027#newcode7
Line 7: Beaming patterns may be altered with the @code{beatGrouping} property:
new texidoc using \overrideBeamSettings

http://codereview.appspot.com/88155/diff/3101/4032
File lily/beam-scheme.cc (right):

http://codereview.appspot.com/88155/diff/3101/4032#newcode10
Line 10: #include "beam-grouping.hh"
swap

http://codereview.appspot.com/88155/diff/3101/4032#newcode26
Line 26: " @var{rule-type} in @var{context}.")
no context arg, doc settings

http://codereview.appspot.com/88155/diff/3101/4032#newcode28
Line 28: LY_ASSERT_TYPE (ly_is_pair, time_signature, 2);
scm_is_pair

http://codereview.appspot.com/88155/diff/3101/4032#newcode30
Line 30: 
type check also for settings?

http://codereview.appspot.com/88155/diff/3101/4032#newcode34
Line 34: ly_assoc_get ((scm_list_2 (time_signature, rule_type)),
excess parens

http://codereview.appspot.com/88155/diff/3101/4032#newcode43
Line 43: "Return grouping for beams of @var{ beam-type} in"
@var{beam-type}

http://codereview.appspot.com/88155/diff/3101/4032#newcode45
Line 45: " @var{rule-type} in @var{context}.")
no context arg, doc settings

http://codereview.appspot.com/88155/diff/3101/4032#newcode46
Line 46: {
type checks?

http://codereview.appspot.com/88155/diff/3101/4032#newcode57
Line 57: {
LY_ASSERT_SMOB (Context, context, 1);

If you don't do this, then unsmob_context () will return NULL if this function
is passed invalid arguments, leading to a null dereference for get_property
("timeSignatureFraction") -> segfault

http://codereview.appspot.com/88155/diff/3101/4033
File lily/beaming-pattern.cc (right):

http://codereview.appspot.com/88155/diff/3101/4033#newcode18
Line 18: #include "beam-grouping.hh"
sort

http://codereview.appspot.com/88155/diff/3101/4034
File lily/include/beam-grouping.hh (right):

http://codereview.appspot.com/88155/diff/3101/4034#newcode8
Line 8: 
To prevent multiple includes:

#ifndef BEAM_GROUPING_HH
#define BEAM_GROUPING_HH

(+ line 14)

http://codereview.appspot.com/88155/diff/3101/4034#newcode14
Line 14: 
#endif // BEAM_GROUPING_HH

http://codereview.appspot.com/88155/diff/3101/4035
File lily/measure-grouping-engraver.cc (right):

http://codereview.appspot.com/88155/diff/3101/4035#newcode14
Line 14: #include "beam-grouping.hh"
sort

http://codereview.appspot.com/88155/diff/3101/4035#newcode66
Line 66: SCM time_signature_fraction = get_property ("timeSignatureFraction");
move to if { } block, then it's only retrieved if required

http://codereview.appspot.com/88155/diff/3101/4038
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3101/4038#newcode20
Line 20: (_i "Define @@var{music} as a quotable music expression named
rogue extra @'s throughout file

http://codereview.appspot.com/88155/diff/3101/4039
File python/convertrules.py (right):

http://codereview.appspot.com/88155/diff/3101/4039#newcode2930
Line 2930: str = re.sub("\\set\w+#\'beatGrouping", "\\setBeatGrouping", str)
won't get here due to search above (and regexp is broken)

http://codereview.appspot.com/88155/diff/3101/4041
File scm/beam-settings.scm (right):

http://codereview.appspot.com/88155/diff/3101/4041#newcode48
Line 48: ;; in 3 4 time:
decided not to restore original setting?

http://codereview.appspot.com/88155/diff/3101/4041#newcode155
Line 155: ;;;; Functions for overriding beam settings
indentation of function bodies

http://codereview.appspot.com/88155/diff/3101/4043
File scm/define-context-properties.scm (right):

http://codereview.appspot.com/88155/diff/3101/4043#newcode126
Line 126: (beatGrouping ,list? "A list of beatgroups, e.g., in 5/8 time
remove