Re: Make music functions callable from Scheme (issue 244840043 by address@hidden)

[dak]

On 2015/06/08 15:58:31, Keith wrote:
> On Sun, 07 Jun 2015 23:09:31 -0700, <mailto:address@hidden> wrote:

> > So your problem appears to be that you understand the code but it
does
> > something you did not expect.  Cross-check from the user end with
the
> > "Extending LilyPond" guide's explanation of optional arguments.

> It is Extending section 2.2.2.  Yuck.  A comment pointing to
Extending-2.2.2
> would help.

> The behavior in the parser is forced because LilyPond uses no
delimiters for the
> arguments to a function.  Scheme has delimiters so you know where the
end of the
> argument list is, so we could use a simpler rule for optional
arguments in
> Scheme interface.

I did in version 2 of the patch set.  Then I reconsidered.  The reason I
reconsidered is that extending some LilyPond command often may involve
adding obscure optional arguments, like context modifications.  Users
will rarely use those, and they don't disturb existing usage when their
type has no overlap with the type of the argument following it.  So if
we use a strictly positional interface in Scheme calls, people will not
be able to translate their LilyPond function calls into Scheme function
calls with confidence.

When I converted about 5 trivial #{ \... #} calls in
ly/music-functions-init.ly to Scheme, I was worried I may have forgot
about some optional argument I would have needed to specify.  This seed
of doubt and lack of forward-compatibility of functions that don't yet
have an optional argument does not seem like it would be doing anybody a
favor.

> Or you could scan arguments using a recursive-descent method in C, so
the
> parallel with the parser is maybe a bit more clear.

Sorry, but no.  This implementation follows the plain language
explanation in the documentation.  And it does this well enough that you
picked up what it did from the code without much apparent difficulty and
complained that it did not match what you thought the code in the parser
did.  So you want to have code that you understood changed to match code
you did not understand.  That's not going to help anybody.  If anything,
it should be the parser code that should be matched to the C code in
order to become comprehensible, but that's basically out of our hand
since we are limited by the expressivity of the Bison grammar.  The
parser has extensive comments laying out the basic problem of the
argument parsing and how it is expressed in the grammar.  That's not the
same as having the actual grammar/code being simple and straightforward
but it's the best we can do as I see it.

> Or maybe advance through 'signature' to the next non-optional argument
> immediately upon using a default value, rather than setting
'skipping'.

I thought about that, yes.  It would give us one outer loop that goes
through actual argument list and signature in lockstep, and one inner
loop that goes through signature only.  I think that I tried this
already and that it did not really work out well.  I'll give it another
attempt but I doubt I'll come up with something better.

> The re-structuring I suggested was intended also to change the if-else
branching
> so that you don't need the 'continue's.

I prefer early continue, break and return statements to nested two-way
conditionals.  A "continue;" clearly spells out "end of loop, you don't
need to further think about this code branch" while "} else {" can still
have relevant loop parts following after the matching "}".

So reducing the number of "continue" statements is definitely not an
attraction for me.  Reducing duplicated code before such statements
might be somewhat more interesting.

https://codereview.appspot.com/244840043/