jcoglan / canopy

A parser compiler for Java, JavaScript, Python, Ruby
http://canopy.jcoglan.com
Mozilla Public License 2.0
418 stars 54 forks source link

Error thrown for some rules with single elemets #32

Closed nasser closed 2 years ago

nasser commented 6 years ago

These work:

grammar test
number <-  [0-9]? "foo" %action
grammar test
number <-  [0-9]+ %action
grammar test
number <-  [0-9]?

But this fails with a grammar parsing error

grammar test
number <-  [0-9]? %action
$ canopy grammar.peg
Line 2: expected [\s], "#", [a-zA-Z_], "(", "&", "!", '"', "'", "`", "[", ".", "<", "/"
number <-  [0-9]? %action
                  ^
SyntaxError: Line 2: expected [\s], "#", [a-zA-Z_], "(", "&", "!", '"', "'", "`", "[", ".", "<", "/"
number <-  [0-9]? %action
                  ^
    at Parser.parse (/usr/lib/node_modules/canopy/lib/canopy.js:2788:11)
    at Object.parse (/usr/lib/node_modules/canopy/lib/canopy.js:2794:19)
    at Canopy.Compiler.parseTree (/usr/lib/node_modules/canopy/lib/canopy.js:4527:20)
    at Canopy.Compiler.toSource (/usr/lib/node_modules/canopy/lib/canopy.js:4539:10)
    at Object.compile (/usr/lib/node_modules/canopy/lib/canopy.js:19:21)
    at Object.<anonymous> (/usr/lib/node_modules/canopy/bin/canopy:25:24)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
jcoglan commented 4 years ago

@nasser Sorry it took me so long to respond to you. There's a reason this doesn't work but it's also prompted me to add support for the thing you're trying to do.

Actions are for constructing new nodes, i.e. instead of Canopy constructing its own parse tree nodes, it delegates to your action to do that. Therefore they only apply to rules that make new nodes which are:

They don't apply to the ? operator because this doesn't construct any nodes itself -- the expression before it constructs a node, and the ? operator just means it's fine if that expression doesn't match anything.

So for example in ("hello world")?, the terminals "hello" and "world" each construct a node, and the sequence ("hello" "world") constructs a new node by combining those, but then the ? operator just passes the sequence node through rather than building any nodes itself. The result of expr? is either the node that expr creates, or nothing.

However I think there is possibly an ergonomic benefit in making expr? %action work, and based on your examples I think it would make sense to interpret it as (expr %action)?. That is, action is used to build the node for expr if it matches, and then ? returns the result if any.

Does that make sense? I've just pushed ca4041d which implements this functionality and I'd value your feedback.

byteit101 commented 3 years ago

I just ran into this too. The ca4041d commit solves some of the problems, but my brief attempts to fix the others didn't work as I expected.

I have a rule of the form:

grammar test
root <- number %action2 
number <-  "NaN" %action1

I was able to get it to parse last night by a similar approach to ca4041d, but not to generate a call to action2.

byteit101 commented 3 years ago

I managed to figure out an approach to solve this that will be fixed by #49

jcoglan commented 2 years ago

I'm going to close this because I believe the central problem identified in this issue (making %action work with rule?) has been addressed. Anyone that wants to discuss expanding the scope of this to references should comment on #49.