handle syntax errors more gracefully #34
Conversation
aaronjanse
changed the title
| let end = self.finish_error_node(); | ||
| self.errors.push(ParseError::Expected( | ||
| TextRange::new(start, end), | ||
| "attribute".to_string(), |
There was a problem hiding this comment.
I guess we should reflect here that strings etc. are also OK.
There was a problem hiding this comment.
What should be the exact message used? Maybe attribute or string? Although I guess a string does represent an attribute here? Please correct me if I'm wrong
|
I pushed a somewhat significant change to behavior. The change matters only for attr path now. I also added an "expected semicolon" error node and updated tests |
aaronjanse
changed the title
|
You need to run this with a fuzzer. Adding better error handling can easily screw up the parser, and I got an out of memory error relatively quickly. I would prefer more general error handling (removing |
|
Wow, good catch @oberblastmeister. The fuzzer is amazing, and after seeings its result, I agree that we need a more general solution here. I'll start working on that. Thank you!! |
|
@aaronjanse I already did a lot of work here #35. Also, I really think we should work on hooking up the new version of |
Oh, sorry about that! I'm still catching up after being away for a while. If you submit a PR for the error handling that was in #35, I'll close #120 in favor of your PR.
As in, you think we should update rnix-lsp to use the latest nix-parser version? I'd be willing to work on doing that |
Yes! I think that would be great. |
|
Sounds good, will do! |
Fixes #33.
I don't know enough about
rowanto cleanly improve error handling, but I thought I'd at least share the patch I've been using locally while implementing rnix-lsp autocomplete.Feel free to re-use parts of this patch for a different PR, push directly to this PR, or something else.