Error handling by parser about gherkin2

[aslakhellesoy]

It would be great if the parser would detect if the next line didn't match what was expected. For example, if a Scenario line isn't followed by a step (ignoring comments). That sort of thing. It would be even better if it reported what line and what column it was on. And even better if it said what would have been valid input on that line (e.g. expected Given, When or Then, but found Crapola). Not sure what the best way to report errors is. Raise an exception? Invoke an error method on the listener? Not sure....

[msassak]

I like the idea of invoking an error method on the listener. That way the client code can decide whether to raise or continue, etc. I think that also jibes with the support Ragel has for handling error conditions.

[msassak]

I made a gist describing some possible error scenarios http://gist.github.com/188120, because it's hard for me to think of many.

[ghnatiuk]

I just pushed a feature to send the listener a table_error when it finds an unclosed table row. That message is sent first, folowed by a table message including all the parseable rows that are found. It's leading me to wonder, though, whether or not we should be tokenizing all the table rows rather than sending a single table message, e.g.

listener.table_start(10) #line number
listener.table_row(["hello","world", nil], 10)
listener.table_row_error("Unclosed table row", "| hello |world", 11)
listener.table_row(["goodbye", "cruel", "world"], 12)
listener.table_end(12)

or something like that. Maybe the table_start and table_end aren't even necessary.
wdyt?

[msassak]

Greg and I have been tossing around for a few days the idea of using Ragel strictly as a tokenizer, and inserting a "Syntax Policy Proxy" (or so we've been calling them) in between the Ragel parser and the listener, whose job it would be to ensure that events aren't called in the wrong order, handle error states, and pass on messages to the listener. Greg changed the parser to do that, and I merged his changes and pushed a very basic FeaturePolicy to my repo in the syntax-policies branch.

The intent, then, is to provide classes like Gherkin::Feature (or even Gherkin::CucumberFeature), which, given a language and a listener, would set everything up with the appropriate parser and policy. I like this idea a lot, not the least reason being that it would allow us to easily support use-cases like Liz Keogh brought up on the JBehave list where you have a bare scenario (or even bare steps) without any of the more complex stuff.

[aslakhellesoy]

This is entirely up to you. If you think it's a good idea, just go ahead with it. I think it sounds smart, but I don't have enough insight now to know about pros and cons.

I assume these policies would have to be implemented in each target language too? I.e. C and Java.

[msassak]

Not unless you wanted to use the parser purely from C or Java--skipping Ruby and MRI/JRuby entirely.

[aslakhellesoy]

I see, pure ruby. That's great.

[ghnatiuk]

We've got the parser sending a syntax_error message now if it finds something that doesn't make sense. Parsing effectively stops at that point. We'll probably leave the message ("Expecting Scenario, Feature, Background....") up to the listener for now, though it wouldn't be too hard to do with the policy proxy, either.

[aslakhellesoy]

Delegating that to the parser is fine for now. We can push it into the policy later if we want.

[msassak]

My master uses two exceptions now: ParsingError and FeatureSyntaxError. ParsingError is thrown by the Ragel parser when it can't parse something (literally when parsing ends in an error state rather than the expected final state). FeatureSyntaxError is thrown by the policy when event messages are received out of order. If the policy is told not to raise, #syntax_error is sent to the listener instead.