Re: PartCombine: Implement part-combine texts on the first real note (issue3285042)

[reinhold . kainhofer]

Reviewers: Valentin Villenave, Neil Puttock,

Message:
Included all comments/suggestions and pushed to master.


http://codereview.appspot.com/3285042/diff/1/lily/part-combine-engraver.cc
File lily/part-combine-engraver.cc (right):

http://codereview.appspot.com/3285042/diff/1/lily/part-combine-engraver.cc#newcode49
lily/part-combine-engraver.cc:49: bool have_note;
On 2010/11/25 11:21:58, Valentin Villenave wrote:
> Wouldn't "has_note" be more informative? Or perhaps I misunderstood
the logic
> here.

I've renamed that variable to note_found_ to make it clearer. (I was
thinking of "we have a tie at that moment"

http://codereview.appspot.com/3285042/diff/1/lily/part-combine-engraver.cc#newcode91
lily/part-combine-engraver.cc:91: text_ = make_item
("CombineTextScript", /*ev?(ev->self_scm ()):*/SCM_EOL);
On 2010/11/25 17:45:21, Neil Puttock wrote:
> What's up with the event-cause setting?

Oops, commented out that part to track down a crash and missed it later
on. ev will not be 0 at that line, since lilypond will have already
crashed due to the first line, anyway. Plus, create_item is only called
for ev!=0.

http://codereview.appspot.com/3285042/diff/1/lily/part-combine-engraver.cc#newcode102
lily/part-combine-engraver.cc:102: if (have_note || !to_boolean
(get_property ("partCombineTextsOnNote")))
On 2010/11/25 17:45:21, Neil Puttock wrote:
> I don't mind either way, but the plural's in keeping with
printPartCombineTexts.

Exactly, I wanted to stay consistent with that property. of course, we
can always rename both properties...

Other properties using plural are e.g. alignBassFigureAccidentals,
extendersOverRests, figuredBassCenterContinuations, harmonicDots,
includeGraceNotes, etc.

Description:
PartCombine: Implement part-combine texts on the first real note

-) If the context property partCombineTextsOnNote is set, the
part-combine
   texts are not printed immediately if there is a rest, but on the next
         following note. If the voice contains a note, they are printed
immediately
         This is needed if one voice has a full measure rest and the other e.g.
         r2 r4 c4.
-) The event triggering the part-combine text is cached in that case
   and the Part_combine_engraver now also listens to note events. The
         texts are then created at the first moment when a note is encountered.

Please review this at http://codereview.appspot.com/3285042/

Affected files:
  A input/regression/part-combine-text-wait.ly
  M lily/part-combine-engraver.cc
  M ly/engraver-init.ly
  M scm/define-context-properties.scm