Re: Revised autobeam settings patch (issue1682049)

[n . puttock]

Hi Carl,

This looks really good.

A few comments:

The regtests fail on beam-beat-grouping.ly, since it uses
\setBeatGrouping to set the obsolete property beamSettings.

The display method for \time is broken due to using ApplyContext (this
is the same problem as issue 765 for \ottava, which I'll post my patch
for later); you'll have to rework \time using a synthetic event so the
properties are visible (though this can wait for a separate patch).

Cheers,
Neil


http://codereview.appspot.com/1682049/diff/3035/9004
File Documentation/notation/rhythms.itely (right):

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1052
Documentation/notation/rhythms.itely:1052: In addition to setting the
printed time signature, the @code{time}
\time

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1055
Documentation/notation/rhythms.itely:1055: @code{baseUnit},
@code{beatStructure}, and @code{beamExceptions}.  The
baseMoment

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1069
Documentation/notation/rhythms.itely:1069:
\overrideTimeSignatureSettings
Can you think of a better name which doesn't cause confusion with
\override and \revert (seeing as this naming convention will have to be
changed)?

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1079
Documentation/notation/rhythms.itely:1079: For convenience, the Scheme
function @code{make-setting} is
`make-setting' is a bit vague

why not simply add these settings as extra arguments to
\overrideTimeSignatureSettings?

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1817
Documentation/notation/rhythms.itely:1817: of the beam, e.g. @code{(1 .
16)}.
e.g.,

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1860
Documentation/notation/rhythms.itely:1860: \new Voice  = two {
Voice =

http://codereview.appspot.com/1682049/diff/3035/9004#newcode1895
Documentation/notation/rhythms.itely:1895: % eliminate beamException
that groups beats 1,2 and 3,4
beam exception

1, 2
3, 4

http://codereview.appspot.com/1682049/diff/3035/9005
File Documentation/snippets/new/automatic-beam-subdivisions.ly (right):

http://codereview.appspot.com/1682049/diff/3035/9005#newcode13
Documentation/snippets/new/automatic-beam-subdivisions.ly:13: } % begin
verbatim
remove

http://codereview.appspot.com/1682049/diff/3035/9009
File
Documentation/snippets/new/conducting-signs,-measure-grouping-signs.ly
(right):

http://codereview.appspot.com/1682049/diff/3035/9009#newcode6
Documentation/snippets/new/conducting-signs,-measure-grouping-signs.ly:6:
Beat grouping within a bar is controlled by the context property
within a measure

http://codereview.appspot.com/1682049/diff/3035/9009#newcode29
Documentation/snippets/new/conducting-signs,-measure-grouping-signs.ly:29:
doctitle = "Conducting signs measure grouping signs"
rename file too (and references in selected snippets)

http://codereview.appspot.com/1682049/diff/3035/9012
File Documentation/snippets/new/sub-dividing-beams.ly (right):

http://codereview.appspot.com/1682049/diff/3035/9012#newcode8
Documentation/snippets/new/sub-dividing-beams.ly:8: sub-divided.  That
is, the three (or more) beams stretch unbroken over
subdivided

etc.

http://codereview.appspot.com/1682049/diff/3035/9012#newcode25
Documentation/snippets/new/sub-dividing-beams.ly:25: } % begin verbatim
remove

http://codereview.appspot.com/1682049/diff/3035/9016
File lily/auto-beam-engraver.cc (right):

http://codereview.appspot.com/1682049/diff/3035/9016#newcode545
lily/auto-beam-engraver.cc:545: "beamExceptions "
mixing tabs and spaces

http://codereview.appspot.com/1682049/diff/3035/9019
File lily/beaming-pattern.cc (right):

http://codereview.appspot.com/1682049/diff/3035/9019#newcode298
lily/beaming-pattern.cc:298: Moment (1, 4));
indent

http://codereview.appspot.com/1682049/diff/3035/9027
File python/convertrules.py (right):

http://codereview.appspot.com/1682049/diff/3035/9027#newcode3008
python/convertrules.py:3008: @rule ((2, 13, 24),
beamSettings and beatLength need convert rules

http://codereview.appspot.com/1682049/diff/3035/9030
File scm/define-context-properties.scm (right):

http://codereview.appspot.com/1682049/diff/3035/9030#newcode133
scm/define-context-properties.scm:133: (beamExceptions ,list? "alist of
exceptions to autobeam rules
Alist

http://codereview.appspot.com/1682049/diff/3035/9030#newcode135
scm/define-context-properties.scm:135: (beatStructure ,list? "List of
baseMoments that are combined
@code{baseMoment}s

http://codereview.appspot.com/1682049/diff/3035/9030#newcode444
scm/define-context-properties.scm:444: subdivided at baseMoment
positions by only drawing one beam over the beat.")
@code{baseMoment}

http://codereview.appspot.com/1682049/diff/3035/9030#newcode472
scm/define-context-properties.scm:472: (timeSignatureSettings ,pair? "A
nested alist of settings for
,cheap-list?

http://codereview.appspot.com/1682049/diff/3035/9032
File scm/lily-library.scm (right):

http://codereview.appspot.com/1682049/diff/3035/9032#newcode70
scm/lily-library.scm:70: (if (null? fraction)
indent

http://codereview.appspot.com/1682049/diff/3035/9032#newcode75
scm/lily-library.scm:75: (cons (ly:moment-main-numerator moment)
indent

http://codereview.appspot.com/1682049/diff/3035/9034
File scm/music-functions.scm (right):

http://codereview.appspot.com/1682049/diff/3035/9034#newcode536
scm/music-functions.scm:536: (define (make-time-settings context)
indent

http://codereview.appspot.com/1682049/diff/3035/9034#newcode546
scm/music-functions.scm:546: (beam-exceptions fraction
time-signature-settings))
indent (or move back up to previous line)

http://codereview.appspot.com/1682049/diff/3035/9034#newcode547
scm/music-functions.scm:547: (new-measure-length  (ly:make-moment num
den)))
(new-measure-length (ly:make-moment num den)))

http://codereview.appspot.com/1682049/diff/3035/9035
File scm/time-signature-settings.scm (right):

http://codereview.appspot.com/1682049/diff/3035/9035#newcode72
scm/time-signature-settings.scm:72: '(
indent

http://codereview.appspot.com/1682049/diff/3035/9035#newcode107
scm/time-signature-settings.scm:107: ((beamExceptions . (((end . (((1 .
16) . (4 4 4 4 4 4 4 4)))))))))
((end

http://codereview.appspot.com/1682049/diff/3035/9035#newcode167
scm/time-signature-settings.scm:167: ((baseUnit . (1 . 8))
baseMoment

http://codereview.appspot.com/1682049/diff/3035/9035#newcode173
scm/time-signature-settings.scm:173: ((baseUnit . (1 . 8))
baseMoment

http://codereview.appspot.com/1682049/diff/3035/9035#newcode183
scm/time-signature-settings.scm:183: "Get setting @code{my-symbol} for
@code{time-signature} from
indent

http://codereview.appspot.com/1682049/diff/3035/9035#newcode199
scm/time-signature-settings.scm:199: "Get baseUnit value for
@code{time-signature} from
@code{baseMoment}

http://codereview.appspot.com/1682049/diff/3035/9035#newcode213
scm/time-signature-settings.scm:213: (/ (* (car numerator) (cdr
denominator))
indent

http://codereview.appspot.com/1682049/show