Menu

#4096 \addlyrics always creates its own Voice context

Verified
nobody
Defect
2014-09-17
2014-09-08
Anonymous
No

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.

Discussion

  • Google Importer

    Google Importer - 2014-09-08

    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

     
  • Google Importer

    Google Importer - 2014-09-08

    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

     
  • Google Importer

    Google Importer - 2014-09-08

    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...

     
  • Google Importer

    Google Importer - 2014-09-08

    Originally posted by: dak@gnu.org

    Ugh

    http://codereview.appspot.com/142720043

    Labels: -Patch-needs_work Patch-new

     
  • Google Importer

    Google Importer - 2014-09-08

    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

     
  • Google Importer

    Google Importer - 2014-09-08

    Originally posted by: dak@gnu.org

    It would appear that *all* of the flagged regtests contain
    \new Staff { ... } \addlyrics

     
  • Google Importer

    Google Importer - 2014-09-08

    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

     
  • Google Importer

    Google Importer - 2014-09-08

    Originally posted by: dak@gnu.org

    Patchy the autobot says: passes tests.

    Labels: -Patch-new Patch-review

     
  • Google Importer

    Google Importer - 2014-09-11

    Originally posted by: pkx1...@gmail.com

    Patch on countdown for September 14th

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2014-09-11

    Originally posted by: pkx1...@gmail.com

    Also, for what it is worth, did a full make doc and it passed

     
  • Google Importer

    Google Importer - 2014-09-13

    Originally posted by: pkx1...@gmail.com

    Patch counted down - please push

    Labels: -Patch-countdown Patch-push

     
  • Google Importer

    Google Importer - 2014-09-14

    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

     
  • Google Importer

    Google Importer - 2014-09-17

    Originally posted by: ColinPKC...@gmail.com

    (No comment was entered for this change.)

    Status: Verified

     
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.