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
markup->string
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?
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?
Thanks,
Harm
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
Hi Paul, thanks for review https://codereview.appspot.com/34743/diff/40001/scm/markup.scm File scm/markup.scm (right): https://codereview.appspot.com/34743/diff/40001/scm/markup.scm#newcode141 scm/markup.scm:141: ;; The string is split at line-breaks, emty strings removed and finally On 2018/11/11 15:43:37, pwm wrote: typo: empty Done. https://codereview.appspot.com/34743/diff/40001/scm/markup.scm#newcode148 scm/markup.scm:148: simple-markup))) On 2018/11/11 15:43:37, pwm wrote: Indent 'list' expression so it is clearer that it is the second argument to 'member' and not another 'and' expression. Done. https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
On 2018/11/11 15:08:57, thomasmorley651 wrote: simplify, catch string-markups patch-set 3 reflects my musing in comment #5 https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
Hi Harm, I took a quick look and it LGTM at a quick read. I have a couple nit-level suggestions. -Paul https://codereview.appspot.com/34743/diff/40001/scm/markup.scm File scm/markup.scm (right): https://codereview.appspot.com/34743/diff/40001/scm/markup.scm#newcode141 scm/markup.scm:141: ;; The string is split at line-breaks, emty strings removed and finally typo: empty https://codereview.appspot.com/34743/diff/40001/scm/markup.scm#newcode148 scm/markup.scm:148: simple-markup))) Indent 'list' expression so it is clearer that it is the second argument to 'member' and not another 'and' expression. https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
On 2018/11/11 11:07:35, dak wrote: On 2018/11/11 10:57:31, thomasmorley651 wrote: > 'all-relevant-markup-commands' tries to get all markup-(list-)commands where > 'markup->string' may return reasonable output. I have to get used to the thought of not having the monopoly on good ideas. That's all. Well, I'm pretty sure while working on https://sourceforge.net/p/testlilyissues/issues/4685/ you pointed me in the direction how to deduce things like that. ;) Though, I just noticed I introduced a so far unnoticed glitch with 4685 markup->string doesn't work on \simple I looked which other markup commands are unnoticed thrown away. Below they are listed, devided into worth-to-keep and to-ignore, as far as I would think: worth-to-keep: wordwrap-string-markup justify-string-markup simple-markup wordwrap-string-internal-markup-list to-ignore: lookup-markup postscript-markup epsfile-markup fret-diagram-markup tied-lyric-markup musicglyph-markup fret-diagram-terse-markup rest-markup verbatim-file-markup harp-pedal-markup For wordwrap-string-markup and justify-string-markup I think they should be special-cased, meaning deleting tab and newline-characters. Maybe simple-markup can be put there as well. Not sure about wordwrap-string-internal-markup-list, keep or ignore? The to-ignore-markups could be appended to the list already defined with 'markup-commands-to-ignore'. Though, this would introduce a manually to maintain instance which I'd like to avoid, but I'm not aware of a condition to exlude them. Otoh 'markup-commands-to-ignore' has already an entry, so it needs to be mantained anyway. Hmm. Hints? Opinions? https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
On 2018/11/11 10:57:31, thomasmorley651 wrote: On 2018/11/10 12:53:18, dak wrote: > I wonder whether it might be reasonable to just have all markup commands with a > last argument of markup? produce their last (recursively treated) argument as > default, and possibly all markup list commands with a last argument of > markup-list? similarly produce their last argument? That leaves fewer commands > to cater for and would also make some user-defined commands work reasonably > well. I think I do not fully understand what you mean. More a case of me talking about stuff I know nothing about. 'all-relevant-markup-commands' tries to get all markup-(list-)commands where 'markup->string' may return reasonable output. I have to get used to the thought of not having the monopoly on good ideas. That's all. I had assumed that you work with large lists of markup commands and rules rather than this kind of deduction and didn't even bother to check. Sorry for the noise. https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
On 2018/11/10 12:53:18, dak wrote: I wonder whether it might be reasonable to just have all markup commands with a last argument of markup? produce their last (recursively treated) argument as default, and possibly all markup list commands with a last argument of markup-list? similarly produce their last argument? That leaves fewer commands to cater for and would also make some user-defined commands work reasonably well. I think I do not fully understand what you mean. 'all-relevant-markup-commands' tries to get all markup-(list-)commands where 'markup->string' may return reasonable output. Currently by looking whether 'cheap-markup?' or 'markup-list?' are somewhere in their predicates. This could be changed to look whether those are last in predicates. Would simplify the coding and 'markup->string' only works in this case anyway. (At least following a comment there.) So far I understood, at least I hope so. I have some other simplifications in mind, will upload with next patch. Though, the amount of caught markup-(list-)commands is the same for both codings. But what do you mean with "produce their last (recursively treated) argument as default" Could you elaborate a bit? https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
I wonder whether it might be reasonable to just have all markup commands with a last argument of markup? produce their last (recursively treated) argument as default, and possibly all markup list commands with a last argument of markup-list? similarly produce their last argument? That leaves fewer commands to cater for and would also make some user-defined commands work reasonably well. https://codereview.appspot.com/34743/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Improve markup->string (issue 347000043 by thomasmorle...@gmail.com)
Reviewers: ,
Message:
Please review.
There are some TODOs in the code where I'd appreciate some feedback.
Thanks-
Description:
Improve markup->string
'all-relevant-markup-commands' is now a toplevel-defined procedure.
So it is not longer a part of the rekursive 'markup->string'.
It needs to be a procedure, because not all bindings of the
lily-module are already done in markup.scm, so it should be
evaluated at the time 'markup->string' is called.
Additionally we gain the chance to have user-defined
markup-commands from '(current-module)' been processed as well.
Please review this at https://codereview.appspot.com/34743/
Affected files (+31, -23 lines):
M scm/markup.scm
Index: scm/markup.scm
diff --git a/scm/markup.scm b/scm/markup.scm
index
14a007a95bb27f8a89da2aeb51a607695af40068..169e52c78e3b8b15c560ff7efffce14f95aca279
100644
--- a/scm/markup.scm
+++ b/scm/markup.scm
@@ -79,29 +79,38 @@ following stencil. Stencils with empty Y extent are
not given
(define markup-commands-to-ignore
'(page-ref-markup))
+(define (all-relevant-markup-commands)
+ ;; Returns a list containing the names of all markup-commands and
+ ;; markup-list-commands with predicate @code{cheap-markup?} or
+ ;; @code{markup-list?} in their @code{markup-command-signature}.
+ ;; It needs to be a procedure, because before it is called in
+ ;; @code{markup->string}, not all bindings in the lily-module are done
and we
+ ;; want to catch user-defined markup-commands from @code{current-module}
+ ;; as well.
+ ;; @code{table-of-contents} is not caught, though.
+ ;; Markup-commands from @code{markup-commands-to-ignore} are removed.
+ (lset-difference eq?
+(map car
+ (filter
+(lambda (x)
+ (let* ((predicates (markup-command-signature (cdr x
+(and predicates
+ (not
+ (null?
+ (lset-intersection eq?
+ '(cheap-markup? markup-list?)
+ (map procedure-name predicates)))
+;; TODO
+;; - other modules to look at?
+;; - need to care about conflicts/duplicates?
+(append
+ (ly:module->alist (current-module))
+ (ly:module->alist (resolve-module '(lily))
+markup-commands-to-ignore))
+
(define-public (markup->string m . argscopes)
(let* ((scopes (if (pair? argscopes) (car argscopes) '(
-(define all-relevant-markup-commands
- ;; Returns a list containing the names of all markup-commands and
- ;; markup-list-commands with predicate @code{cheap-markup?} or
- ;; @code{markup-list?} in their @code{markup-command-signature}.
- ;; @code{table-of-contents} is not caught, same for user-defined
commands.
- ;; markup-commands from @code{markup-commands-to-ignore} are removed.
- (lset-difference eq?
-(map car
- (filter
-(lambda (x)
- (let* ((predicates (markup-command-signature (cdr x
-(and predicates
- (not
- (null?
- (lset-intersection eq?
- '(cheap-markup? markup-list?)
- (map procedure-name predicates)))
-(ly:module->alist (resolve-module '(lily)
-markup-commands-to-ignore))
-
;; helper functions to handle string cons like string lists
(define (markup-cons->string-cons c scopes)
(if (not (pair? c)) (markup->string c scopes)
@@ -113,8 +122,7 @@ following stencil. Stencils with empty Y extent are
not given
(string-join (list (car c) (string-cons-join (cdr c))) "")))
;; We let the following line in for future debugging
-;; (display-scheme-music (sort all-relevant-markup-commands symbol+;; (display-scheme-music (sort (all-relevant-markup-commands)
symbol
Remark: below only works, if markup?- or markup-list? arguments
are the
last listed arguments in the commands definition
@@ -158,7 +166,7 @@ following stencil. Stencils with empty Y extent are
not given
(markup->string (ly:modules-lookup scopes var) (cons mod scopes
((member (car m)
- (primitive-eval (cons 'list all-relevant-markup-commands)))
+ (primitive-eval (cons 'list (all-relevant-markup-commands
(markup->string
(if (> (length (last-pair m)) 1)
(last-pair m)
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Improve markup->string (issue 282740043 by thomasmorle...@gmail.com)
Reviewers: ,
Message:
please review
nb there are some TODO's
Description:
Improve markup->string
Search and filter lily-module for all relevant markup-(list)-commands
to prevent error-prone manual selecting.
Special-casing put-adjacent and fill-with-pattern
Please review this at https://codereview.appspot.com/282740043/
Affected files (+53, -32 lines):
M scm/markup.scm
Index: scm/markup.scm
diff --git a/scm/markup.scm b/scm/markup.scm
index
b3b7b34c30a9b0f33e077cf11cbed2200d181469..96046a8de0f45ba033296e45c1c7c795b5e9231f
100644
--- a/scm/markup.scm
+++ b/scm/markup.scm
@@ -70,66 +70,87 @@ following stencil. Stencils with empty Y extent are
not given
(define-public (markup->string m . argscopes)
(let* ((scopes (if (pair? argscopes) (car argscopes) '(
-;; markup commands with one markup argument, formatting ignored
-(define markups-first-argument '(list
- bold-markup box-markup caps-markup
dynamic-markup finger-markup
- fontCaps-markup huge-markup
italic-markup large-markup larger-markup
- medium-markup normal-size-sub-markup
normal-size-super-markup
- normal-text-markup normalsize-markup
number-markup roman-markup
- sans-markup simple-markup
small-markup smallCaps-markup smaller-markup
- sub-markup super-markup teeny-markup
text-markup tiny-markup
- typewriter-markup underline-markup
upright-markup bracket-markup
- circle-markup hbracket-markup
parenthesize-markup rounded-box-markup
-
- center-align-markup
center-column-markup column-markup dir-column-markup
- fill-line-markup justify-markup
justify-string-markup left-align-markup
- left-column-markup line-markup
right-align-markup right-column-markup
- vcenter-markup wordwrap-markup
wordwrap-string-markup ))
-
-;; markup commands with markup as second argument, first argument
-;; specifies some formatting and is ignored
-(define markups-second-argument '(list
- abs-fontsize-markup fontsize-markup
magnify-markup lower-markup
- pad-around-markup pad-markup-markup
pad-x-markup raise-markup
- halign-markup hcenter-in-markup
rotate-markup translate-markup
- translate-scaled-markup
with-url-markup scale-markup ))
+
+(define all-relevant-markup-commands
+ ;; Returns a list containing the names of all markup-commands and
+ ;; markup-list-commands with predicate @code{cheap-markup?} or
+ ;; @code{markup-list?} in their @code{markup-command-signature}.
+ ;; @code{table-of-contents} is not caught, same for user-defined
commands.
+ (map car
+(filter
+ (lambda (x)
+(let* ((predicates (markup-command-signature (cdr x
+ (and predicates
+ (not
+ (null?
+ (lset-intersection eq?
+ '(cheap-markup? markup-list?)
+ (map procedure-name predicates)))
+ (ly:module->alist (resolve-module '(lily))
;; helper functions to handle string cons like string lists
(define (markup-cons->string-cons c scopes)
(if (not (pair? c)) (markup->string c scopes)
- (cons (markup->string (car c) scopes) (markup-cons->string-cons
(cdr c) scopes
+ (cons
+(markup->string (car c) scopes)
+(markup-cons->string-cons (cdr c) scopes
(define (string-cons-join c)
(if (not (pair? c)) c
(string-join (list (car c) (string-cons-join (cdr c))) "")))
+;; We let the following line in for future debugging
+;(display-scheme-music (sort all-relevant-markup-commands symbol+ Remark: below only works, if markup?- or markup-list? arguments
are the
+ last listed arguments in the commands definition
+ TODO: which other markup-(list)-commands should be special cased
or
+ completely excluded?
(cond
((string? m) m)
((null? m) "")
((not (pair? m)) "")
+ special cases: \concat, \put-adjacent, \fill-with-pattern and
+ \fromproperty-markup
+
+ TODO do we really want a string-joined return-value for \concat
and
+ \put-adjacent?
+ \overlay or \combine will return a string with spaces
+
;; handle \concat (string-join without spaces)
((and
Re: Issue 3117: Markup/string identifiers in lyrics cause an error (issue 8102043)
As usual with parser patches, i'm not the right person to review them... i think this one is ok. Thanks, Janek https://codereview.appspot.com/8102043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
[issue 1482] \caps \fromproperty and markup->string
http://code.google.com/p/lilypond/issues/detail?id=1482
Hello lists,
using \fromproperty, \smallCaps (=\caps) and markup->string leads into
some pitfalls:
1. if you want to use custom (self written) markup-commands, the default
"markup->string" function will drop the content of them.
2. if you want to use caps, that function expects a string to capsify,
so that if you try "\caps \italic text" the text will only be italic but
not capsified.
3. if you want to use \fromproperty #'sym within caps or markup->string,
the result will be empty.
I post these three in one mail, because the problems are related.
Here are my tries overcoming these obstacles:
1. a redesigned markup->string function, that is command-independent and
looks for fromproperty-markups
2. a recaps markup-command relying on a recursive function capsifying
all found strings and fromproperty-markup-results
This design may lead to other problems: E.g. if you have an arbitrary
markup-command, wich takes a markup to scale another one, that
gauge-markup will be put into the string
Right now this is not a patch for devel, because I am running stable and
use this for my current projects.
Cheers,
Jan-Peter
--snip--
\version "2.14.2"
% taken from markup.scm
#(define (markup-function? x)
(and (markup-command-signature x)
(not (object-property x 'markup-list-command
% a markup-command-independent markup->string function
#(define-public (markup->string mup . mprops)
(let ((conc (if (> (length mprops) 0) (car mprops) #f))
(layout (if (> (length mprops) 1) (cadr mprops) #f))
(props (if (> (length mprops) 2) (caddr mprops) '()))
(mup? (lambda (m)(or (string? m)(list? m)(markup? m
(result ""))
(cond ((string? mup) (set! result mup))
((null? mup) (set! result ""))
((and (pair? mup) (equal? (car mup) concat-markup))
(set! result (markup->string (cdr mup) #t layout props)))
((and (pair? mup) (equal? (car mup) fromproperty-markup))
(set! result (markup->string (chain-assoc-get (cadr mup)
props "???") conc layout props)))
((and (pair? mup)(markup-function? (car mup)))
(for-each (lambda (m)(set! result (string-append result
(if (or conc (string=? result "")) "" " ") (markup->string m conc layout
props
(filter mup? (cdr mup
((list? mup)
(for-each (lambda (m)(set! result (string-append result
(if (or conc (string=? result "")) "" " ") (markup->string m conc layout
props
(filter mup? mup)))
(else (ly:message "~A" mup)))
result))
% plain text markup
#(define-markup-command (plain-text layout props arg)(markup?)
(interpret-markup layout props (markup (markup->string arg #f layout
props
% apply caps to all strings found in m
#(define-public (rcaps m . mprops)
(let ((layout (if (> (length mprops) 0) (car mprops) #f))
(props (if (> (length mprops) 1) (cadr mprops) '()))
(mup? (lambda (m)(or (string? m)(list? m)(markup? m
(ret (markup)))
(set! ret
(cond
((string? m) (markup #:caps m))
((null? m) "")
((and (pair? m) (equal? (car m) fromproperty-markup))
(markup #:caps (chain-assoc-get (cadr m) props "???")))
((and (pair? m)(markup-function? (car m))) (cons (car m)
(map (lambda (mup)(if (mup? mup) (rcaps mup layout
props) mup)) (cdr m
((list? m)
(map (lambda (mup)(rcaps mup layout props)) m))
(else "ERROR, unable to caps markup"))
)
ret)
)
% recursive caps markup command
#(define-markup-command (recaps layout props mup)(markup?)
(interpret-markup layout props (rcaps mup layout props)))
% example/test
test = \markup {
Hallo \recaps { Welt \fontsize #3 Du große \concat { \italic { Welt 2
3 } \fromproperty #'dummytext } }
}
#(display (markup->string test #f #f '(((dummytext . "bla"))) ))
\markup {
\override #'(dummytext . "bla") \column {
\test
\plain-text \test
}
}
--snip--
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
