Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(488)

Issue 150067: Fixes issue 786, "Extenders in lyrics stop prematurely if a single underscore is found." (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by csnyder
Modified:
9 years, 6 months ago
Reviewers:
joeneeman, Neil Puttock
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

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.

Patch Set 1 #

Patch Set 2 : Fixed extender lines; issue 800 is still unresolved. #

Patch Set 3 : This patch fixes both 786 and 800. #

Total comments: 4

Patch Set 4 : Fixes requested by Joe Neeman #

Patch Set 5 : Issue 800 fix removed #

Patch Set 6 : With comments #

Patch Set 7 : Properly fixes issue 800 #

Total comments: 1

Patch Set 8 : Code formatting #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -18 lines) Patch
M lily/extender-engraver.cc View 1 2 3 4 5 6 7 6 chunks +32 lines, -13 lines 0 comments Download
M lily/hyphen-engraver.cc View 2 3 4 chunks +7 lines, -3 lines 0 comments Download
M lily/lyric-engraver.cc View 1 2 3 4 5 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 9
csnyder
This patch fixes 786 (and 800, which I think should be labeled a dupe). No ...
9 years, 7 months ago (2009-11-09 20:43:44 UTC) #1
csnyder
Oops - comment for the 2nd patch set should be "Fixed hyphens..." Not sure what ...
9 years, 7 months ago (2009-11-10 17:28:19 UTC) #2
csnyder
Third time's a charm, hopefully. Patch Set 3, which has just been uploaded, fixes both ...
9 years, 7 months ago (2009-11-10 17:58:17 UTC) #3
joeneeman
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 ...
9 years, 7 months ago (2009-11-10 20:00:00 UTC) #4
csnyder
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_ ...
9 years, 7 months ago (2009-11-10 21:57:00 UTC) #5
csnyder
I've uploaded a patch that reverts the "fix" for issue 800, as it reintroduced the ...
9 years, 7 months ago (2009-11-11 15:20:29 UTC) #6
csnyder
The latest patch looks to me to be a proper fix for issue 800.
9 years, 7 months ago (2009-11-12 15:03:58 UTC) #7
joeneeman
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") ...
9 years, 7 months ago (2009-11-12 19:20:52 UTC) #8
Neil Puttock
9 years, 7 months ago (2009-11-13 20:25:32 UTC) #9
Hi Chris,

Thanks for working on these issues.

I've applied your patch to master.

Cheers,
Neil
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b