Fix Issue 770: Lyrics attached to a voice-derived context are off by 1

Fix Issue 770: Lyrics attached to a voice-derived context are off by 1

The Lyric_combine_music_iterator had an explicit check if the CreateContext
event was really for a context of type "Voice". Unfortunately, that
check fails if the lyrics are supposed to be attached to a CueVoice
context (or some self-defined Voice-derived context!), because
"CueVoice"!="Voice".

Unfortunately, I don't know how to check whether a context "CVoice"
(identified only by a the context type string; we don't have any
Context* object!) is an alias for Voice. If we had the Context* object,
we could use is_alias, but we only have the string "CVoice" and need to
check if a context of that type is an alias for "Voice".

However, I don't think that check is necessary at all. It simply prevents
find_voice from being called in cases when no Voice is generated (and thus
no new context to possibly attach the lyrics to). If that check is removed,
find_voice will also be called for any other context created while
waiting for a voice to attach the lyrics to, but it will not find an
appropriate voice anyway.

This patch fixes issue 770:
http://code.google.com/p/lilypond/issues/detail?id=770

[Carl, 2011-07-02 14:00:29]

LGTM

Carl

[pkx166h, 2011-07-02 23:36:58]

Passes reg tests

james

[Neil Puttock, 2011-07-07 19:21:16]

Hi Reinhold,

This LGTM, but do you know why removing this code doesn't break the regtest
added for issue 221 (lyric-combine-switch-voice.ly)?

Cheers,
Neil

http://codereview.appspot.com/4672041/diff/2001/input/regression/lyric-combin...
File input/regression/lyric-combine-derived-voice.ly (right):

http://codereview.appspot.com/4672041/diff/2001/input/regression/lyric-combin...
input/regression/lyric-combine-derived-voice.ly:12: { \voiceOne r2 }
indent

http://codereview.appspot.com/4672041/diff/2001/input/regression/lyric-combin...
input/regression/lyric-combine-derived-voice.ly:15: }
indent

[reinhold_kainhofer.com, 2011-07-07 23:20:09]

Am Donnerstag, 7. Juli 2011, 21:21:16 schrieben Sie:
> Hi Reinhold,
> 
> This LGTM, but do you know why removing this code doesn't break the
> regtest added for issue 221 (lyric-combine-switch-voice.ly)?

AFAIU, the main change of Han-Wen's patch 
(6a3360c40308285434e06a1de031efb073c015fa) was to listen for new voice 
creation, even when a voice was already assigned. If associatedVoice was 
changed, this allows to assign a new voice.

The check that I removed doesn't change this. It simply calls find_voice in 
some more cases (i.e. when a non-Voice context is created), but if the 
associatedVoice wasn't changed, at worst the original voice is found again or 
no appropriate voice is found at all. In both cases, nothing really changes. 
In the first case, we simply assign the same voice again and in the second 
case, nothing happens at all. Similarly, if the associatedVoice was changed, 
it will either find the new voice (even if some other context was created), or 
not.

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[Colin Campbell, 2011-07-10 01:53:36]

This has had a 48-hour countdown, and should be pushed and closed, please.

Colin