LGTM.
http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly File input/regression/part-combine-force.ly (right): http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly#newcode4 input/regression/part-combine-force.ly:4: \partcombineApart and \partcombineApartOnce are internally implemented as @code{ } http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly#newcode11 input/regression/part-combine-force.ly:11: \version "2.13.29" "2.13.34" http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly#newcode13 input/regression/part-combine-force.ly:13: mI = \relative c' { test \partcombineUnisono[Once] too? http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly#newcode14 input/regression/part-combine-force.ly:14: \partcombineApartOnce e4 e \partcombineApartOnce c2 | fix indentation http://codereview.appspot.com/1698054/diff/2001/input/regression/part-combine-force.ly#newcode21 input/regression/part-combine-force.ly:21: c \partcombineChords c | If this (conflicting) setting is deliberate, should mention error message expected and add (ly:set-option 'warning-as-error #f) http://codereview.appspot.com/1698054/diff/2001/scm/define-context-properties.scm File scm/define-context-properties.scm (right): http://codereview.appspot.com/1698054/diff/2001/scm/define-context-properties.scm#newcode365 scm/define-context-properties.scm:365: combiner. Possible values are #'apart, #'chords, #'unisono, @code{#'apart} etc. http://codereview.appspot.com/1698054/diff/2001/scm/define-context-properties.scm#newcode367 scm/define-context-properties.scm:367: This property is handled entirely different (in scm/part-combiner.scm) Is this comment necessary here? http://codereview.appspot.com/1698054/diff/2001/scm/define-context-properties.scm#newcode368 scm/define-context-properties.scm:368: than all other context properties!") If this message is staying, change the exclamation mark to a full stop. http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm File scm/part-combiner.scm (right): http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode269 scm/part-combiner.scm:269: ;; Go through all moments recursively and check if the events of that The indentation throughout this new code is poor. http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode278 scm/part-combiner.scm:278: (define (f? x) needs a more descriptive name http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode281 scm/part-combiner.scm:281: (equal? (ly:event-property x 'class) 'UnsetProperty)) (or (ly:in-event-class? x 'SetProperty) (ly:in-event-class? x 'UnsetProperty)) http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode291 scm/part-combiner.scm:291: (if (equal? (ly:event-property x 'class) 'UnsetProperty) (ly:in-event-class? x 'UnsetProperty) http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode296 scm/part-combiner.scm:296: (let* ((res (car (map new-val (reverse evs))))) let http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode303 scm/part-combiner.scm:303: (let* ((val1 (car force1)) let http://codereview.appspot.com/1698054/diff/2001/scm/part-combiner.scm#newcode308 scm/part-combiner.scm:308: ((equal? val1 val2) val1) ;; both voices have same value => easy These comments make the lines too long; I think they'd be better off above each line. (same below in body of `analyse-forced-combine') http://codereview.appspot.com/1698054/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
