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

[nicolas . sceaux]

Hi,

When defining a user markup command, it would be better not to modify at
all the variables from the (lily) module, even if you took care of the
memory leak. Also, there is one file to be deleted, and the associated
\include to be removed from an init .ly file.

Nicolas


http://codereview.appspot.com/160048/diff/4008/6002
File ly/markup-init.ly (left):

http://codereview.appspot.com/160048/diff/4008/6002#oldcode5
ly/markup-init.ly:5: %%;; to be define later, in a closure
If this file is empty, then it shall be deleted

http://codereview.appspot.com/160048/diff/4008/6007
File scm/markup.scm (right):

http://codereview.appspot.com/160048/diff/4008/6007#newcode101
scm/markup.scm:101: (set! body (cddr body)))
Why is this needed?
Could #:allow-other-keys take care of that?

http://codereview.appspot.com/160048/diff/4008/6007#newcode131
scm/markup.scm:131: ;; Register the new function, for markup
documentation
I still have the feeling that user defined commands should not modify
variables from (lily) module.
Could there be e.g. a module check, so that the macro expand into the
documentation related settings only for builtin commands?

http://codereview.appspot.com/160048/diff/4008/6007#newcode168
scm/markup.scm:168: (set! body (cddr body)))
Same as above.

http://codereview.appspot.com/160048/diff/4008/6007#newcode199
scm/markup.scm:199: (hashq-set! markup-list-functions ,command-name #t)
Same as for define-markup-command

http://codereview.appspot.com/160048