Re: markup->string
Le 15/11/2022 à 15:18, David Kastrup a écrit : Conversion rules can look at the number of arguments and only revert to a generic replacement when the number of arguments cannot be determined (like when the arguments are too complex for the patterns, or markup-string is not used as a call but as a function). Yes. That is what !1732 does (with a simple pattern). Even then, the generic call can be given a name like headers-markup->string or markup->string can get a #:rest argument that will get interpreted in the old manner when there are no optional arguments (in order to encourage only the new syntax but maintain backward compatibility) or generally (though this would further proliferation of the old syntax). I scanned the 91 results of https://lists.gnu.org/archive/cgi-bin/namazu.cgi?query=markup-%3Estring&submit=Search%21&idxname=lilypond-user&max=20&result=normal&sort=date%3Alate and did not find a single call to markup->string with the optional argument. I think it is of little importance to keep the previous syntax working. The "complexity" we are talking about is the length of the output of convert-ly relative to the input. There is no complex logic at all. If you are talking about human-readable complexity, for the average user this is eyes-glaze-over material in most contexts (the average user does not even know lambda* and it is not a core Scheme construct). At this point, everybody agrees that the lambda* replacement was a bad idea. The current debate is between (1) a simple warning in convert-ly, without automated replacement, and (2) a new optional/keyword argument to markup->string or a new function. If you are talking about computer-readable complexity, lambda* is a huge beast. See above. If you are talking about conceptual complexity, headers-property-alist-chain depends on internal data structure details for the conversion of headers to a property-alist-chain. What "internal data structure details"? Since the interface now does optional argument processing, there is no point in not having markup->string x #:headers headers at the very least in order to obviate the knowledge of internal processing at the call point. And this is the kind of conversion call that convert-ly should aim for. See the question above. OpenPGP_signature Description: OpenPGP digital signature
Re: markup->string
On Tue, Nov 15, 2022 at 02:49:55PM +0100, Jean Abou Samra wrote: > Le 15/11/2022 à 14:43, David Kastrup a écrit : > > If it's "mundane", why would the conversion result in a complex > > replacement? > > Have you looked at the replacement? > > It is > > (lambda* (m #:optional headers) > (if headers > (markup->string m #:props (headers-property-alist-chain headers)) > (markup->string m)) I agree with others' assessment that this doesn't "look" good: it's taking logic that, in a perfect world, would be internal to convert-ly, and inserting it into users' documents. We wouldn't advise users to write this logical check in a new score. We would tell them only to use whichever of the two options applies to their use. > The "complexity" we are talking about is the length of the output > of convert-ly relative to the input. There is no complex logic > at all. Well yes, but for many users it would be replacing something short or simple with something they are not familiar with. The merge request you raised to address it looks good (especially because it only warns for anything more complex than `(markup->string )` Kevin
Re: markup->string
Jean Abou Samra writes: > Le 15/11/2022 à 14:43, David Kastrup a écrit : >> If it's "mundane", why would the conversion result in a complex >> replacement? > > > > Have you looked at the replacement? > > It is > > (lambda* (m #:optional headers) > (if headers > (markup->string m #:props (headers-property-alist-chain headers)) > (markup->string m)) > > which is the only thing convert-ly can programmatically > replace markup->string with to make the change equivalent to > > (markup->string x) -> (markup->string x) [no change] > (markup->string x headers) -> (markup->string x #:props > (headers-property-alist-chain headers)) Conversion rules can look at the number of arguments and only revert to a generic replacement when the number of arguments cannot be determined (like when the arguments are too complex for the patterns, or markup-string is not used as a call but as a function). Even then, the generic call can be given a name like headers-markup->string or markup->string can get a #:rest argument that will get interpreted in the old manner when there are no optional arguments (in order to encourage only the new syntax but maintain backward compatibility) or generally (though this would further proliferation of the old syntax). > The "complexity" we are talking about is the length of the output of > convert-ly relative to the input. There is no complex logic at all. If you are talking about human-readable complexity, for the average user this is eyes-glaze-over material in most contexts (the average user does not even know lambda* and it is not a core Scheme construct). If you are talking about computer-readable complexity, lambda* is a huge beast. If you are talking about conceptual complexity, headers-property-alist-chain depends on internal data structure details for the conversion of headers to a property-alist-chain. Since the interface now does optional argument processing, there is no point in not having markup->string x #:headers headers at the very least in order to obviate the knowledge of internal processing at the call point. And this is the kind of conversion call that convert-ly should aim for. -- David Kastrup
Re: markup->string
Le 15/11/2022 à 14:43, David Kastrup a écrit : If it's "mundane", why would the conversion result in a complex replacement? Have you looked at the replacement? It is (lambda* (m #:optional headers) (if headers (markup->string m #:props (headers-property-alist-chain headers)) (markup->string m)) which is the only thing convert-ly can programmatically replace markup->string with to make the change equivalent to (markup->string x) -> (markup->string x) [no change] (markup->string x headers) -> (markup->string x #:props (headers-property-alist-chain headers)) The "complexity" we are talking about is the length of the output of convert-ly relative to the input. There is no complex logic at all. OpenPGP_signature Description: OpenPGP digital signature
Re: markup->string
Jean Abou Samra writes: > Le 15/11/2022 à 14:17, David Kastrup a écrit : >> Alternatively, providing something approaching the previous behavior >> under a different name, assuming that this functionality has at least >> some motivation or possibility to continue existing. > > > > It's not a behavior change, just a mundane calling convention > change that makes the function more flexible and consistent > with interpret-markup. No "functionality" is being removed. If it's "mundane", why would the conversion result in a complex replacement? -- David Kastrup
Re: markup->string
Le 15/11/2022 à 14:17, David Kastrup a écrit : Alternatively, providing something approaching the previous behavior under a different name, assuming that this functionality has at least some motivation or possibility to continue existing. It's not a behavior change, just a mundane calling convention change that makes the function more flexible and consistent with interpret-markup. No "functionality" is being removed. OpenPGP_signature Description: OpenPGP digital signature
Re: markup->string
Jean Abou Samra writes: > Le 15/11/2022 à 00:56, Jean Abou Samra a écrit : >>> Nevertheless the insertion done by convert-ly is not nice, imho. As a >>> mere user I'd think some bug happened. >> In that case, NOT_SMART is the way to go (i.e., not changing > anything but just printing “Not smart enough to convert…”). > > > > https://gitlab.com/lilypond/lilypond/-/merge_requests/1732 Alternatively, providing something approaching the previous behavior under a different name, assuming that this functionality has at least some motivation or possibility to continue existing. -- David Kastrup
Re: markup->string
Le 15/11/2022 à 00:56, Jean Abou Samra a écrit : Nevertheless the insertion done by convert-ly is not nice, imho. As a mere user I'd think some bug happened. In that case, NOT_SMART is the way to go (i.e., not changing anything but just printing “Not smart enough to convert…”). https://gitlab.com/lilypond/lilypond/-/merge_requests/1732 OpenPGP_signature Description: OpenPGP digital signature
Re: markup->string
> Le 15 nov. 2022 à 00:48, Thomas Morley a écrit : > > Well, I have to admit I can't follow. > In my understanding the old markup->string has one or more arguments, > the first must be of type markup. > Obviously my understanding is not entirely correct. It is correct. The problem is that parsing Scheme syntax is not as straightforward as you might think. > Nevertheless the insertion done by convert-ly is not nice, imho. As a > mere user I'd think some bug happened. In that case, NOT_SMART is the way to go (i.e., not changing anything but just printing “Not smart enough to convert…”). > Would it be feasable to do a type-checking for `headers' in > `headers-property-alist-chain'? > At least one could eliminate the (if ...) in the convert-rule. I don’t think it’s a good thing on the long term to make headers-property-alist-chain accept #f.
Re: markup->string
Am So., 13. Nov. 2022 um 16:30 Uhr schrieb Jean Abou Samra :
>
>
>
> > Le 13 nov. 2022 à 16:22, Thomas Morley a écrit :
> >
> > Nevertheless, _if_ the old code is just (markup->string
> > ), would it be possible to leave it untouched while
> > running convert-ly? After all it continues to work with 2.23. in this
> > simple manor, only inserting a more complex expression, if the old
> > code already has an optional argument?
> > Can't check myself, my python is as non-existent as my C++ ...
>
>
> How do you know if the old code does not use the optional argument? It could
> be any Scheme expression, or even a #{ … #} expression. Scheme has more
> syntax than one might think: there could be ; or #! or #| comments and all
> sorts of things.
>
> One could special-case (markup->string ), catching a subset of those
> cases. I’m not bothered by the current replacement, but would that make you
> happier? In any case, we can’t reliably detect all cases of markup->string
> applies to one argument only.
>
> Jean
Well, I have to admit I can't follow.
In my understanding the old markup->string has one or more arguments,
the first must be of type markup.
Obviously my understanding is not entirely correct.
Nevertheless the insertion done by convert-ly is not nice, imho. As a
mere user I'd think some bug happened.
Would it be feasable to do a type-checking for `headers' in
`headers-property-alist-chain'?
At least one could eliminate the (if ...) in the convert-rule.
Something at the lines of:
(define-public (headers-property-alist-chain headers)
"Take a list of @code{\\header} blocks (Guile modules). Return an
alist chain containing all of their bindings where the names have been
prefixed with @code{header:}. This alist chain is suitable for
interpreting a markup in the context of these headers."
(map
(lambda (module)
(map
(lambda (entry)
(cons
(string->symbol
(string-append "header:"
(symbol->string (car entry
(cdr entry)))
(ly:module->alist module)))
(or headers '(
^
Or if this to bold/optimistic:
(if (list? headers) headers '())
Cheers,
Harm
Re: markup->string
> Le 13 nov. 2022 à 16:22, Thomas Morley a écrit :
>
> Nevertheless, _if_ the old code is just (markup->string
> ), would it be possible to leave it untouched while
> running convert-ly? After all it continues to work with 2.23. in this
> simple manor, only inserting a more complex expression, if the old
> code already has an optional argument?
> Can't check myself, my python is as non-existent as my C++ ...
How do you know if the old code does not use the optional argument? It could be
any Scheme expression, or even a #{ … #} expression. Scheme has more syntax
than one might think: there could be ; or #! or #| comments and all sorts of
things.
One could special-case (markup->string ), catching a subset of those
cases. I’m not bothered by the current replacement, but would that make you
happier? In any case, we can’t reliably detect all cases of markup->string
applies to one argument only.
Jean
Re: markup->string
Am So., 13. Nov. 2022 um 15:43 Uhr schrieb Jean Abou Samra :
>
> Le 13/11/2022 à 14:41, Thomas Morley a écrit :
> > Hi,
> >
> > please consider below (originally for 2.20. _not_ converted):
> >
> > \version "2.23.80"
> >
> > \header {
> >myTitle = "myTitle"
> >myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
> >title = $(markup->string myOtherTitle)
> > }
> >
> > \markup { "TEST" }
> >
> > A title is not printed.
> > Though, I'm at loss, am I doing something wrongly or is it a bug
> > (\fromproperty has an `as-string'-setting) or _should_ it not work?
>
>
>
>
> This is working as expected. \fromproperty is not different from
> other markup commands. If you try #(display myOtherTitle), you will
> see that myTitle has not been replaced with its value at the time of
> creation of the markup. It is only when the markup is interpreted
> that the definition of the \fromproperty command looks up
> the value of myTitle, and it does so in the props argument
> passed to interpret-markup in the call (interpret-markup layout props mkup).
> When the \header title is implicitly interpreted by LilyPond,
> this is done with a props argument reflecting the header fields,
> as you can see from
>
> \version "2.23.80"
>
> \header {
>foo = "foo value"
>bar = "bar value"
>title = \markup $(markup-lambda (layout props) ()
> ((@ (ice-9 pretty-print) pretty-print) props)
> empty-stencil)
> }
>
> \markup Test
>
>
> The way you are calling markup->string, it does not have this information
> at hand. You need to provide it with
>
>
> \version "2.23.80"
>
> \header {
>myTitle = "myTitle"
>myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
>title = $(markup->string myOtherTitle #:props
> (headers-property-alist-chain (list (current-module
> }
>
> \markup { "TEST" }
>
>
>
> > tl;dr
> > In a private mail the user wondered why running convert-ly over old
> > code containing `markup->string' results in such a complex insertion:
> > 2.20.0 code:
> > title = $(markup->string myOtherTitle)
> > becomes with 2.23.80:
> > title = $(
> > (lambda* (m #:optional headers)
> >(if headers
> >(markup->string m #:props (headers-property-alist-chain headers))
> >(markup->string m)))
> > myOtherTitle)
> >
> > Alas, it doesn't work with the initial code above, still no title.
> >
> > Thus, if it does not work either way, do we need the complex result of
> > convert-ly at all?
>
>
>
>
> But your example code
>
> \header {
>myTitle = "myTitle"
>myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
>title = $(markup->string myOtherTitle)
> }
>
> \markup { "TEST" }
>
>
> did not work originally in 2.20, did it? I can't test that version on
> my machine (Pango throws an assertion error), but I confirm that in
> 2.22 it does not work. That is as expected, for exactly the same
> reason as above. It does work with
>
> \version "2.22.2"
>
> \header {
>myTitle = "myTitle"
>myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
>title = $(markup->string myOtherTitle (list (current-module)))
> }
>
> \markup { "TEST" }
>
>
>
> In 2.22, the second optional argument to markup->string was a list
> of header modules. In 2.23, there are two optional keyword arguments,
> #:layout and #:props, and the equivalent of the previous headers
> argument is achieved using headers-property-alist-chain for #:props.
> The convert-ly replacement is a way to cater for that change of
> signature. Admittedly, it is a bit complex, but the alternative would have
> been a NOT_SMART, and I'm not sure that would have been better...
>
> Jean
>
>
Thanks for your detailed explanations.
Nevertheless, _if_ the old code is just (markup->string
), would it be possible to leave it untouched while
running convert-ly? After all it continues to work with 2.23. in this
simple manor, only inserting a more complex expression, if the old
code already has an optional argument?
Can't check myself, my python is as non-existent as my C++ ...
Cheers,
Harm
Re: markup->string
Le 13/11/2022 à 14:41, Thomas Morley a écrit :
Hi,
please consider below (originally for 2.20. _not_ converted):
\version "2.23.80"
\header {
myTitle = "myTitle"
myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
title = $(markup->string myOtherTitle)
}
\markup { "TEST" }
A title is not printed.
Though, I'm at loss, am I doing something wrongly or is it a bug
(\fromproperty has an `as-string'-setting) or _should_ it not work?
This is working as expected. \fromproperty is not different from
other markup commands. If you try #(display myOtherTitle), you will
see that myTitle has not been replaced with its value at the time of
creation of the markup. It is only when the markup is interpreted
that the definition of the \fromproperty command looks up
the value of myTitle, and it does so in the props argument
passed to interpret-markup in the call (interpret-markup layout props mkup).
When the \header title is implicitly interpreted by LilyPond,
this is done with a props argument reflecting the header fields,
as you can see from
\version "2.23.80"
\header {
foo = "foo value"
bar = "bar value"
title = \markup $(markup-lambda (layout props) ()
((@ (ice-9 pretty-print) pretty-print) props)
empty-stencil)
}
\markup Test
The way you are calling markup->string, it does not have this information
at hand. You need to provide it with
\version "2.23.80"
\header {
myTitle = "myTitle"
myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
title = $(markup->string myOtherTitle #:props
(headers-property-alist-chain (list (current-module
}
\markup { "TEST" }
tl;dr
In a private mail the user wondered why running convert-ly over old
code containing `markup->string' results in such a complex insertion:
2.20.0 code:
title = $(markup->string myOtherTitle)
becomes with 2.23.80:
title = $(
(lambda* (m #:optional headers)
(if headers
(markup->string m #:props (headers-property-alist-chain headers))
(markup->string m)))
myOtherTitle)
Alas, it doesn't work with the initial code above, still no title.
Thus, if it does not work either way, do we need the complex result of
convert-ly at all?
But your example code
\header {
myTitle = "myTitle"
myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
title = $(markup->string myOtherTitle)
}
\markup { "TEST" }
did not work originally in 2.20, did it? I can't test that version on
my machine (Pango throws an assertion error), but I confirm that in
2.22 it does not work. That is as expected, for exactly the same
reason as above. It does work with
\version "2.22.2"
\header {
myTitle = "myTitle"
myOtherTitle = \markup { \italic \fromproperty #'header:myTitle }
title = $(markup->string myOtherTitle (list (current-module)))
}
\markup { "TEST" }
In 2.22, the second optional argument to markup->string was a list
of header modules. In 2.23, there are two optional keyword arguments,
#:layout and #:props, and the equivalent of the previous headers
argument is achieved using headers-property-alist-chain for #:props.
The convert-ly replacement is a way to cater for that change of
signature. Admittedly, it is a bit complex, but the alternative would have
been a NOT_SMART, and I'm not sure that would have been better...
Jean
OpenPGP_signature
Description: OpenPGP digital signature
