Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

Issue 5339043: parser.yy: allow postevents in function arguments in general (Closed)

Can't Edit
Can't Publish+Mail
Start Review
7 years, 7 months ago by dak
7 years, 7 months ago


parser.yy: allow postevents in function arguments in general This change is on top of the dev/syntax branch still in review. It removes the "polymorphism" of music functions used explicitly as events. The previous behavior was such that trailing arguments of kind ly:music? were parsed as postevents instead of music, so that \tweak had a chance to be implemented as a music function. This concept has become increasingly fishy when ly:music? predicates where treated just like any old Scheme predicate. Event functions (and music functions used as events) have not inherited the new power of music function argument lists. Doing that with something akin to the previous semantics would have required a large amount of code duplication since one can't easily replicate a complex _recursive_ grammar while exchanging a core element. This patch sidesteps the issue by just allowing both postevents and normal music as function arguments for both music and event functions. It passes the regtests, but there are consequences: a) a music function can't figure out whether it is used in a postevent or a music event setting by looking at the type of music argument it receives. That type is orthogonal from how its return value gets used. b) a music function might receive postevents where previously syntax errors occured. To be fair: most music functions were not bothering checking whether they might be employed in postevent settings anyway, so it is not like this adds a fundamentally new class of surprises. I am still putting it out for separate review.

Patch Set 1 #

Patch Set 2 : Rebased on current dev/staging, ready for review. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -30 lines) Patch
M lily/parser.yy View 1 8 chunks +31 lines, -30 lines 0 comments Download


Total messages: 3
Patch fails to apply to current master --snip-- jlowe@jlowe-lilybuntu2:~/lilypond-git$ patch -p1 < ../Desktop/issue5339043_1.diff patching file ...
7 years, 7 months ago (2011-11-05 11:54:16 UTC) #1
On 2011/11/05 11:54:16, J_lowe wrote: > Patch fails to apply to current master Hardly surprising. ...
7 years, 7 months ago (2011-11-05 12:02:34 UTC) #2
7 years, 7 months ago (2011-11-08 23:09:30 UTC) #3
passes make and no reg tests diffs.

Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b