Re: Make define-builtin-markup{, -list}-command #:category #:properties keywords (issue160048)

[David Kastrup]

address@hidden writes:

> On 2009/12/17 11:23:04, dak wrote:
>
>> Well, patch set 7 does that, but after rethinking this, I suppose I'd
>> call it a mistake.  Even if the option gets implemented for other
>> constructs than markup, the time and memory savings are likely
>> negligible.  Checking for the option is likely causing more overhead
>> than registering some markup.
>
> Another way to achieve this would have been to do something like
>
> (define index-markup-command
>   (if (ly:get-option 'make-index)
>       (lambda ...) ;; return code that indexes the commands
>       (lambda ...))) ;; return empty code
>
> and then call the index-markup-command function in the
> define-markup-command.

I've been thinking about it, also about evaluating the ly:get-option
part in the macro instead of outside.  However, I thought that _if_ the
user wanted to index his own markups, calling ly:set-option would no
longer work, and when using -dmake-index, he would automatically get
Lilypond's default markups indexed as well.

In short: messing with the behavior became more intricate and less
predictable, at a very modest saving (if at all) of memory and
performance.

>> So my take would be to apply patch set 6, and possibly file a TODO
>> item "just index markup, macros, whatever on documentation runs" with
>> low priority that may refer to this discussion and/or patch set 7.
>
> OK. This issue is not crucial anyway, so I'll commit patch 6 (+ remove
>markup-init.ly and its include in declarations-init.ly)

Oops.  I was of the opinion that I had done this already.  In my git
archive, it is a separate commit, but you are right that it does not yet
appear in patch set 6.  Sorry, I thought I had factored this better on
Rietveld, but it is rather trivial.

-- 
David Kastrup