Re: Adds glissando stems to Lilypond. (issue4661061)

[hanwenn]

can you show png examples of what you're trying to do?

I think this patch has serious issues.


http://codereview.appspot.com/4661061/diff/13/lily/music-sequence.cc
File lily/music-sequence.cc (right):

http://codereview.appspot.com/4661061/diff/13/lily/music-sequence.cc#newcode118
lily/music-sequence.cc:118: continue;
why can't you do this with an override for the to-relative function
property?

http://codereview.appspot.com/4661061/diff/13/lily/note-spacing.cc
File lily/note-spacing.cc (right):

http://codereview.appspot.com/4661061/diff/13/lily/note-spacing.cc#newcode53
lily/note-spacing.cc:53: if (to_boolean (stem->get_property
("glissando-stem")))
if (a&&b) ..

http://codereview.appspot.com/4661061/diff/13/lily/note-spacing.cc#newcode258
lily/note-spacing.cc:258: if (!stem || !stem->is_live () ||
Stem::is_invisible (stem) || to_boolean (stem->get_property
("glissando-stem")))
didn't we have a gop or something prohibiting tabs?

http://codereview.appspot.com/4661061/diff/13/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/4661061/diff/13/lily/stem.cc#newcode1082
lily/stem.cc:1082: Stem::after_line_breaking (SCM smob)
can you rethink your functionality to not need this callback?

we should try to kill the {after/before}_linebreaking functions as they
dont play well with the  callback on get_property() mechanism

http://codereview.appspot.com/4661061/diff/13/lily/stem.cc#newcode1116
lily/stem.cc:1116: Spanner *gls = dynamic_cast<Spanner *>
(glissandi[i]);
to_spanner() ?

http://codereview.appspot.com/4661061/diff/13/lily/stem.cc#newcode1118
lily/stem.cc:1118: Spanner *orig = gls->is_broken () ? gls :
dynamic_cast<Spanner *> (gls->original ());
hosed if gls == NULL

http://codereview.appspot.com/4661061/diff/13/lily/stem.cc#newcode1133
lily/stem.cc:1133:
huh?

What is this needed for?  The break processing automatically substitutes
spanners to only point to the ones in the same system.

http://codereview.appspot.com/4661061/