Hi Reinhold, I really like where you're going with this! As always, I'm not a coder (yet) so this is just my naive point of view.
http://codereview.appspot.com/3334043/diff/1/lily/part-combine-engraver.cc File lily/part-combine-engraver.cc (right): http://codereview.appspot.com/3334043/diff/1/lily/part-combine-engraver.cc#newcode52 lily/part-combine-engraver.cc:52: bool item_to_create_; if it's really a grob, then why are you calling it "item"? http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc File lily/part-combine-iterator.cc (right): http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode69 lily/part-combine-iterator.cc:69: Stream_event *unisono_event_; How about declaring these variables on the same line? (I'm not sure this style is used in Lily's source code, though.) http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode110 lily/part-combine-iterator.cc:110: void broadcast_new_state (Music_iterator *new_active, Stream_event *new_state_event); I'm not familiar with C++ coding style, but that looks like an awfully long line. http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode133 lily/part-combine-iterator.cc:133: mmrest_event_ = 0; Since all these variables are used together a lot, another possibility would be to use an array? (At least that's what I've been reading, but I'm not sure it's worth considering :) http://codereview.appspot.com/3334043/diff/1/scm/define-music-types.scm File scm/define-music-types.scm (right): http://codereview.appspot.com/3334043/diff/1/scm/define-music-types.scm#newcode37 scm/define-music-types.scm:37: (ApartEvent I find this name a bit hard to understand. Is ApartEvent some kind of a voice-agnostic solo-event, that can either be a solo-one- or solo-two-event? (in which case "SoloEvent" could be a better name?) (Now that I'm thinking about it, in the future it could be interesting to have a single solo-event kind, that could take a "thread-number" event-property such as 'one or 'two, which would then be passed to the iterator so that any event could be forced to be regarded as a solo-one- or solo-two-event... Or maybe even make the iterator able to take any arbitrary number of music "threads", not just two... Ok, I'm dreaming here.) http://codereview.appspot.com/3334043/ _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
