Allow defining markup commands in LilyPond syntax (issue 324140043 by dak@gnu.org)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Allow defining markup commands in LilyPond syntax (issue 324140043 by dak@gnu.org)

lemzwerg
By mere inspection: LGTM

https://codereview.appspot.com/324140043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Allow defining markup commands in LilyPond syntax (issue 324140043 by dak@gnu.org)

Thomas Morley-2
If I understand correctly the main advantage for the user will be the
possibility to define custom markup-commands with LilyPond-syntax.
Collecting a subset of preexisting commands to substitute tedious
coding/typing.
P.e.
\markup my-name = \markup \with-color #red \rotate #90 \italic \fontsize
#3 \etc
and use it as
\markup \my-name "My text!"

Whereas markup-commands with completely new functionality would need to
be defined as before:
  (define-markup-command (command-name layout props args...)
    args-signature
    [ #:category category ]
    [ #:properties property-bindings ]
    documentation-string
    ..body..)

I should download/apply this patch. After having played around with it
I'll probably be able to say more.

For now two remarks/questions:


https://codereview.appspot.com/324140043/diff/1/scm/document-markup.scm
File scm/document-markup.scm (right):

https://codereview.appspot.com/324140043/diff/1/scm/document-markup.scm#newcode96
scm/document-markup.scm:96: (set! all-markup-commands (sort!
all-markup-commands markup-name<?))
Looks like 'all-markup-(list)-commands' could be used to simplify
'all-relevant-markup-commands' in 'markup->string' from markup.scm

https://codereview.appspot.com/324140043/diff/1/scm/markup-macros.scm
File scm/markup-macros.scm (right):

https://codereview.appspot.com/324140043/diff/1/scm/markup-macros.scm#newcode113
scm/markup-macros.scm:113: (defmacro*-public markup-lambda
Not sure I understand what 'markup-(list-)lambda' (as an "unregistered"
markup command) is used for.
Is it for use (as an example) in on-the-fly?
It's public, so if a user could use it directly, why not document it or
at least put up a regtest.
Or am I totally wrong?

https://codereview.appspot.com/324140043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
dak
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Allow defining markup commands in LilyPond syntax (issue 324140043 by dak@gnu.org)

dak
In reply to this post by lemzwerg
Reviewers: lemzwerg, thomasmorley651,

Message:
On 2017/07/30 15:41:56, thomasmorley651 wrote:
> If I understand correctly the main advantage for the user will be the
> possibility to define custom markup-commands with LilyPond-syntax.
Collecting a
> subset of preexisting commands to substitute tedious coding/typing.
> P.e.
> \markup my-name = \markup \with-color #red \rotate #90 \italic
\fontsize #3 \etc
> and use it as
> \markup \my-name "My text!"

Well, that is already possible as

my-name-markup = \markup \with-color #red \rotate #90 \italic \fontsize
#3 \etc

However, this does not give you the make-my-name-markup convenience
function and
consequently does not work in the markup macro.  Also, the user needs to
"mangle"
the name for the markup command himself by appending "-markup" which is
ugly.

> Whereas markup-commands with completely new functionality would need
to be
> defined as before:
>   (define-markup-command (command-name layout props args...)
>     args-signature
>     [ #:category category ]
>     [ #:properties property-bindings ]
>     documentation-string
>     ..body..)

Basically yes, like with \etc for music functions.

> I should download/apply this patch. After having played around with it
I'll
> probably be able to say more.

> For now two remarks/questions:


https://codereview.appspot.com/324140043/diff/1/scm/document-markup.scm
> File scm/document-markup.scm (right):


https://codereview.appspot.com/324140043/diff/1/scm/document-markup.scm#newcode96
> scm/document-markup.scm:96: (set! all-markup-commands (sort!
all-markup-commands
> markup-name<?))
> Looks like 'all-markup-(list)-commands' could be used to simplify
> 'all-relevant-markup-commands' in 'markup->string' from markup.scm

Good grief.  What is an expensive calculation like that doing inside of
a recursive function definition?  When checking the respective condition
as-needed would be so much faster than checking the weirdly stated
condition

      ((member (car m)
               (primitive-eval (cons 'list
all-relevant-markup-commands)))

This looks like a remnant of debug code?  Can't really tell.

> https://codereview.appspot.com/324140043/diff/1/scm/markup-macros.scm
> File scm/markup-macros.scm (right):


https://codereview.appspot.com/324140043/diff/1/scm/markup-macros.scm#newcode113
> scm/markup-macros.scm:113: (defmacro*-public markup-lambda
> Not sure I understand what 'markup-(list-)lambda' (as an
"unregistered" markup
> command) is used for.
> Is it for use (as an example) in on-the-fly?
> It's public, so if a user could use it directly, why not document it
or at least
> put up a regtest.
> Or am I totally wrong?

Basically,

\markup blabla = #(markup-lambda (...) ...)

is the same as
#(define-markup-command (blabla ...) ...)

That's all.  You are right that it would make sense to generally replace

\on-the-fly #(lambda (layout props whatever) ...)

with

$(markup-lambda (layout props whatever) (markup?) ...)

However, I am not really sure why we don't have all the \on-the-fly
purposed functions as proper markup functions in the first place.  Some
sort of scoping problem?

I think it would be too invasive to change that for 2.20 though.
Something to keep on the agenda.

Description:
Allow defining markup commands in LilyPond syntax

This works with assignments of the form

     \markup with-red = \markup \with-color #red \etc

or similar.  The resulting definition (in addition to being available
as \with-red command inside of markups) can be used with the `markup'
macro and also gets a `make-with-red-markup' convenience function.


Also contains commits:

markup-partial regtest: use \markup function assignment



Changes: show \markup xxx = ... \etc assignments



Parser: let `assignment_id' return a symbol

That's actually what's needed rather than a string.



Split off `markup-lambda' from `define-markup-command'

Also markup-list-lambda from define-markup-list-command.



Let `make-markup' fetch the signature itself

Also don't export it from markup-macros.scm: it is an internal function.



Reorganize markup commands to use object properties

This loosens the ties between the actual markup function and its
calling methods.

Please review this at https://codereview.appspot.com/324140043/

Affected files (+215, -176 lines):
   M Documentation/changes.tely
   M input/regression/markup-partial.ly
   M lily/include/lily-imports.hh
   M lily/lexer.ll
   M lily/lily-imports.cc
   M lily/parser.yy
   M lily/text-interface.cc
   M scm/document-markup.scm
   M scm/markup-macros.scm



_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Loading...