strange replacement 100 → hundred in NR

[Werner LEMBERG]

[git 9c3803fba4960b4afa63baeb50201a0cfa48f8f1]

I've just built the whole documentation, and I get a strange string
replacement in Appendix A of the NR (from file
`markup-list-commands.texi`), see attached image.

As far as I remember, this didn't happen previously.  Is it possible
that this code from `input.itely`

  \paper {
    #(add-text-replacements!
      '(("100" . "hundred")
        ("dpi" . "dots per inch")))
  }

changes the state of lilypond while lilypond-book now processes more
files in one run?


    Werner

[Han-Wen Nienhuys-3]

On Wed, Sep 30, 2020 at 8:05 PM Werner LEMBERG <[hidden email]> wrote:

>
> [git 9c3803fba4960b4afa63baeb50201a0cfa48f8f1]
>
> I've just built the whole documentation, and I get a strange string
> replacement in Appendix A of the NR (from file
> `markup-list-commands.texi`), see attached image.
>
As far as I remember, this didn't happen previously.  Is it possible

> that this code from `input.itely`
>
>   \paper {
>     #(add-text-replacements!
>       '(("100" . "hundred")
>         ("dpi" . "dots per inch")))
>   }
>
> changes the state of lilypond while lilypond-book now processes more
> files in one run?
>

Yes, very likely. See commit 9f16e2962ac71e6a30526012e314d55cec6b7e01.

For some reason the check for replacement_cache_alist_key_ is not
working as intended.

--
Han-Wen Nienhuys - [hidden email] - http://www.xs4all.nl/~hanwen

[Han-Wen Nienhuys-3]

On Wed, Sep 30, 2020 at 10:11 PM Han-Wen Nienhuys <[hidden email]> wrote:

>
>
> On Wed, Sep 30, 2020 at 8:05 PM Werner LEMBERG <[hidden email]> wrote:
>
>>
>> [git 9c3803fba4960b4afa63baeb50201a0cfa48f8f1]
>>
>> I've just built the whole documentation, and I get a strange string
>> replacement in Appendix A of the NR (from file
>> `markup-list-commands.texi`), see attached image.
>>
> As far as I remember, this didn't happen previously.  Is it possible
>> that this code from `input.itely`
>>
>>   \paper {
>>     #(add-text-replacements!
>>       '(("100" . "hundred")
>>         ("dpi" . "dots per inch")))
>>   }
>>
>> changes the state of lilypond while lilypond-book now processes more
>> files in one run?
>>
>
> Yes, very likely. See commit 9f16e2962ac71e6a30526012e314d55cec6b7e01.
>
> For some reason the check for replacement_cache_alist_key_ is not
> working as intended.
>

It looks like the C++ code is working as intended, but

 #(define (add-text-replacements! alist)
   (set! text-font-defaults
         (assoc-set! text-font-defaults 'replacement-alist
                     (cdaar
                      (internal-add-text-replacements (list
text-font-defaults) alist)))))

modifies text-font-defaults in-place. It looks like the definition being
changed is the default paper setting, which is shared across files.

what was the last version in which this was seen working as expected?

--
Han-Wen Nienhuys - [hidden email] - http://www.xs4all.nl/~hanwen

[David Kastrup]

Han-Wen Nienhuys <[hidden email]> writes:

> On Wed, Sep 30, 2020 at 10:11 PM Han-Wen Nienhuys <[hidden email]> wrote:
>
>>
>>
>> On Wed, Sep 30, 2020 at 8:05 PM Werner LEMBERG <[hidden email]> wrote:
>>
>>>
>>> [git 9c3803fba4960b4afa63baeb50201a0cfa48f8f1]
>>>
>>> I've just built the whole documentation, and I get a strange string
>>> replacement in Appendix A of the NR (from file
>>> `markup-list-commands.texi`), see attached image.
>>>
>> As far as I remember, this didn't happen previously.  Is it possible
>>> that this code from `input.itely`
>>>
>>>   \paper {
>>>     #(add-text-replacements!
>>>       '(("100" . "hundred")
>>>         ("dpi" . "dots per inch")))
>>>   }
>>>
>>> changes the state of lilypond while lilypond-book now processes more
>>> files in one run?
>>>
>>
>> Yes, very likely. See commit 9f16e2962ac71e6a30526012e314d55cec6b7e01.
>>
>> For some reason the check for replacement_cache_alist_key_ is not
>> working as intended.
>>
>
> It looks like the C++ code is working as intended, but
>
>  #(define (add-text-replacements! alist)
>    (set! text-font-defaults
>          (assoc-set! text-font-defaults 'replacement-alist
>                      (cdaar
>                       (internal-add-text-replacements (list
> text-font-defaults) alist)))))
>
> modifies text-font-defaults in-place. It looks like the definition being
> changed is the default paper setting, which is shared across files.

assoc-set! on a grob property is not just playing fast and loose across
files but breaks in-file consistency completely as well.

> what was the last version in which this was seen working as expected?

Probably never did but people got lucky.

--
David Kastrup

[Werner LEMBERG]

>> what was the last version in which this was seen working as
>> expected?

For me, three weeks ago.

> Probably never did but people got lucky.

Yes, it looks like that.


    Werner

[Dev mailing list]

Am Donnerstag, den 01.10.2020, 05:07 +0200 schrieb Werner LEMBERG:
> >> what was the last version in which this was seen working as
> >> expected?
>
> For me, three weeks ago.
>
> > Probably never did but people got lucky.
>
> Yes, it looks like that.

So I think it's more likely that the issue got exposed by
commit 5a957021a3 which runs lilypond once per document passed to
lilypond-book instead of per included file. This increases the chance
of being unlucky...

As far as I understand, the correct solution is to reset text-font-
defaults per session. I've put up a MR that seems to work for local
tests: https://gitlab.com/lilypond/lilypond/-/merge_requests/431
Due to the nature of the issue, I need to run two full doc builds (on
master and with the change) to ensure it really solves the problem...

Jonas

[David Kastrup]

Jonas Hahnfeld <[hidden email]> writes:

> Am Donnerstag, den 01.10.2020, 05:07 +0200 schrieb Werner LEMBERG:
>> >> what was the last version in which this was seen working as
>> >> expected?
>>
>> For me, three weeks ago.
>>
>> > Probably never did but people got lucky.
>>
>> Yes, it looks like that.
>
> So I think it's more likely that the issue got exposed by
> commit 5a957021a3 which runs lilypond once per document passed to
> lilypond-book instead of per included file. This increases the chance
> of being unlucky...
>
> As far as I understand, the correct solution is to reset text-font-
> defaults per session.

No, the correct solution is not to use assq-set! on shared variables,
and variables accessible to \override are shared.

> I've put up a MR that seems to work for local tests:
> https://gitlab.com/lilypond/lilypond/-/merge_requests/431 Due to the
> nature of the issue, I need to run two full doc builds (on master and
> with the change) to ensure it really solves the problem...
>
> Jonas
>

--
David Kastrup

[Dev mailing list]

Am Donnerstag, den 01.10.2020, 14:07 +0200 schrieb David Kastrup:

> Jonas Hahnfeld <[hidden email]> writes:
>
> > Am Donnerstag, den 01.10.2020, 05:07 +0200 schrieb Werner LEMBERG:
> > > > > what was the last version in which this was seen working as
> > > > > expected?
> > >
> > > For me, three weeks ago.
> > >
> > > > Probably never did but people got lucky.
> > >
> > > Yes, it looks like that.
> >
> > So I think it's more likely that the issue got exposed by
> > commit 5a957021a3 which runs lilypond once per document passed to
> > lilypond-book instead of per included file. This increases the chance
> > of being unlucky...
> >
> > As far as I understand, the correct solution is to reset text-font-
> > defaults per session.
>
> No, the correct solution is not to use assq-set! on shared variables,
> and variables accessible to \override are shared.

Yes, that is what's needed to actually make the session variable work.

I'm now doing assoc-set! on (alist-copy text-font-defaults), see the
MR. Is that what you're saying?

> > I've put up a MR that seems to work for local tests:
> > https://gitlab.com/lilypond/lilypond/-/merge_requests/431
> >  Due to the
> > nature of the issue, I need to run two full doc builds (on master and
> > with the change) to ensure it really solves the problem...
> >
> > Jonas

[David Kastrup]

Jonas Hahnfeld <[hidden email]> writes:

> Am Donnerstag, den 01.10.2020, 14:07 +0200 schrieb David Kastrup:
>> Jonas Hahnfeld <[hidden email]> writes:
>>
>> > Am Donnerstag, den 01.10.2020, 05:07 +0200 schrieb Werner LEMBERG:
>> > > > > what was the last version in which this was seen working as
>> > > > > expected?
>> > >
>> > > For me, three weeks ago.
>> > >
>> > > > Probably never did but people got lucky.
>> > >
>> > > Yes, it looks like that.
>> >
>> > So I think it's more likely that the issue got exposed by
>> > commit 5a957021a3 which runs lilypond once per document passed to
>> > lilypond-book instead of per included file. This increases the chance
>> > of being unlucky...
>> >
>> > As far as I understand, the correct solution is to reset text-font-
>> > defaults per session.
>>
>> No, the correct solution is not to use assq-set! on shared variables,
>> and variables accessible to \override are shared.
> Yes, that is what's needed to actually make the session variable work.
> I'm now doing assoc-set! on (alist-copy text-font-defaults), see the
> MR. Is that what you're saying?

I think it would be saner to use acons here rather than assoc-set!.

--
David Kastrup

[Michael Käppler-2]

Am 01.10.2020 um 12:24 schrieb Jonas Hahnfeld via Discussions on
LilyPond development:
> So I think it's more likely that the issue got exposed by
> commit 5a957021a3 which runs lilypond once per document passed to
> lilypond-book instead of per included file. This increases the chance
> of being unlucky...
Another topic, but possibly related:
Could this potentially affect regtests, that
use (ly:set-option 'warning-as-error) or similar?
In the sense, that the state change triggered by ly:set-option leaks
through to other
snippets?

Btw., is it still good practise to use
the

(ly:set-option 'warning-as-error)
(ly:expect-warning ...)

construct in regtests? I vaguely remember a discussion about that some
time ago on -devel...)

Cheers,
Michael

[Dev mailing list]

Am Donnerstag, den 01.10.2020, 16:03 +0200 schrieb Michael Käppler:
> Am 01.10.2020 um 12:24 schrieb Jonas Hahnfeld via Discussions on
> LilyPond development:
> > So I think it's more likely that the issue got exposed by
> > commit 5a957021a3 which runs lilypond once per document passed to
> > lilypond-book instead of per included file. This increases the chance
> > of being unlucky...
> Another topic, but possibly related:
> Could this potentially affect regtests, that
> use (ly:set-option 'warning-as-error) or similar?

As you note, it's a different topic and IMHO should be discussed
separately. If there's been a recent change, please start a new thread.
In any case, I don't see a relation to the clear regression described
by Werner and the mentioned commit as collated-files.tely includes no
other .itely or .itexi files, only @lilypondfile's.

Jonas