Fixes issue 786, "Extenders in lyrics stop prematurely if a single underscore is found."

Fixes issue 786, "Extenders in lyrics stop prematurely if a single underscore is found."
This commit changes the lyrics engraver to create LyricText objects
even for "empty" lyrics (underscores). This change is necessary because the old behavior
(pre-7531ea6b3 commit) relied on extenders continuing until the presence of another lyric
syllable, which was not always the case. That commit changed the behavior to completize
extenders when no more lyrics were present, which fixed the neverending-extender bug but
introduced the extenders-stopping-prematurely bug.

By adding the "empty" LyricText objects, the extender engraver can now tell the difference
between melismas and the end of a block of lyrics.

[csnyder, 2009-11-09 20:43:44]

This patch fixes 786 (and 800, which I think should be labeled a dupe).

No regression tests were adversely affected, but there could possibly be some
issues with creating "empty" LyricText objects.

[csnyder, 2009-11-10 17:28:19]

Oops - comment for the 2nd patch set should be "Fixed hyphens..."

Not sure what to do about issue 800 - it might be a bug in some other portion of
the code that my earlier patch uncovered.

[csnyder, 2009-11-10 17:58:17]

Third time's a charm, hopefully. Patch Set 3, which has just been uploaded,
fixes both 786 and 800 without adversely affecting any regression tests.

(I spoke too soon about potentially uncovering an underlying bug - all of the
bugs were mine)

[joeneeman, 2009-11-10 20:00:00]

http://codereview.appspot.com/150067/diff/2003/3005
File lily/extender-engraver.cc (right):

http://codereview.appspot.com/150067/diff/2003/3005#newcode83
lily/extender-engraver.cc:83: }
current_lyric_is_skip_ = ly_is_equal (test, scm_from_locale_string (" "));

http://codereview.appspot.com/150067/diff/2003/3005#newcode113
lily/extender-engraver.cc:113: if (!melisma_busy (voice) &&
!current_lyric_is_skip_ && lyric_acknowledged_)
Could you please add a sentence or two explaining why this test is here?

http://codereview.appspot.com/150067/diff/2003/3006
File lily/hyphen-engraver.cc (right):

http://codereview.appspot.com/150067/diff/2003/3006#newcode63
lily/hyphen-engraver.cc:63: 
current_lyric_is_skip_ = ly_is_equal (text, scm_from_locale_string (" "))

http://codereview.appspot.com/150067/diff/2003/3007
File lily/lyric-engraver.cc (right):

http://codereview.appspot.com/150067/diff/2003/3007#newcode67
lily/lyric-engraver.cc:67: text_ = make_item ("LyricText", event_->self_scm ());
Indentation.

Also, please add a comment explaining (at a high level) why we need to pass
empty lyrics to the hyphen and extender engravers.

[csnyder, 2009-11-10 21:57:00]

On 2009/11/10 20:00:00, joeneeman wrote:
> http://codereview.appspot.com/150067/diff/2003/3005#newcode113
> lily/extender-engraver.cc:113: if (!melisma_busy (voice) &&
> !current_lyric_is_skip_ && lyric_acknowledged_)
> Could you please add a sentence or two explaining why this test is here?
> 
> Also, please add a comment explaining (at a high level) why we need to pass
> empty lyrics to the hyphen and extender engravers.

I'll try to explain both of these together. Here are the cases I'm addressing:

1) "La __ _ _ la la" (issue 786)
Extender followed by manual melismata (worked before commit 7531ea6b3, broken
after)

2) Same as above, but with an additional voice with a different rhythm (issue
800)
(worked before 7531ea6b3, broken after)

3) "La -- _ _ la la" (worked both before and after 7531ea6b3, but needed to be
addressed while fixing 786)

4) "La __" with music continuing after the end of the lyrics (broken in some
cases before 7531ea6b3, fixed after)

Pre-7531ea6b3, extenders were only finished at either (a) the next lyrics in the
block or (b) the end of the associated voice. This led to problems when the
voice continued without additional lyrics (a situation I encountered a couple of
times when engraving vocal music with multiple verses and a single refrain,
alternate texts, etc.).

7531ea6b3, my previous commit, added a check in
Extender_engraver::stop_translation_timestep to check if the associated voice
was in a melisma, and to completize the extender if not. This fixed case 4, but
broke 1 and 2. The reason is, the extender-engraver had no way of knowing
whether the lack of additional lyrics was due to reaching the end of the lyrics
block or because there were manual melismata (underscores). By changing the
lyrics-engraver to not kill LyricsText objects caused by manual melismata, the
extender-engraver is now able to tell the difference between the two cases and
react accordingly.

The empty LyricsText objects introduced a problem with hyphens, however, since
the hyphen's right bound would attach to the empty LyricsText object instead of
the next non-empty LyricsText, necessitating some changes to the hyphen-engraver
to accomodate this.

Regarding issue 800 (case 2 above): I'm starting to second-guess my solution to
that. In fact, I might have even completely un-done the fix from my earlier
commit. I think the problem is that melisma_busy only works if the current
timestep coincides with a note in that voice. It seems to me that melisma_busy
should be modified to return true in this case.

[csnyder, 2009-11-11 15:20:29]

I've uploaded a patch that reverts the "fix" for issue 800, as it reintroduced
the neverending-extender bug. I also added some comments in the code as
requested.

[csnyder, 2009-11-12 15:03:58]

The latest patch looks to me to be a proper fix for issue 800.

[joeneeman, 2009-11-12 19:20:52]

lgtm

http://codereview.appspot.com/150067/diff/3018/4006
File lily/extender-engraver.cc (right):

http://codereview.appspot.com/150067/diff/3018/4006#newcode118
lily/extender-engraver.cc:118: Moment *start_mom = column ? unsmob_moment
(column->get_property ("when") ) : 0;
no space between ) )

[Neil Puttock, 2009-11-13 20:25:32]

Hi Chris,

Thanks for working on these issues.

I've applied your patch to master.

Cheers,
Neil