Re: Fix issue 75: Allow multiple concurrent slurs (issue4643067)

[reinhold . kainhofer]

Thanks, Neil for your excellent code review (again and again, I'm really
fascinated by your abilities!).

I've uploaded a new patch that includes all those suggestions.

Cheers,
Reinhold


http://codereview.appspot.com/4643067/diff/15002/input/regression/phrasing-slur-multiple.ly
File input/regression/phrasing-slur-multiple.ly (right):

http://codereview.appspot.com/4643067/diff/15002/input/regression/phrasing-slur-multiple.ly#newcode12
input/regression/phrasing-slur-multiple.ly:12: altPhSlur = #(ly:export
(make-music 'PhrasingSlurEvent 'span-direction START 'spanner-id "alt"))
On 2011/07/07 19:07:13, Neil Puttock wrote:
> remove ly:export

> (identifiers don't need it; it's only useful for exporting scheme code
directly
> inside a music block)

thanks for pointing that out. Initially, I had it without the ly:export,
but I got some error messages (can't remember which), so the first thing
I tried was adding those ly:export calls (and probably something else to
fix those errors) and I didn't get the errors any more ;-)

Yeah, I'm code a lot by trial-and-error ;-)

http://codereview.appspot.com/4643067/diff/15002/lily/phrasing-slur-engraver.cc
File lily/phrasing-slur-engraver.cc (right):

http://codereview.appspot.com/4643067/diff/15002/lily/phrasing-slur-engraver.cc#newcode52
lily/phrasing-slur-engraver.cc:52: vector<Stream_event *> start_events_;
On 2011/07/07 19:07:13, Neil Puttock wrote:
> why not use
> Drul_array<vector<Stream_event * > > events_;

Because (1) I don't like such nested data structures and (2) it doesn't
simplify the code.

http://codereview.appspot.com/4643067/diff/15002/lily/slur-engraver.cc
File lily/slur-engraver.cc (right):

http://codereview.appspot.com/4643067/diff/15002/lily/slur-engraver.cc#newcode176
lily/slur-engraver.cc:176: for (int j = slurs_.size () - 1 ; j >= 0;
j--)
On 2011/07/07 19:07:13, Neil Puttock wrote:
> for (vsize j = slurs_.size (); j--;)

Ah, nice idea to use the check to decrement j to the proper value for
each loop!

http://codereview.appspot.com/4643067/diff/15002/ly/grace-init.ly
File ly/grace-init.ly (right):

http://codereview.appspot.com/4643067/diff/15002/ly/grace-init.ly#newcode15
ly/grace-init.ly:15: s1*0\graceSlur
On 2011/07/07 19:07:13, Neil Puttock wrote:
> \startGraceSlur ?

I was thinking of \melisma + \melismaEnd ... But you are right, most
other spannes use \start... + \stop...

http://codereview.appspot.com/4643067/