Re: Revised autobeam settings patch (issue1682049)

[Carl . D . Sorensen]

Thanks for the careful review!

Carl


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

http://codereview.appspot.com/1682049/diff/35001/36004#newcode1055
Documentation/notation/rhythms.itely:1055: predefined default values for
these values can be found in
On 2010/07/11 23:31:50, Neil Puttock wrote:
> for these properties

Done.

http://codereview.appspot.com/1682049/diff/35001/36004#newcode1060
Documentation/notation/rhythms.itely:1060: \score{
On 2010/07/11 23:31:50, Neil Puttock wrote:
> \score {

Done.

http://codereview.appspot.com/1682049/diff/35001/36004#newcode1730
Documentation/notation/rhythms.itely:1730: @funindex beamExceptions
On 2010/07/11 23:31:50, Neil Puttock wrote:
> + beatStructure

Done, + alphabetized the entries.

http://codereview.appspot.com/1682049/diff/35001/36004#newcode1755
Documentation/notation/rhythms.itely:1755: for the beam type, use it to
determine the valid places where
On 2010/07/11 23:31:50, Neil Puttock wrote:
> beam-type ?

I think not, because we're not talking about an argument to a function
call or an entry in an alist here.  We're talking about something that
comes from the music.  But I could be convinced otherwise.

http://codereview.appspot.com/1682049/diff/35001/36004#newcode1824
Documentation/notation/rhythms.itely:1824: @emph{complete} exceptions
list.  That is, every exception that should
On 2010/07/11 23:31:50, Neil Puttock wrote:
> lists

Changed to singular for all.  beamExceptions is now a context property,
and there is only *one* beamException value at any given time.

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

http://codereview.appspot.com/1682049/diff/35001/36009#newcode16
Documentation/snippets/new/conducting-signs,-measure-grouping-signs.ly:16:
the measure.  @code{time} and @code{set-time-signature} both apply
On 2010/07/11 23:31:50, Neil Puttock wrote:
> @code{\time}

Done.

http://codereview.appspot.com/1682049/diff/35001/36009#newcode18
Documentation/snippets/new/conducting-signs,-measure-grouping-signs.ly:18:
@code{beatStructure} or @code{baseUnit} that are set in
On 2010/07/11 23:31:50, Neil Puttock wrote:
> baseMoment

Done.

http://codereview.appspot.com/1682049/diff/35001/36014
File input/regression/auto-beam-beaming-override.ly (right):

http://codereview.appspot.com/1682049/diff/35001/36014#newcode11
input/regression/auto-beam-beaming-override.ly:11: \version "2.13.27"
On 2010/07/11 23:31:50, Neil Puttock wrote:
> 2.13.28

Done.

http://codereview.appspot.com/1682049/diff/35001/36016
File input/regression/beaming-ternary-metrum.ly (right):

http://codereview.appspot.com/1682049/diff/35001/36016#newcode2
input/regression/beaming-ternary-metrum.ly:2: \version "2.13.27"
On 2010/07/11 23:31:50, Neil Puttock wrote:
> 2.13.28

Done.

http://codereview.appspot.com/1682049/diff/35001/36017
File input/regression/les-nereides.ly (right):

http://codereview.appspot.com/1682049/diff/35001/36017#newcode1
input/regression/les-nereides.ly:1: \version "2.13.27"
On 2010/07/11 23:31:50, Neil Puttock wrote:
> 2.13.28

Done.

http://codereview.appspot.com/1682049/diff/35001/36019
File lily/beam-engraver.cc (right):

http://codereview.appspot.com/1682049/diff/35001/36019#newcode309
lily/beam-engraver.cc:309: "baseMoment ",
On 2010/07/11 23:31:50, Neil Puttock wrote:
> move to top

Done.

http://codereview.appspot.com/1682049/diff/35001/36025
File lily/timing-translator.cc (right):

http://codereview.appspot.com/1682049/diff/35001/36025#newcode62
lily/timing-translator.cc:62: context ()->set_property ("baseMoment",
On 2010/07/11 23:31:50, Neil Puttock wrote:
> add to translator doc (+ others missing)

Done.

http://codereview.appspot.com/1682049/diff/35001/36026
File ly/bagpipe.ly (right):

http://codereview.appspot.com/1682049/diff/35001/36026#newcode12
ly/bagpipe.ly:12: \version "2.13.27"
On 2010/07/11 23:31:50, Neil Puttock wrote:
> 2.13.28

Done.

http://codereview.appspot.com/1682049/diff/35001/36028
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/1682049/diff/35001/36028#newcode21
ly/music-functions-init.ly:21: \version "2.13.27"
On 2010/07/11 23:31:50, Neil Puttock wrote:
> 2.13.28

Done.

http://codereview.appspot.com/1682049/diff/35001/36028#newcode675
ly/music-functions-init.ly:675: (revert-time-signature-setting
time-signature context))
On 2010/07/11 23:31:50, Neil Puttock wrote:
> indent

Done.

http://codereview.appspot.com/1682049/diff/35001/36030
File scm/auto-beam.scm (right):

http://codereview.appspot.com/1682049/diff/35001/36030#newcode62
scm/auto-beam.scm:62: (not (eq? (member moment beat-structure) #f)))
On 2010/07/11 23:31:50, Neil Puttock wrote:
> (pair? (member moment beat-structure))

Oh, OK.  We return at least a single-item list if it's a member.

http://codereview.appspot.com/1682049/diff/35001/36030#newcode127
scm/auto-beam.scm:127: ;; no rule applies, so end at beatLength or
measure end
On 2010/07/11 23:31:50, Neil Puttock wrote:
> indent

Done.

Also, changed the names to reflect exception groupings, and cleaned up
the comments to make them more clear, I hope.

http://codereview.appspot.com/1682049/diff/35001/36030#newcode129
scm/auto-beam.scm:129: ;; end if measure-pos matches a specified ending
moment
On 2010/07/11 23:31:50, Neil Puttock wrote:
> indent

Done.

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

http://codereview.appspot.com/1682049/diff/35001/36037#newcode253
scm/time-signature-settings.scm:253: (define (revert-property-setting
context property setting)
On 2010/07/11 23:31:50, Neil Puttock wrote:
> public?

Maybe I actually want to have neither one public.  I don't really want
users calling these routines.  What do you think?

http://codereview.appspot.com/1682049/diff/35001/36037#newcode270
scm/time-signature-settings.scm:270: time-signature setting . rest)
On 2010/07/11 23:31:50, Neil Puttock wrote:
> this indentation is unmaintainable

Done.

http://codereview.appspot.com/1682049/diff/35001/36037#newcode273
scm/time-signature-settings.scm:273: (context-spec-music
On 2010/07/11 23:31:50, Neil Puttock wrote:
> indent

Done.

Messed this up when I got rid of ly:export...

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