[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: A few questions regarding markup
From: |
David Kastrup |
Subject: |
Re: A few questions regarding markup |
Date: |
Mon, 16 Nov 2009 20:32:31 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) |
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