Re: A few questions regarding markup

[Carl Sorensen]

On 11/15/09 11:15 PM, "David Kastrup" <address@hidden> wrote:

> Carl Sorensen <address@hidden> writes:
> 
>> On 11/14/09 1:29 AM, "David Kastrup" <address@hidden> wrote:
>> 
>>> 
>>> 
>>> Ok, I am digging through harp-pedals.scm and looking at
>>> define-builtin-markup-command.
>>> 
>>> Now from the definition of define-builtin-markup-command it looks to me
>>> like you can specify default properties and those are let to the
>>> specified default values (#f if unspecified) or the respective
>>> properties from the list.
>>> 
>>> Now the harp-pedal command defines the property signature
>>> 
>>>   ((size 1.0)
>>>    (harp-pedal-details)
>>>    (thickness 0.5))
>> 
>> The property signature is a documentation-only convention.  It has no
>> functionality except for producing documentation.
> 
> Thanks, but wrong.  Please look in the definition of
> define-builtin-markup-command.  The property signature symbols are
> let-bound to the respective property values (or the default if none are
> there) within the body of define-builtin-markup-command.

OK, not the first time I've been wrong.

> 
> Here is the respective part from the macro that does it:
> 
>               `(define-public (,command-name ,@args)
>                  ,documentation
>                  (let ,(filter identity
>                                (map (lambda (prop-spec)
>                                       (if (pair? prop-spec)
>                                           (let ((prop (car prop-spec))
>                                                 (default-value (if (null? (cdr
> prop-spec))
>                                                                    #f
>                                                                    (cadr
> prop-spec)))
>                                                 (props (cadr args)))
>                                             `(,prop (chain-assoc-get ',prop
> ,props ,default-value)))
>                                           #f))
>                                     properties))
>                    ,@real-body)))
> 
> 
> If indeed no active Lilypond developer _realises_ that this is the case,
> then the respective code should just be thrown out instead of wasting
> performance with property lookups that are ignored.

So there is some history here.  Originally there was no
define-builtin-markup-command; all markups were defined as
define-markup-command, where chain-assoc-get was used, and defaults were put
in the chain-assoc-get call.

Then the documentation code was added, and define-builtin-markup-command was
added.  I never looked too closely at the code that was added.  Sorry about
the mistake.

So, IIUC, the let-binding actually doesn't do anything, because the code in
the body of the function overwrites it with its own chain-assoc-get call.
Is that right?

> 
> I find it unfortunate that the arguments of
> define-builtin-markup-command and define-markup-command diverge in this
> manner, since converting a markup routine to an internal one _in style_
> should just entail adding the documentation, not restructuring the code.

I agree here.  But from my (apparently clueless) point of view, that was
fundamentally what happened.  In order to convert from define-markup-command
to define-builtin-markup-command, two arguments were added: a documentation
category and a default properties list.

> 
> The idea to have properties automatically defaulted and looked up at the
> top is nice, however.  But if the lookup results are needed in a
> different scope (as is the case with harp pedals) or only conditionally
> in some code paths, the scheme gets less workable.

So what if, instead of having define-builtin-markup-command do the let
binding for the properties, we just had it add those property defaults on
the end of the props argument?  That would allow any functions written to
work with define-markup-command to use chain-assoc-get as normal, and
defaults that were added by define-builtin-markup-command would replace
those in the scheme function (because the values would be added to props, so
chain-assoc-get in the function would never get the default value).

It would seem to me like this is doable (although I haven't tried it).
> 
> And if nobody realises what the functionality actually does, this does
> not help.
>

I agree.

Thanks,

Carl