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

Issue 545590045: Simplify define-markup-[list-]command-internal,

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by hanwenn
Modified:
1 month, 1 week ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Simplify define-markup-[list-]command-internal, This reverts additions made in commit d2199b0163c33bcb7504c87e57eefbea93e08c88 "Issue 5167/3: Split off `markup-lambda' from `define-markup-command'" In this commit, extra support for the case where command-and-args is empty was added, ie. - (let* ((command (car command-and-args)) - (args (cdr command-and-args)) + (let* ((command (if (pair? command-and-args) + (car command-and-args) + command-and-args)) + (args (and (pair? command-and-args) (cdr command-and-args))) However, markup commands are functions that are always called with arguments 'layout' and 'props', so there can never be a case that 'args' would be empty.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -18 lines) Patch
M scm/markup-macros.scm View 2 chunks +9 lines, -18 lines 0 comments Download

Messages

Total messages: 7
hanwenn
David, this goes on top of https://codereview.appspot.com/555300043/ maybe I am missing something, but the regtest ...
1 month, 1 week ago (2020-02-16 20:37:34 UTC) #1
dak
On 2020/02/16 20:37:34, hanwenn wrote: > David, this goes on top of https://codereview.appspot.com/555300043/ > > ...
1 month, 1 week ago (2020-02-16 20:51:27 UTC) #2
dak
> In this commit, extra support for the case where command-and-args is empty was added, ...
1 month, 1 week ago (2020-02-16 20:57:54 UTC) #3
hanwenn
On Sun, Feb 16, 2020 at 9:57 PM <dak@gnu.org> wrote: > > In this commit, ...
1 month, 1 week ago (2020-02-16 21:23:56 UTC) #4
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Sun, Feb 16, 2020 at 9:57 PM <dak@gnu.org> wrote: ...
1 month, 1 week ago (2020-02-16 21:40:17 UTC) #5
dak
On 2020/02/16 21:23:56, hanwenn wrote: > On Sun, Feb 16, 2020 at 9:57 PM <mailto:dak@gnu.org> ...
1 month, 1 week ago (2020-02-16 22:12:24 UTC) #6
hanwenn
1 month, 1 week ago (2020-02-17 09:27:35 UTC) #7
On Sun, Feb 16, 2020 at 10:40 PM David Kastrup <dak@gnu.org> wrote:

> >> > In this commit, extra support for the case where command-and-args is
> >> empty was added, ie.
> >>
> >> That characterisation is completely wrong.  The support is not for the
> >> cases "where command-and-args is empty" but rather where
> >> command-and-args is not a list but a single symbol.  Just like
> >>
> >> (define symbol value)
> >> and
> >> (define (symbol arg1 arg2) body...)
> >>
> >
> > I'm trying to get the markup macros working with GUILE 2.x compilation,
> > which means that all calls to module-define! have to go.
> >
> > As a prelude to that, I'm trying to understand what this code is trying
> to
> > do.
> >
> > I don't think supporting
> >
> >   (define-markup-command sym1 sym2)
> >
> > is very useful, but if you really want to have this, it should be
> > documented and tested.
>
> It is not as much (define-markup-command sym1 sym2) rather than
>
> (define-markup-command sym1 (markup-lambda (...))
>
> where the markup-lambda expression is independently created.
>
> This was supposed to be the implementation used in parser.yy for things
> like
>
> \markup big-red = \markup \big \with-color #red \etc
>
> where defined name and definition are separate entities.  It turned out
> that calling macros from C did not work well, so I instead I had to call
> the non-macro internals.  It still made no sense scrapping that
> functionality since it is similarly useful from Scheme where it looks
> decidedly more natural than using the internals.
>
> It would appear I am not alone in considering that useful:
>
> git blame d03a3486639de982cfb6c6c315d76039bc5a0ac5 ly/markup-init.ly
>
> shows Nicolas Sceaux already implementing this ability independently in
> 2006.
>

I'll take the blame for not having reviewed it carefully enough. This
functionality was undocumented and untested in 2006, and I bet that I
didn't understand it back then. I had a hard time keeping up with Nicolas'
mastery of Scheme macros.

I propose that if we keep it, it gets commented, documented and tested.
Could you propose change to do so?

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen
Sign in to reply to this message.

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