https://codereview.appspot.com/7377046/diff/17001/input/regression/scheme-text-spanner.ly File input/regression/scheme-text-spanner.ly (right):
https://codereview.appspot.com/7377046/diff/17001/input/regression/scheme-text-spanner.ly#newcode129 input/regression/scheme-text-spanner.ly:129: side-position-interface::y-aligned-side) I really don't understand why you ask on the developer list about gratuitous prefix changes because of a different implementation language when you propose such changes right afterwards. https://codereview.appspot.com/7377046/diff/17001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): https://codereview.appspot.com/7377046/diff/17001/lily/axis-group-interface.cc#newcode261 lily/axis-group-interface.cc:261: if (!g->is_live ()) Shouldn't you check for liveness before anything else? https://codereview.appspot.com/7377046/diff/17001/lily/grob-property.cc File lily/grob-property.cc (right): https://codereview.appspot.com/7377046/diff/17001/lily/grob-property.cc#newcode345 lily/grob-property.cc:345: if (is_simple_closure (unpure)) This is not related to this patch and review, but can anybody explain why this simple_closure $#!+ can't be replaced by a callable closure created using a smob type made callable via scm_set_smob_apply? Is it because of the delayed arg? Can't we deal with that by using a fluid (in Scheme, a parameter) in evaluate_with_simple_closure? These pseudo "simple" closures really make my head ache, and being able to throw all of them out would be quite a boon. https://codereview.appspot.com/7377046/diff/17001/lily/grob-scheme.cc File lily/grob-scheme.cc (right): https://codereview.appspot.com/7377046/diff/17001/lily/grob-scheme.cc#newcode454 lily/grob-scheme.cc:454: SCM_ASSERT_TYPE (ly_is_procedure (proc) || is_unpure_pure_container (proc), proc, SCM_ARG2, __FUNCTION__, "procedure or unpure pure container"); Possibly worth a named type predicate of its own? https://codereview.appspot.com/7377046/diff/17001/lily/grob.cc File lily/grob.cc (right): https://codereview.appspot.com/7377046/diff/17001/lily/grob.cc#newcode866 lily/grob.cc:866: if (to_boolean (scm_object_property (me->get_property_data ("stencil"), ly_symbol2scm ("ly:stencil?")))) Where is this object property being set? https://codereview.appspot.com/7377046/diff/17001/scm/output-lib.scm File scm/output-lib.scm (right): https://codereview.appspot.com/7377046/diff/17001/scm/output-lib.scm#newcode61 scm/output-lib.scm:61: (define-public (grob::wrap-in-unpure-pure-container fn) After issue 3200 passed, this is just ly:make-unpure-pure-container. Remove, change all its callers. https://codereview.appspot.com/7377046/diff/17001/scm/output-lib.scm#newcode89 scm/output-lib.scm:89: (define-public grob::always-vertical-skylines-from-stencil I don't think that these are worth a definition of their own. Better expand them inline. An unpure-pure container is cheap enough that having separate containers for each use is not really a problem. https://codereview.appspot.com/7377046/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
