Simplify define-markup-[list-]command-internal,

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.

[hanwenn, 2020-02-16 20:37:34]

David, this goes on top of https://codereview.appspot.com/555300043/

maybe I am missing something, but the regtest completes successfully. Why did
you add this stuff?

[dak, 2020-02-16 20:51:27]

On 2020/02/16 20:37:34, hanwenn wrote:
> David, this goes on top of https://codereview.appspot.com/555300043/
> 
> maybe I am missing something, but the regtest completes successfully. Why did
> you add this stuff?

To allow for making aliases etc.  A markup command definition involves a number
of things.  It registers with the markup macro, it creates a make-...-markup
command, it creates a ...-markup definition.  So a bunch of bindings need to get
created.  markup-lambda does not do all that.  What it does is register the
signature.

So you can in effect do
(define-markup-command blah (markup-lambda ...)) (or use the respective part of
an existing markup function or have an autogenerated markup-lambda definition)
and have all the required bits for putting a markup command into the name space.

Is there a particular reason you want to see this functionality killed?

[dak, 2020-02-16 20:57:54]

> 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...)

are both possible.

[hanwenn, 2020-02-16 21:23:56]

On Sun, Feb 16, 2020 at 9:57 PM <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.

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

[dak, 2020-02-16 21:40:17]

Han-Wen Nienhuys <hanwenn@gmail.com> writes:

> On Sun, Feb 16, 2020 at 9:57 PM <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.

-- 
David Kastrup

[dak, 2020-02-16 22:12:24]

On 2020/02/16 21:23:56, hanwenn wrote:
> On Sun, Feb 16, 2020 at 9:57 PM <mailto: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.

No, it means that all calls to module-define! from within a macro have to go. 
Here the macro is already split into the part not trying to evaluate symbols and
argument lists, and a part (the "internal" one) that is executed at
interpretation time where modules are in working order.  As long as you can
specify the _name_ of the internal in a manner where it is found at
interpretation time (since specifying its _value_ does not work for the byte
compiler) you should be fine.  An example of how to do that is in
scm/parser-ly-from-scheme.scm currently:

    (list (if (guile-v2)
              '(@@ (lily) read-lily-expression-internal)
              read-lily-expression-internal)
          lily-string filename line (cons 'list (reverse! closures)))))

It's not really clear how this will play with -dsafe but let's fix stuff one
point at a time.

[hanwenn, 2020-02-17 09:27:35]

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