Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(432)

Issue 88155: New format for autobeaming rules (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by Carl
Modified:
9 years, 6 months ago
Reviewers:
joeneeman, Neil Puttock
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

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)

Patch Set 1 #

Patch Set 2 : Eliminate trailing whitespace, fix lsr snippets #

Patch Set 3 : Patch set 2 with diffs from origin #

Total comments: 14

Patch Set 4 : Revisions following Neil's comments #

Patch Set 5 : Potential release candidate #

Total comments: 8

Patch Set 6 : Fix displayMusic methods, respond to comments #

Patch Set 7 : Fixed indentation, eliminated commented code #

Total comments: 1

Patch Set 8 : Fix broken regression, update docs #

Total comments: 4

Patch Set 9 : Respond to Joe's comments; add setBeatGrouping #

Total comments: 24

Patch Set 10 : Latest changes in response to Joe and Neil #

Patch Set 11 : Release candidate #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1906 lines, -2049 lines) Patch
M Documentation/de/user/rhythms.itely View 5 6 7 8 9 10 55 chunks +212 lines, -408 lines 0 comments Download
M Documentation/es/user/rhythms.itely View 5 6 7 8 9 10 3 chunks +57 lines, -257 lines 0 comments Download
M Documentation/fr/user/rhythms.itely View 5 6 7 8 9 10 55 chunks +152 lines, -354 lines 0 comments Download
M Documentation/topdocs/NEWS.tely View 2 chunks +18 lines, -3 lines 0 comments Download
M Documentation/user/music-glossary.tely View 1 chunk +0 lines, -3 lines 0 comments Download
M Documentation/user/rhythms.itely View 1 2 3 4 5 6 7 8 chunks +106 lines, -237 lines 0 comments Download
M input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly View 1 2 3 4 5 6 7 8 9 10 4 chunks +35 lines, -9 lines 0 comments Download
M input/lsr/beam-endings-in-score-context.ly View 1 2 3 4 5 6 7 8 9 10 4 chunks +27 lines, -13 lines 0 comments Download
M input/lsr/beam-grouping-in-7-8-time.ly View 1 2 3 4 5 6 7 8 9 10 3 chunks +21 lines, -9 lines 0 comments Download
M input/lsr/chordchanges-for-fretboards.ly View 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -3 lines 0 comments Download
M input/lsr/compound-time-signatures.ly View 1 2 3 4 5 6 7 8 9 10 6 chunks +13 lines, -10 lines 0 comments Download
M input/lsr/conducting-signs,-measure-grouping-signs.ly View 1 2 3 4 5 6 7 8 9 10 3 chunks +28 lines, -21 lines 0 comments Download
M input/lsr/grouping-beats.ly View 1 2 3 4 5 6 7 8 9 10 3 chunks +13 lines, -10 lines 0 comments Download
M input/lsr/making-slurs-with-complex-dash-structure.ly View 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M input/lsr/non-default-tuplet-numbers.ly View 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M input/lsr/non-traditional-key-signatures.ly View 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M input/lsr/reverting-default-beam-endings.ly View 1 2 3 4 5 6 7 8 9 10 3 chunks +23 lines, -17 lines 0 comments Download
M input/lsr/specifying-context-with-beatgrouping.ly View 10 3 chunks +41 lines, -22 lines 0 comments Download
M input/lsr/using-beatlength-and-beatgrouping.ly View 10 3 chunks +43 lines, -38 lines 0 comments Download
M input/manual/fretted-headword.ly View 1 chunk +3 lines, -2 lines 0 comments Download
M input/mutopia/claop.py View 1 chunk +1 line, -1 line 0 comments Download
A input/new/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly View 1 chunk +80 lines, -0 lines 0 comments Download
A input/new/beam-endings-in-score-context.ly View 1 chunk +50 lines, -0 lines 0 comments Download
A input/new/beam-grouping-in-7-8-time.ly View 1 chunk +24 lines, -0 lines 0 comments Download
A input/new/compound-time-signatures.ly View 1 chunk +35 lines, -0 lines 0 comments Download
A input/new/conducting-signs,-measure-grouping-signs.ly View 1 3 4 5 1 chunk +43 lines, -0 lines 0 comments Download
A input/new/grouping-beats.ly View 1 chunk +23 lines, -0 lines 0 comments Download
A input/new/reverting-default-beam-endings.ly View 1 chunk +27 lines, -0 lines 0 comments Download
A input/new/specifying-context-with-beatgrouping.ly View 10 1 chunk +48 lines, -0 lines 0 comments Download
A input/new/using-beatlength-and-beatgrouping.ly View 10 1 chunk +54 lines, -0 lines 0 comments Download
M input/regression/auto-beam-beat-grouping.ly View 1 chunk +0 lines, -30 lines 0 comments Download
M input/regression/beam-beat-grouping.ly View 1 chunk +5 lines, -4 lines 0 comments Download
M lily/auto-beam-engraver.cc View 1 chunk +1 line, -1 line 0 comments Download
A lily/beam-setting-scheme.cc View 1 chunk +69 lines, -0 lines 0 comments Download
M lily/beaming-pattern.cc View 9 4 chunks +5 lines, -4 lines 0 comments Download
A lily/include/beam-settings.hh View 1 chunk +16 lines, -0 lines 0 comments Download
M lily/measure-grouping-engraver.cc View 1 3 4 5 6 7 8 9 4 chunks +24 lines, -12 lines 0 comments Download
M ly/bagpipe.ly View 1 chunk +8 lines, -12 lines 0 comments Download
M ly/engraver-init.ly View 1 2 34 chunks +65 lines, -65 lines 0 comments Download
M ly/music-functions-init.ly View 1 3 4 5 6 7 8 9 3 chunks +56 lines, -0 lines 0 comments Download
M python/convertrules.py View 1 3 4 5 6 7 8 9 16 chunks +43 lines, -20 lines 0 comments Download
M scm/auto-beam.scm View 1 2 3 4 5 6 7 8 2 chunks +56 lines, -331 lines 0 comments Download
A scm/beam-settings.scm View 1 2 3 4 5 6 7 8 9 1 chunk +228 lines, -0 lines 0 comments Download
M scm/c++.scm View 1 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M scm/define-context-properties.scm View 1 3 4 5 6 7 8 9 9 chunks +14 lines, -15 lines 0 comments Download
M scm/define-music-display-methods.scm View 6 7 22 chunks +58 lines, -53 lines 0 comments Download
M scm/lily.scm View 1 chunk +1 line, -0 lines 0 comments Download
M scm/music-functions.scm View 1 2 3 4 5 6 7 8 27 chunks +70 lines, -77 lines 0 comments Download

Messages

Total messages: 7
Neil Puttock
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 ...
9 years, 11 months ago (2009-07-12 17:13:53 UTC) #1
Neil Puttock
Carl, I haven't commenting on them directly, but there are quite a few indentation errors ...
9 years, 11 months ago (2009-07-14 21:57:40 UTC) #2
Neil Puttock
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 ...
9 years, 11 months ago (2009-07-15 21:45:37 UTC) #3
joeneeman
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 ...
9 years, 11 months ago (2009-07-21 18:24:35 UTC) #4
Carl
OK, I've responded to Joe's comments. I've created a new file lily/beam-scheme.cc, and put c++ ...
9 years, 11 months ago (2009-07-22 19:25:06 UTC) #5
joeneeman
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 ...
9 years, 11 months ago (2009-07-22 22:15:24 UTC) #6
Neil Puttock
9 years, 11 months ago (2009-07-22 23:23:04 UTC) #7
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b