Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(514)

Issue 324140043: Allow defining markup commands in LilyPond syntax (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 8 months ago by dak
Modified:
5 years, 8 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

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.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+215 lines, -176 lines) Patch
M Documentation/changes.tely View 1 chunk +1 line, -1 line 0 comments Download
M input/regression/markup-partial.ly View 1 chunk +5 lines, -5 lines 0 comments Download
M lily/include/lily-imports.hh View 2 chunks +3 lines, -0 lines 0 comments Download
M lily/lexer.ll View 1 chunk +1 line, -2 lines 0 comments Download
M lily/lily-imports.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M lily/parser.yy View 5 chunks +57 lines, -8 lines 0 comments Download
M lily/text-interface.cc View 1 chunk +2 lines, -7 lines 0 comments Download
M scm/document-markup.scm View 5 chunks +51 lines, -27 lines 1 comment Download
M scm/markup-macros.scm View 5 chunks +92 lines, -126 lines 1 comment Download

Messages

Total messages: 3
lemzwerg
By mere inspection: LGTM
6 years, 8 months ago (2017-07-28 19:10:35 UTC) #1
thomasmorley651
If I understand correctly the main advantage for the user will be the possibility to ...
6 years, 8 months ago (2017-07-30 15:41:56 UTC) #2
dak
6 years, 8 months ago (2017-07-30 16:07:17 UTC) #3
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#newco...
> 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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b