It's not really clear to me where I am supposed to go from here with what I proposed. I lean towards going back to creating my own version of this again since this one contains so much stuff that does not ring a bell with me.
https://codereview.appspot.com/313240043/diff/200001/lily/extender-engraver.cc File lily/extender-engraver.cc (right): https://codereview.appspot.com/313240043/diff/200001/lily/extender-engraver.cc#newcode33 lily/extender-engraver.cc:33: #include "iostream" Why? https://codereview.appspot.com/313240043/diff/200001/lily/extender-engraver.cc#newcode87 lily/extender-engraver.cc:87: ASSIGN_EVENT_ONCE (hy_, ev); If we are not the primary engraver for an item, we should not complain about how often an event is assigned I think. https://codereview.appspot.com/313240043/diff/200001/lily/extender-engraver.cc#newcode109 lily/extender-engraver.cc:109: if (ex_ && !hy_) Why check for hy_ here? If there is an explicit extender event, why would we mess with it? https://codereview.appspot.com/313240043/diff/200001/lily/extender-engraver.cc#newcode112 lily/extender-engraver.cc:112: extender_->set_property ("force-extender", SCM_BOOL_T); I think it might make more sense to have, if at all, a property for implicitly generated extenders so that any extender generated by a user-created engraver sounds as forced. We could probably could just look up the event_cause and see whether it is a lyric-event, but when user-created extenders are expected at all, it might probably be advisable to have a more explicit property one can set to switch between explicit and implicit behavior. https://codereview.appspot.com/313240043/diff/200001/lily/lyric-extender.cc File lily/lyric-extender.cc (right): https://codereview.appspot.com/313240043/diff/200001/lily/lyric-extender.cc#newcode54 lily/lyric-extender.cc:54: bool no_auto_extenders = to_boolean (me->get_property ("no-auto-extenders")); That is awful. Creating extenders with properties like "no-extender" or "no-auto-extenders" in order to decide whether one did not want an extender in the first place is nonsensical. If we want to suppress extenders on anything but visual appearance, we should have context properties for that and not even generate them. Because we might want to be able to create similar behavior in Midi or other backends where no grobs are available. https://codereview.appspot.com/313240043/diff/200001/lily/lyric-extender.cc#newcode63 lily/lyric-extender.cc:63: while (hs && robust_scm2double (heads[hs-1]->get_property Same as previously I see. https://codereview.appspot.com/313240043/diff/200001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/313240043/diff/200001/lily/self-alignment-interface.cc#newcode63 lily/self-alignment-interface.cc:63: ly_symbol2scm ("lyric-syllable-interface")))) Why is this necessary? https://codereview.appspot.com/313240043/diff/200001/lily/self-alignment-interface.cc#newcode146 lily/self-alignment-interface.cc:146: ly_symbol2scm ("lyric-syllable-interface")))) And this? https://codereview.appspot.com/313240043/diff/200001/lily/self-alignment-interface.cc#newcode156 lily/self-alignment-interface.cc:156: ly_symbol2scm ("lyric-syllable-interface")))) And this? Shouldn't we rather try to fix the condition rather than remove the warning? https://codereview.appspot.com/313240043/diff/200001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): https://codereview.appspot.com/313240043/diff/200001/scm/define-grob-properties.scm#newcode188 scm/define-grob-properties.scm:188: (collapse-length ,ly:dimension? "An automatically generated collapse-width maybe? Length is more like a list length? https://codereview.appspot.com/313240043/diff/200001/scm/define-grob-properties.scm#newcode684 scm/define-grob-properties.scm:684: (no-auto-extenders ,boolean? "Inhibits automatic generation of lyric Why do we need this also? There are just too many properties. https://codereview.appspot.com/313240043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
