#4509 Enhancement: automatically engrave lyric extenders

[Anonymous]

Originally created by: *anonymous

Originally created by: simon.al...@mail.de

Actually, this is a content vs. presentation issue. The current approach has lyric extenders ‘hardcoded’ within the lyricmode input, whereas often it depends on layout whether I want an extender printed or not:
– In tight horizontal spacing, we might not need an extender, but when spacing is stretched, it might become necessary. This can come through different (page/line) breaking, parallel contexts present only in some editions (part vs. score), Completion_heads_engraver (mensural without barlines/transcription with barlines).
– Long syllables might not need an extender, where short syllables do.
– Often, all voices share the same text, but have extenders in different places. If extenders need not be given explicitly, the lyricmode input code can be reused much easier.

After all, the extenders don’t add any additional meaning, but only serve to improve legibility in such cases where they do.

This would require:
– Recognising the end of a word by absence of a hyphen.
– Comparing printed length of the melisma notes vs. the syllable, likely after line breaking. After all, extenders will never influence horizontal spacing. They might, however, affect vertical spacing. (unless we chose to omit (or shift) the extender in that case?)
– Personally, I think very short extenders shouldn’t be printed. There should be some kind of threshold.
It’s also one of the usecases where a proper representation of a ‘lyric word’ would be helpful, along with issue 2458.

Possibly related:
issue 4098

Version 2.12 had this listed as a Known issue.

https://codereview.appspot.com/313240043 )

[Google Importer]

Originally posted by: simon.al...@mail.de

(No comment was entered for this change.)

Status: New

[Dan Eble]

Hiding unnecessary extenders (when the text extends at least as far as the extender should) could be tracked as a separate issue. It would be useful whether extender creation is automatic or manual, and this ticket would be that much less daunting to a potential contributor.

[Simon Albrecht]

I’m trying to figure out how these parts would be separated. Do you mean a process like the following:

1. Extender creation
   a. through explicit __
   b. automatically create an extender for any syllable with a melisma (the objective of this issue)
2. Kill unnecessary extenders if they have negative, zero, or very short length (such would I interpret your comment, along with my own remark about a ‘threshold’ above)

[Dan Eble]

Yes, that is what I meant. Looking from the outside (without knowledge of the implementation), it seems that (2) is independent of (1).

[Dan Eble]

After all, extenders will never influence horizontal spacing.

Not so: "If an extender starts at the very end of a system, allow extra space before the barline..." (Behind Bars p.447).

[Simon Albrecht]

Since both of us can only guess about implementations, it’s an audacious assumption that they are independent. As it stands, I think it’s better to have the information in one place.

[Simon Albrecht]

Forwarded from Alexander Kobel, out of the duplicate issue 5018:

Automatic LyricExtenders

As written and communicated by Knut Petersen.
For reference, see the threads
http://lists.gnu.org/archive/html/lilypond-devel/2016-12/msg00223.html
http://lists.gnu.org/archive/html/lilypond-devel/2016-12/msg00121.html
http://lists.gnu.org/archive/html/lilypond-user/2016-12/msg00523.html
http://lists.gnu.org/archive/html/lilypond-user/2016-12/msg00303.html
(sorry, this discussion grew quite long in the meantime).

This is a genuine Request For Comments at the current point in time.

http://codereview.appspot.com/313240043

[Alexander Kobel]

Lyric autoextenders: Silence some warnings

http://codereview.appspot.com/313240043

[Anonymous]

Passes make, make check and a full make doc.

VIsual Reg diffs attached

Also note that I am seeing the following additons in the reg test output that may or may not be significant - I am not qualified to say:

--snip--

input/regression/display-lily-tests.log     

@@ -1,6 +1,44 @@
 Processing `./display-lily-tests.ly'
 Parsing...
 Renaming input to: `/home/james/lilypond-git/input/regression/display-lily-tests.ly'
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:94:1: warning: Test unequal: BUG.
+in  = \lyrics { a b }
+out = \lyrics { a __ b __ }
+
+
+\test ##[ \lyrics { a b } #]
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:95:1: warning: Test unequal: BUG.
+in  = \lyricmode { a -- b }
+out = \lyricmode { a -- b __ }
+
+
+\test ##[ \lyricmode { a -- b } #]         % HyphenEvent
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:96:1: warning: Test unequal: BUG.
+in  = \lyricmode { a __ b }
+out = \lyricmode { a __ b __ }
+
+
+\test ##[ \lyricmode { a __ b } #]         % ExtenderEvent
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:97:1: warning: Test unequal: BUG.
+in  = \lyricmode { "a " }
+out = \lyricmode { "a " __ }
+
+
+\test ##[ \lyricmode { "a " } #]           % LyricEvent
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:98:1: warning: Test unequal: BUG.
+in  = \lyricsto "foo" { bla bla  }
+out = \lyricsto "foo" { bla __ bla __  }
+
+
+\test ##[ \lyricsto "foo" { bla bla  } #]      % LyricCombineMusic
+/home/james/lilypond-git/input/regression/display-lily-tests.ly:99:1: warning: Test unequal: BUG.
+in  = { { c4 d4 }
+  \addlyrics { bla bla  } }
+out = { { c4 d4 }
+  \addlyrics { bla __ bla __  } }
+
+
+\test ##[ { { c4 d4 }
 Interpreting music...Test unequal: NOT A BUG.
 in  = \relative { c'4 b4 }
 out = { c'4 b4 }

and a couple of tests put out these two additional warnings:

input/regression/typography-demo.log    

@@ -8,6 +8,8 @@
 Finding the ideal number of pages...
 Fitting music on 1 page...
 Drawing systems...
+warning: Not drawing a box with negative dimension, -0.25 by 0.08.
+warning: Not drawing a box with negative dimension, -0.90 by 0.08.
 Writing header field `texidoc' to `./typography-demo.texidoc'...
 Layout output to `./typography-demo.eps'...
 Layout output to `./typography-demo-1.eps'...

and

input/regression/profile-property-access.log    

@@ -8,6 +8,8 @@
 Finding the ideal number of pages...
 Fitting music on 1 page...
 Drawing systems...
+warning: Not drawing a box with negative dimension, -0.25 by 0.08.
+warning: Not drawing a box with negative dimension, -0.90 by 0.08.
 Writing header field `texidoc' to `./profile-property-access.texidoc'...
 Layout output to `./profile-property-access.eps'...
 Layout output to `./profile-property-access-1.eps'...
@@ -17,28 +19,28 @@

[Anonymous]

Second reg test diff.

[Alexander Kobel]

Tell parser to ignore "__" token

http://codereview.appspot.com/313240043

[Anonymous]

From Rietveld:

On 2016/12/26 19:14:00, pkx166h wrote:
On 2016/12/25 21:53:55, akobel wrote:

Bottom line: I withdraw both proposals.

Can you then re-submit a new patch or delete the one(s) that are invalid?

Sorry, I'm a bit drawn up between my position as the Rietveld-proxy of Knut and
my own Alexander-role as a commenter.
I forwarded all patches by Knut, and I withdrew my two comments from #5 and the
one from the mailing list that Knut answered in #6.

The patch set is up to date.

I don't know what has and what has not been tested for sure, as far as I can
tell only patch set 1 has been tested which had a slew of warning messages
during the reg test output that I don't know are expected, desirable or
something else.

W.r.t. the regtest differences, as far as I (Alexander) can tell:

The warnings should be gone with Knut's parser modification in patch set 3.

The image differences are expected. A few short extenders are removed (due to
the different handling of collapse-length vs. the old minimum-length). The exact
setting of default collapse-length could be discussed, but IMHO the setting is
fine.
A few extenders appear that have not been written explicitly before, but that is
expected, too.
I guess for page-spacing-nonstaff-lines-independent.ly, the extender should be
forbidden, to have the intended effect of this regtest shown more pronounced.

However, I propose that we first discuss the patch itself, and settle on syntax
and choice of default parameters (collapse-length, force-length). AFAICS, once
this is done, we will need a pass over the entire documentation and regtests
anyway that removes all __ in lyrics - not for functionality, but to purge
outdated and possibly confusing syntax. That's basically a sed -e 's/ __//g',
but we will have to do a quick review of at least the affected docs to check for
errors.

Also, a convert-ly rule still has to be written that will, e.g., also solve the
difference in the page-spacing-nonstaff-lines-independent.ly regtest.

---

Patch Meister:

To that end I am setting this back to 'needs work', when something (or not) changes then set the call back to new and then we can re-test the 'latest' itteration of whatever is currently, or subsequently put, up there (on Rietveld).

That way we all know where we are.

Thanks

James

[Alexander Kobel]

Version 2017-01-01

http://codereview.appspot.com/313240043

[Alexander Kobel]

Version 2017-01-01

http://codereview.appspot.com/313240043

[Anonymous]

Passes make, make check and a full make doc.

Reg test diffs attached.

I am still getting the same warnings as before for example:

input/regression/typography-demo.log    

@@ -8,6 +8,8 @@
 Finding the ideal number of pages...
 Fitting music on 1 page...
 Drawing systems...
+warning: Not drawing a box with negative dimension, -0.25 by 0.08.
+warning: Not drawing a box with negative dimension, -0.89 by 0.08.
 Writing header field `texidoc' to `./typography-demo.texidoc'...
 Layout output to `./typography-demo.eps'...
 Layout output to `./typography-demo-1.eps'...

and

input/regression/display-lily-tests.log     

@@ -1,6 +1,44 @@
 Processing `./display-lily-tests.ly'
 Parsing...
 Renaming input to: `/home/jlowe/lilypond-git/input/regression/display-lily-tests.ly'
+/home/jlowe/lilypond-git/input/regression/display-lily-tests.ly:94:1: warning: Test unequal: BUG.
+in  = \lyrics { a b }
+out = \lyrics { a __ b __ }
...

[Anonymous]

Reg test diffs

[Alexander Kobel]

Hi all.

The visual diffs are expected and okay.
I suggest to replace \skip2 by "" in input/regression/page-spacing-nonstaff-lines-independent.ly, though.

The stderr diffs are debatable.
For display-lily-tests.ly, the output is expected and could be marked "NOT A BUG", but it isn't pretty. Since there is no need to enter __ manually anymore (and it is ignored by the parser), \displayLilyMusic should not emit it either.
typography-demo.ly points to an actual oversight in the extender handling of the patch set: the broken extenders from the first to second system seem not to be subject to collapse-length. The warning stems from the fact that the second part of broken extender before "Licht!" has no room available. (Note that it is not related to the fact that there is an acciaccatura there.)

On a more or less unrelated note, it seems that we have no way to stop an extender before an acciaccatura or grace note. That's not a new problem, as far as I can see, but might be slighly more annoying with automatic extenders:

\fixed c'' { c4 \acciaccatura { e16 d } c4 4 \appoggiatura { d8 } c4 }
\addlyrics { A B C D }

[Dan Eble]

Since there is no need to enter __ manually anymore (and it is ignored by the parser), \displayLilyMusic should not emit it either.

This reminds me of a question I raised in the code review which went unacknowledged as far as I can tell. Why should the parser ignore the double underscore token? Why is such a token even defined if there is no use for it? Shouldn't it rather be treated as a syntax error in new versions?

[David Kastrup]

For display-lily-tests.ly, the output is expected and could be marked "NOT A BUG", but it isn't pretty. Since there is no need to enter __ manually anymore (and it is ignored by the parser), \displayLilyMusic should not emit it either.

I don't think that this is a problem of \displayLilyMusic: if lyric extenders no longer convey any meaning present in the input, they should not be part of the music expression in the first place. Music expressions are supposed to represent the input. If a LyricEvent should automatically cause the actions associated with a LyricExtender, then the solution is to change the respective action of the engraver for lyric extenders so that it no longer relies on extender events but works on the LyricEvent s directly.