https://codereview.appspot.com/321930043/diff/60001/ly/init.ly File ly/init.ly (right):
https://codereview.appspot.com/321930043/diff/60001/ly/init.ly#newcode36 ly/init.ly:36: #(use-modules (scm merge-rests-engraver)) I'm not sure why you are defining a separate module. The usual procedure would be to add your functionality to an existing file in the load list or add your new file to the load list (in scm/lily.scm -- see init-scheme-files-body). The Span_stem_engraver, for example, puts the bulk of its code in scm/music-functions-init.scm. (There is also some code in scm.scheme-engravers.scm -- I'm not sure if you ought to add something there.) https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm File scm/merge-rests-engraver.scm (right): https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode14 scm/merge-rests-engraver.scm:14: (eq? Here (and elsewhere) use eqv? to compare numbers. https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode23 scm/merge-rests-engraver.scm:23: (define-public merge-rests-engraver Here (and below) use the scheme-engraver macro for consistency. As far as I'm aware, all Scheme engravers in the code base use this now. See scm/scheme-engravers.scm or input/regression/scheme-text-spanner.ly for examples. https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode35 scm/merge-rests-engraver.scm:35: (if (eq? 'Rest (assoc-ref (ly:grob-property grob 'meta) 'name)) (See comment about recognizing grobs below.) https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode39 scm/merge-rests-engraver.scm:39: (eq? eqv? https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode44 scm/merge-rests-engraver.scm:44: (eq? (ly:grob-property mmrest 'measure-count) 1)) eqv? https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode70 scm/merge-rests-engraver.scm:70: (let* ((curr-rests '()) let* not needed -- use let https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode81 scm/merge-rests-engraver.scm:81: (if (eq? 'MultiMeasureRest (assoc-ref (ly:grob-property grob 'meta) 'name)) You could use grob::name here. Ordinarily, though, objects are recognized by their interfaces. So, here you should try (if (grob::has-interface grob 'multi-measure-rest-interface) [...] https://codereview.appspot.com/321930043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel