[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Wrong type in part-combiner.scm
From: |
David Kastrup |
Subject: |
Re: Wrong type in part-combiner.scm |
Date: |
Tue, 22 Sep 2020 19:29:49 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Aaron Hill <lilypond@hillvisions.com> writes:
> On 2020-09-22 5:54 am, David Kastrup wrote:
>> The whole code in the find-non-empty loop, apart from this error, is
>> garbage. Here is a bit more code and analysis:
>> (define-public (add-quotable name mus)
>> (let* ((tab (eval 'musicQuotes (current-module)))
>> (voicename (get-next-unique-voice-name))
>> ;; recording-group-emulate returns an assoc list
>> (reversed!), so
>> ;; hand it a proper unique context name and extract that key:
>> (ctx-spec (context-spec-music mus 'Voice voicename))
>> (listener (ly:parser-lookup 'partCombineListener))
>> (context-list (reverse (recording-group-emulate ctx-spec
>> listener)))
>> (raw-voice (assoc voicename context-list))
>> (quote-contents (if (pair? raw-voice) (cdr raw-voice) '())))
>> ;; If the context-specced quoted music does not contain
>> anything, try to
>> ;; use the first child, i.e. the next in context-list after
>> voicename
>> ;; That's the case e.g. for \addQuote "x" \relative c \new Voice
>> {...}
>> (if (null? quote-contents)
>> (let find-non-empty ((current-tail (member raw-voice
>> context-list)))
>> ;; if voice has contents, use them, otherwise check next ctx
>> (cond ((null? current-tail) #f)
>> ((and (pair? (car current-tail))
>> (pair? (cdar current-tail)))
>> (set! quote-contents (cdar current-tail)))
>> (else (find-non-empty (cdr current-tail))))))
>> (if (not (null? quote-contents))
>> (hash-set! tab name (list->vector (reverse! quote-contents
>> '())))
>> (ly:music-warning mus (ly:format (_ "quoted music `~a' is
>> empty") name)))))
>> So raw-voice is an assoc into context-list. This assoc is then
>> searched
>> for in the find-non-empty loop. For each found instance, a test is
>> made. If that test is not successful, the search is repeated for more
>> instances. However, since all found instances are _guaranteed_ to be
>> the same (since assoc is only called once, and then member searches for
>> the same key/value pair rather than just the same key), the test is
>> guaranteed to deliver the same result for each found instance.
>> This is just crap. Presumably the idea was to search for the same
>> key
>> but potentially different values?
>
> I don't think this code matches multiple times. The call to member
> only occurs upon the first invocation of find-non-empty.
Ouch. I need to recheck my Scheme-fu.
> Each subsequent time, the current-tail parameter gets the value of
> (cdr current-tail) which is simply iterating the list until running
> out of items.
OK.
> The problem with raw-voice being #f, though, is an issue. It is
> highly unlikely to be found in the context-list, so member produces
> another #f that ultimately leads to the failing (cdr #f). At first, I
> would wonder if raw-voice should never be #f but then I see code
> checking (pair? raw-voice), so it must be valid edge case.
>
> What I cannot tell is whether the entire context-list is to be
> searched when raw-voice is #f or whether the find-non-empty logic
> should be skipped altogether.
>
> So, if we want to fall back to searching the entire list, do this:
>
> ;;;;
> (let find-non-empty ((current-tail (or (member raw-voice context-list)
> context-list)))
> ;;;;
>
> Or if we want to skip the searching, do this:
>
> ;;;;
> (if (and (null? quote-contents) (pair? raw-voice))
> ;;;;
>
> Absolutely no idea which is correct. Sorry. :/
Dan? I think this issue started occuring after a change of yours, so do
you have an idea about the meaning of raw-voice becoming #f here?
--
David Kastrup