Better error messages on parse failure about angle-grinder HOT 14 CLOSED

Did you have something in mind for how to handle this? Looking at the nom error management docs, the Merr pattern matching option seemed like a reasonable solution.

from angle-grinder.

from angle-grinder.

Looked into this a bit more -- we probably want to combine early-return from errors with merr. (Although merr might be overkill, the grammar is pretty stable)

from angle-grinder.

Cool. I'm finishing up a change to add the limit operator. I can try the different approaches for errors and see how it goes after I'm done with limit.

from angle-grinder.

from angle-grinder.

The "merr" stuff in nom doesn't seem to be working anymore. The return_error!() macro seems to be sufficient, though. I'll try to put together something that handles a couple error cases, like non-terminated string and no column names for count by and open a PR.

from angle-grinder.

If you have some time, can you check out this commit on my error-wip branch and say if you okay with the direction.

Here's a screenshot of the results so far:

from angle-grinder.

(this is from a fairly brief skim on my phone, more comments later)
First off, the results are gorgeous! I never would have thought to try to use the same library that rustc uses.

A couple of thoughts:

• I'm not in love with sticking postions ad-hoc into operators. What if we did something like:

WithPosition<T> {
  position: [Positional information needed for syntax errors]
  obj: T
}

We could have the parser return a seq of Positioned<Operator>

I think I'd prefer that strategy in general (wrapping objects with their positions) rather than having positional information leak into the underlying structs. We could even push that all the way down the execution engine for generating nice error messages at some point.

I'd also probably refactor the grammar a bit to push the positioning down a little bit so that you didn't need to create start and end tokens manually in most cases.

from angle-grinder.

eg.

Limit {
  count: Positioned<f64>
}

WithPosition can define into for T to easily extract the underlying type

from angle-grinder.

Another note: If it simplifies things, I'd be totally fine only exposing the first error in a pipeline

from angle-grinder.

I don't like the position stuff either. I like your proposal, I'll see if I can make it work.

from angle-grinder.

I've updated my error-wip branch if you want to take a look again. Let me know if that's fine for a PR or if you want me to break it up or do some more changes.

I was able to get your Positioned suggestion working by adding a with_pos!() parser that wraps the result of the child parser with a Positioned object. It's a definite improvement.

Thanks!

from angle-grinder.

from angle-grinder.

Main work is done in #44 , remaining tasks are incremental, closing.

from angle-grinder.