[doorgan]

Spitfire.parse_with_comments("@x ]") crashes instead of returning an error:

** (CaseClauseError) no case clause matching: {:error, :no_fuel_remaining}
    (spitfire 0.2.1) lib/spitfire.ex:174: Spitfire.parse_with_comments/2
    iex:27: (file)

This fixes that, and other very similar errors related to stray closing delimiters

[mhanberg]

I'm not sure this exactly the right fix, as the error cases showing 'unknown token: eot' is exposing an implementation detail that isn't a real syntax error.

I think you can maybe split the struct error into its own PR.

I'll dig into it more, how does the tokenizer actually tokenize that special << character?

[doorgan]

I'll dig into it more, how does the tokenizer actually tokenize that special << character?

As [171] which corresponds to "«", which seems correct

I think you can maybe split the struct error into its own PR.

Will do. Also for that one I'm not 100% either that the behavior I chose is the one we really want either, we can discuss further there

[mhanberg]

I merged #67 which fixes the weird character issue.

I need to go to bed, but have another fix in flight for the other error.

The issue is that it should detect if there is a closing bracket/brace/paren when it wasn't expected one, report the error, and continue parsing.

If you want to look into it tomorrow during the day, it'll be around the validate_peek function and look at those "is_list", etc variables in the parse_expression function. Otherwise i can tackle it tomorrow evening.

[doorgan]

@mhanberg it seems the issue was caused by parse_program not advancing the parser if a sub-parser returned before a closing delimiter token. A closing delimiter is never a legal token to start parsing, so:

• In code like @x ] it entered an infinite loop, because it would never advance the parser and it'd try to parse the same token over and over
• In code like Foo.bar(baz)} the sub-parser would consume Foo.bar(baz) but still keep the parser at the closing ), causing it to report "unexpected token" for both ) and }

[doorgan]

One thing that caught my attention is that we have a test with this snippet(doesn't drop the cursor node):

%{state |
          foo: s
        __cursor__()
        ,
          bar: Foo.Bar.load(state.foo, state.baz)}

This would incorrectly report an unexpected ) for the Foo.Bar.load call before this fix, but then I noticed that after parsing, it gets recovered to this:

%{state | foo: s}
s(__cursor__())
nil
Foo.Bar.load(state.foo, state.baz)
nil
nil

Essentially we:

1. Find the unexpected __cursor__() and close the map
2. Extract the following lines from that map
3. The bar: part doesn't make sense outside of a map/kw list, so we drop it

I think dropping bar: there makes sense, but shouldn't the second line be just __cursor__()? The s is already preserved inside the map under the foo key

[mhanberg]

One thing that caught my attention is that we have a test with this snippet(doesn't drop the cursor node):

%{state |

          foo: s

        __cursor__()

        ,

          bar: Foo.Bar.load(state.foo, state.baz)}

This would incorrectly report an unexpected ) for the Foo.Bar.load call before this fix, but then I noticed that after parsing, it gets recovered to this:

%{state | foo: s}

s(__cursor__())

nil

Foo.Bar.load(state.foo, state.baz)

nil

nil

Essentially we:

1. Find the unexpected __cursor__() and close the map

2. Extract the following lines from that map

3. The bar: part doesn't make sense outside of a map/kw list, so we drop it

I think dropping bar: there makes sense, but shouldn't the second line be just __cursor__()? The s is already preserved inside the map under the foo key

Probably best to discuss in a separate issue about improving error recovery.

[mhanberg]

@doorgan please rebase this and update the description if needed. I think i lost track of what this is fixing because we broke other parts into other PRs