Re: Allows inheritence for slur engravers (issue 7437048)

[address@hidden]

Thanks for the review!

Cheers,
MS

> 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_ ...

Done.

> 
> 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...

Done.

> 
> 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.

Done.

> 
> 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.

Done.

> 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.

Done.  Had to make the check slightly different:

 if ((double_property_name_ != "")
      && to_boolean (get_property (double_property_name_.c_str ())))

as GCC complains that there is no operator && for the check you're suggesting.

> 
> 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.
> 

Done.

https://codereview.appspot.com/7437048/