Thanks for the rebase!
https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc File lily/slur-engraver.cc (right): https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc#newcode51 lily/slur-engraver.cc:51: event_name_ = "slur-event"; As a matter of style, I'd declare all of those const and initialize them in the initializer list, so Slur_engraver::Slur_engraver () : grob_name_ ("Slur"), event_name_ ... That way it is clear that those values are for the life time of the engraver. It would make even more sense to declare them virtual, but C++ only allows virtual functions, so one would have to make them get_grob_name (), get_event_name_ () etc and that's probably too much of a nuisance even though it would squeeze off a few bytes per instance. https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc#newcode64 lily/slur-engraver.cc:64: context ()->set_property ("slurMelismaBusy", m ? SCM_BOOL_T : SCM_BOOL_F); Well, we have ly_bool2scm. Not a fabulous code saver, but as long as we have it... https://codereview.appspot.com/7437048/diff/1/lily/slur-engraver.cc#newcode70 lily/slur-engraver.cc:70: internal_process_music (); I'd rather make this function the default behavior of Slur_proto_engraver::process_music and make set_melisma a virtual function of Slur_proto_engraver that does nothing by default and is overriden by Slur_engraver::set_melisma. https://codereview.appspot.com/7437048/diff/1/lily/slur-proto-engraver.cc File lily/slur-proto-engraver.cc (right): https://codereview.appspot.com/7437048/diff/1/lily/slur-proto-engraver.cc#newcode119 lily/slur-proto-engraver.cc:119: slurs_[i]->warning (_ (("unterminated "+warning_name_).c_str ())); I'd not use warning_name_ but rather something like object_name_. After all, there is no logical connection to warnings. https://codereview.appspot.com/7437048/diff/1/lily/slur-proto-engraver.cc#newcode138 lily/slur-proto-engraver.cc:138: && make_double_slur_) "doubleSlurs" is not generic but rather specific. I'd use something like if (double_property_name_ && to_boolean (get_property (double_property_name_))) instead, then we can, if desired, have a separate doublePhrasingSlur property at some point of time. https://codereview.appspot.com/7437048/diff/1/lily/slur-proto-engraver.cc#newcode183 lily/slur-proto-engraver.cc:183: slur->programming_error ("slur without a cause"); warning_name_ (or, after my proposal, _object_name_) here. https://codereview.appspot.com/7437048/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
