Re: Implement optional music function arguments (issue 5023044)

[dak]

http://codereview.appspot.com/5023044/diff/2001/lily/parser.yy
File lily/parser.yy (right):

http://codereview.appspot.com/5023044/diff/2001/lily/parser.yy#newcode1184
lily/parser.yy:1184: | EXPECT_MARKUP EXPECT_OPTIONAL function_arglist
function_markup_argument {
On 2011/09/15 10:45:11, Reinhold wrote:
> Can't we shorten this long list of alternatives somehow?

Not really.  You need to combine each argument type x with a list of
argument types different from x.  So each of the n(n-1) combinations has
no constituents shared with the other constituents.  You could factor
out parts of that list.  Then you have n different factors for which you
need a good fitting name each.  And the amount of rule lines increases
even more, and there is no real advantage in readability because there
is not much of a system except you need to cover the O(n^2) cases.

This is definitely the ugly part of the patch.  It will not
significantly affect performance, thanks to Bison and LALR(1), but it is
a headache to read.  And I see no way around it.

http://codereview.appspot.com/5023044/diff/2001/scm/music-functions.scm
File scm/music-functions.scm (right):

http://codereview.appspot.com/5023044/diff/2001/scm/music-functions.scm#newcode762
scm/music-functions.scm:762: "Helper macro for `ly:make-music-function'.
On 2011/09/15 10:45:11, Reinhold wrote:
> It's also a helper for ly:make-scheme-function...

There is no ly:make-scheme-function, so it can't help it.  All syntactic
functions are created with ly:make-music-function.

While I have some leaning towards define-lily-function (as it takes Lily
arguments), this is not actually anything introduced by this patch
(rather part of the define-scheme-function patch series).  So if you
want different names/doc strings, you should file them as a separate
issue.

http://codereview.appspot.com/5023044/diff/2001/scm/music-functions.scm#newcode792
scm/music-functions.scm:792: "
On 2011/09/15 10:45:11, Reinhold wrote:
> Here you should add a description how optional arguments are given! In
> particular, the argX-type? is no longer valid in general.

Yes to the first, not quite to the second.  The argX-type? remains
valid.  Anybody have a good suggestion what to name the parameters in
the DOC string?  They can be either predicate? or (predicate? default)
for an optional parameter taking a default value.  The default is
evaluated at definition time, so using #{...#} and similar in the
defaulsts does not impact execution time.

http://codereview.appspot.com/5023044/