Re: A few questions regarding markup

[David Kastrup]

Carl Sorensen <address@hidden> writes:

> On 11/15/09 11:15 PM, "David Kastrup" <address@hidden> wrote:
>
>> Carl Sorensen <address@hidden> writes:
>> 
>>> 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.
>> 
>> 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 can't vouch for anything except for the harp-pedal code right now.
With the harp-pedal code, the let-binding doesn't do anything because
the body of the function just calls an external work function for the
brunt work.  And this other function indeed uses chain-assoc-get, and
would not see the let-bindings from the scope of the other function,
anyway.

Ok, I did an actual grep for uses of define-builtin-markup-command.

With very few exceptions (about 2 or 3, one being the harp-pedal code),
all the commands appear to use the let-binding mechanism.

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

My code review disagrees.  The let-binding and property lookup appears
to be quite consistently in use.

> 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 involves copying the whole property list chain (using append rather
than append!).  An efficiency problem.

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

The way it looks it would make a lot of existing code uglier.

>> And if nobody realises what the functionality actually does, this
>> does not help.
>
> I agree.

The code review does not seem to support my premise.  Obviously the
programmer(s) introducing markups with define-builtin-markup-command
have predominantly realized what this does.

-- 
David Kastrup