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/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
