Originally created by: *anonymous
Originally created by: dak@gnu.org
Originally owned by: dak@gnu.org
\addlyrics never has been able to make use of an existing Voice context, always creating its own one.
That precludes use of context definitions like
\new Voice \with { ... } { ... } \addlyrics { ... }
This has never worked, and the respective code in scm/ly-syntax-constructors.scm is faulty to the degree that it definitely never found anything (the resulting action would have triggered an error anyway). In addition, the code would only have worked with a _named_ context, namely \new Voice = "whatever" ...
Fixing this is only part of the problem: the grammar is written in a manner where
\new Voice { ... } \addlyrics { ... }
would be factored as
\new Voice { { ... } \addlyrics { ... } }
again precluding \addlyrics from working on the existing Voice.
Originally posted by: dak@gnu.org
Issue 4096: \addlyrics always creates its own Voice context
Contains commits:
Give \addlyrics the power of christening the associated context
With
\new Voice \with { \stemDown } { c1 } \addlyrics { Oh }
as opposed to
\new Voice = "upper" \with { \stemDown } { c1 } \addlyrics { Oh }
\addlyrics created its own context (consequently not using \stemDown)
since it had no context name for the Lyrics context to attach to. In
this case (namely being preceded by an unnamed new Voice context), it
will now give a name to the context itself.
Fix bugs preventing \addlyrics from finding an existing context to attach to
Refactor grammar to give \addlyrics a good target to attach to
So far,
\new Staff \new Voice { c1 } \addlyrics { Oh } \addlyrics { Ah }
was factored as
\new Staff \new Voice { { c1 } \addlyrics { Oh } \addlyrics { Ah } }
which resulted in something like
\new Staff \new Voice
<< \context Voice = "uniqueContext0" { c1 }
\new Lyrics \lyricsto "uniqueContext0" { Oh }
\new Lyrics \lyricsto "uniqueContext0" { Ah }
>>
This made it impossible to work with the likes of
\new Staff \new Voice \with { \stemUp } { c1 } \addlyrics ...
since the voice the lyrics attached to was always a new voice.
Fixing the grouping in the grammar is a first required step for getting
this under control.
Refactor grammar, folding re_rhythmed_music into composite_music
Refactor grammar, folding complex_music into composite_music
http://codereview.appspot.com/142720043
Labels: Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: Fails make check. Well, it would have been nice...
Labels: -Patch-new Patch-needs_work
Originally posted by: dak@gnu.org
Example:
dak@lola:/usr/local/tmp/lilypond$ out/bin/lilypond input/regression/system-start-bracket.ly
GNU LilyPond 2.19.14
Processing `input/regression/system-start-bracket.ly'
Parsing...
input/regression/system-start-bracket.ly:15:18: error: syntax error, unexpected NOTENAME_PITCH
\new Staff
d
input/regression/system-start-bracket.ly:16:18: error: syntax error, unexpected NOTENAME_PITCH
\new Staff
e
input/regression/system-start-bracket.ly:11:1: error: errors found, ignoring music expression
{
fatal error: failed files: "input/regression/system-start-bracket.ly"
Back to the old drawing board...
Originally posted by: dak@gnu.org
Ugh
http://codereview.appspot.com/142720043
Labels: -Patch-needs_work Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: Quite a number of changed regtests with spurious systems, accompanied by
error messages of the kind
Parsing...
Renaming input to: `/tmp/lilypond-autobuild/input/regression/lyric-hyphen-grace.ly'
Interpreting music...
+/tmp/lilypond-autobuild/input/regression/lyric-hyphen-grace.ly:15:14: warning: cannot find Voice `uniqueContext16'
+
+ \addlyrics
+ {
+/tmp/lilypond-autobuild/input/regression/lyric-hyphen-grace.ly:26:14: warning: cannot find Voice `uniqueContext17'
+
+ \addlyrics
+ {
Preprocessing graphical objects...
Calculating line breaks...
Drawing systems...
Labels: -Patch-new Patch-needs_work
Originally posted by: dak@gnu.org
It would appear that *all* of the flagged regtests contain
\new Staff { ... } \addlyrics
Originally posted by: dak@gnu.org
\new Staff ... \addlyrics -> \new Staff \new Voice ... \addlyrics
http://codereview.appspot.com/142720043
Labels: -Patch-needs_work Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: passes tests.
Labels: -Patch-new Patch-review
Originally posted by: pkx1...@gmail.com
Patch on countdown for September 14th
Labels: -Patch-review Patch-countdown
Originally posted by: pkx1...@gmail.com
Also, for what it is worth, did a full make doc and it passed
Originally posted by: pkx1...@gmail.com
Patch counted down - please push
Labels: -Patch-countdown Patch-push
Originally posted by: dak@gnu.org
Pushed to staging as
* commit [r5b2267a97cfb085c4492e2954c0b9eec36fc11f8]
|\ Merge: 92f1830 5937be9
| | Author: David Kastrup <dak@gnu.org>
| | Date: Sun Sep 14 07:46:29 2014 +0200
| |
| | Merge branch 'issue4096' into HEAD
| |
| * commit [r5937be99ecf9872f5dad2bc689099179400b70ff]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Sep 8 15:32:47 2014 +0200
| |
| | Add regtest for issue 4096, \addlyrics using existing contexts
| |
| * commit [r21af7576f26f9b79bc7a4dfcfa041b3b5ced47e7]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Sep 8 17:24:37 2014 +0200
| |
| | Convert \new Staff ... \addlyrics into \new Staff \new Voice ... \addlyrics
| |
| * commit [reda1a9e297d93e074d10e86dfdeae1226f86b4cd]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Sep 8 11:28:14 2014 +0200
| |
| | Give \addlyrics the power of christening the associated context
| |
| | With
| |
| | \new Voice \with { \stemDown } { c1 } \addlyrics { Oh }
| |
| | as opposed to
| |
| | \new Voice = "upper" \with { \stemDown } { c1 } \addlyrics { Oh }
| |
| | \addlyrics created its own context (consequently not using \stemDown)
| | since it had no context name for the Lyrics context to attach to. In
| | this case (namely being preceded by an unnamed new Voice context), it
| | will now give a name to the context itself.
| |
| * commit [r32f47bdec7c2008ebff80d68463d022946db5df2]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Sep 8 11:15:19 2014 +0200
| |
| | Fix bugs preventing \addlyrics from finding an existing context to attach to
| |
| * commit [r4015468c822eae4e9421ca6640b0ecf6e75e8d7d]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Sep 8 11:01:56 2014 +0200
| |
| | Refactor grammar to give \addlyrics a good target to attach to
| |
| | So far,
| |
| | \new Staff \new Voice { c1 } \addlyrics { Oh } \addlyrics { Ah }
| |
| | was factored as
| |
| | \new Staff \new Voice { { c1 } \addlyrics { Oh } \addlyrics { Ah } }
| |
| | which resulted in something like
| |
| | \new Staff \new Voice
| | << \context Voice = "uniqueContext0" { c1 }
| | \new Lyrics \lyricsto "uniqueContext0" { Oh }
| | \new Lyrics \lyricsto "uniqueContext0" { Ah }
| | >>
| |
| | This made it impossible to work with the likes of
| |
| | \new Staff \new Voice \with { \stemUp } { c1 } \addlyrics ...
| |
| | since the voice the lyrics attached to was always a new voice.
| | Fixing the grouping in the grammar is a first required step for getting
| | this under control.
| |
| * commit [r5c2efacf079a69d97da4466dd1179530759bc114]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Aug 4 09:55:17 2014 +0200
| |
| | Refactor grammar, folding re_rhythmed_music into composite_music
| |
| * commit [rb29bd8c074843e26aba40cebc5e226bea69d045a]
|/ Author: David Kastrup <dak@gnu.org>
| Date: Mon Aug 4 09:55:17 2014 +0200
|
| Refactor grammar, folding complex_music into composite_music
Labels: -Patch-push Fixed_2_19_14
Status: Fixed
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Status: Verified