Fix 1214: cueDuring and quoteDuring should also quote voices that create subvoices

Fix 1214: cueDuring and quoteDuring should also quote voices that create subvoices

If a voice was quoted that created a subvoice, add-quotable wrongly selected
only that subvoice for quoting rather than the original voice.

The proper fix is to correctly use the assoc list returned by
recording-group-emulate. We need to hand it a proper unique voice name
and can then extract the events for just this voice, no matter which other
voices are created.

[pkx166h, 2011-07-22 19:52:11]

Make works but fails on reg test check.

---snip--
Processing
`/home/jlowe/lilypond-git/build/out/lybook-testdb/aa/lily-c55a6eaf.ly'
Parsing...
Renaming input to: `/home/jlowe/lilypond-git/input/regression/quote-cyclic.ly'
Interpreting music... 
Interpreting music... 
Interpreting music... ERROR: Value out of range: 0

--snip--

\sourcefilename "/home/jlowe/lilypond-git/input/regression/quote-cyclic.ly"

James

[pkx166h, 2011-07-24 09:57:44]

make passes and reg test check is ok.

[Keith, 2011-07-26 02:17:28]

I like it, although I can still get the error if I put a new voice in the quoted
expression like this:
  quoteMe = \relative c' \new Voice {

http://codereview.appspot.com/4816044/diff/3001/input/regression/quote-during...
File input/regression/quote-during-subvoice.ly (right):

http://codereview.appspot.com/4816044/diff/3001/input/regression/quote-during...
input/regression/quote-during-subvoice.ly:4: texidoc = "@code{\\quoteDuring} and
@code{\\cueDuring} shall properly quote
trailing space makes git complain

http://codereview.appspot.com/4816044/diff/3001/input/regression/quote-during...
input/regression/quote-during-subvoice.ly:12: \new Voice {
You might keep the main voice going in parallel
<< { c4 c }
   \new Voice { g4 g } >>
to ensure that the patch continues to work (as it does now) to quote an
instrument that is itself quoting third. (I've actually wanted to to this, for
taking cues, in a couple situations.)

It is interesting, in a good way, how it works with
  << {c4 c} \\ {g g} >>
divisi parts if both parts have the same rhythm,
but I would not encourage that we try to guarantee nor advertise that.

[Reinhold, 2011-07-28 12:46:17]

On 2011/07/26 02:17:28, Keith wrote:
> I like it, although I can still get the error if I put a new voice in the
quoted
> expression like this:
>   quoteMe = \relative c' \new Voice {

Yeah, but then we have a general problem: Which voice do we want to quote?


>
http://codereview.appspot.com/4816044/diff/3001/input/regression/quote-during...
> input/regression/quote-during-subvoice.ly:12: \new Voice {
> You might keep the main voice going in parallel
> << { c4 c }
>    \new Voice { g4 g } >>
> to ensure that the patch continues to work (as it does now) to quote an
> instrument that is itself quoting third. (I've actually wanted to to this, for
> taking cues, in a couple situations.)

Yes, good idea, too.

> It is interesting, in a good way, how it works with
>   << {c4 c} \\ {g g} >>
> divisi parts if both parts have the same rhythm,
> but I would not encourage that we try to guarantee nor advertise that.

The << \\ >> construct uses two new voices, so none of the notes will be quoted.
Cheers,
Reinhold

[Keith, 2011-07-29 04:39:27]

On Thu, 28 Jul 2011 05:46:29 -0700, <reinhold.kainhofer@gmail.com> wrote:

> Message:
> On 2011/07/26 02:17:28, Keith wrote:
>> I like it, although I can still get the error if I put a new voice in
> the quoted
>> expression like this:
>>    quoteMe = \relative c' \new Voice {
>
> Yeah, but then we have a general problem: Which voice do we want to
> quote?
>
I would not expect LilyPond to divine the answer to that question,
but it would be nice to avoid exiting the program with
  ERROR: Value out of range: 0


>> It is interesting, in a good way, how it works with
>>    << {c4 c} \\ {g g} >>
>> divisi parts if both parts have the same rhythm,
>> but I would not encourage that we try to guarantee nor advertise that.
>
> The << \\ >> construct uses two new voices, so none of the notes will be
> quoted.
>

But somehow your patch quotes them both, showing as a << c g >> in the cue
voice.

[reinhold_kainhofer.com, 2011-07-29 09:09:35]

Am Freitag 29 Juli 2011, 06:38:49 schrieb Keith OHara:
> On Thu, 28 Jul 2011 05:46:29 -0700, <reinhold.kainhofer@gmail.com> wrote:
> > Message:
> > 
> > On 2011/07/26 02:17:28, Keith wrote:
> >> I like it, although I can still get the error if I put a new voice in
> >>the quoted expression like this:
> >>    quoteMe = \relative c' \new Voice {
> > 
> > Yeah, but then we have a general problem: Which voice do we want to
> > quote?
> 
> I would not expect LilyPond to divine the answer to that question,
> but it would be nice to avoid exiting the program with
>   ERROR: Value out of range: 0

That's true. I will change this to add a check and print out a warning, 
instead.

> >> It is interesting, in a good way, how it works with
> >> 
> >>    << {c4 c} \\ {g g} >>
> >> 
> >> divisi parts if both parts have the same rhythm,
> >> but I would not encourage that we try to guarantee nor advertise that.
> > 
> > The << \\ >> construct uses two new voices, so none of the notes will be
> > quoted.
> 
> But somehow your patch quotes them both, showing as a << c g >> in the cue
> voice.

Ah, interesting, I'll have to take a closer look at how recording-group-
emulate works. One problem might appear if the two expressions do not have the 
same rhythm...

I'll add a regtest and investigat a bit more.

Cheers,
Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, Vienna University of Technology, Austria
email: reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at/
 * Edition Kainhofer Music Publishing, http://www.edition-kainhofer.com/
 * LilyPond music typesetting software, http://www.lilypond.org/

[Reinhold, 2011-08-10 14:08:50]

On 2011/07/29 04:39:27, Keith wrote:
> On Thu, 28 Jul 2011 05:46:29 -0700, <mailto:reinhold.kainhofer@gmail.com>
wrote:
> 
> > Message:
> > On 2011/07/26 02:17:28, Keith wrote:
> >> I like it, although I can still get the error if I put a new voice in
> > the quoted
> >> expression like this:
> >>    quoteMe = \relative c' \new Voice {
> >
> > Yeah, but then we have a general problem: Which voice do we want to
> > quote?
> >
> I would not expect LilyPond to divine the answer to that question,
> but it would be nice to avoid exiting the program with
>   ERROR: Value out of range: 0

I have now fixed this by (1) always using the first subexpression that really
has some contents (in your example, the contents of the \new Voice) and (2)
printing a warning in the output (with the exact location of the addQuote call).

[Neil Puttock, 2011-08-10 17:28:48]

LGTM.

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
File input/regression/quote-during-subvoice.ly (right):

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
input/regression/quote-during-subvoice.ly:1: \version "2.15.6"
2.15.9

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
input/regression/quote-during-subvoice.ly:18: \addQuote quoteMe \quoteMe
"quoteMe"

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
input/regression/quote-during-subvoice.ly:26: \addQuote quoteMeI \quoteMeI
"quoteMeI"

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
input/regression/quote-during-subvoice.ly:34: \addQuote quoteMeII \quoteMeII
"quoteMeII"

http://codereview.appspot.com/4816044/diff/15001/input/regression/quote-durin...
input/regression/quote-during-subvoice.ly:42: \addQuote quoteMeIII \quoteMeIII
"quoteMeIII"

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm
File scm/part-combiner.scm (right):

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:574: (voicename (get-next-unique-voice-name))
indent

(most of the following lines are also out by at least a space)

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:578: (ly:parser-lookup parser 'partCombineListener))))
indent

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:582: ;; If the context-specced quoted music does not
contain anything, try to
indent

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:586: (let find-non-empty ((current-tail (member raw-voice
context-list)))
indent

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:589: ((null? current-tail) #f)
indent

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:592: (set! quote-contents (cdar current-tail)))
indent

http://codereview.appspot.com/4816044/diff/15001/scm/part-combiner.scm#newcod...
scm/part-combiner.scm:597: (ly:music-message mus (ly:format (_ "quoted music
`~a' is empty") name) ))))
name)))))

[Reinhold, 2011-08-13 10:39:51]

Pushed with commit 1b11dc0cb1bec0e353773aac6b8afba9030f88c4