Promote R "parseError"
errors to ParseResult::SyntaxError
#840
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #598
Closes #722
@lionel- I do want to chat about this one with you
Some caveats and things to think about:
``
is not a"parseError"
. It's a regular R error that gets thrown from the parser when R tries to callinstall()
on that symbol, andinstall()
throws its own R error. So we still don't capture it with this, which stinks.42 + _
and1 |> {}
and"\s"
now, which are all"parseError"
s, yay!"parseError"
class was only added in R 4.3+, so I expect some test failures for the moment in our 4.2 check."parseError"
class is a structured error class that does havelineno
as an attribute, so if we had a way to provide a custom handler totry_catch()
, then we could pluck out information we care about from there, and that way we would not have to makeline
optional. Right now it just gives us themessage
and theclass
(among other things) and we have no way to customize it.Some questions:
R_ParseVector()
throws any kind of error, then we should convert it to aParseResult::SyntaxError
? The intent would be to capture the``
case, and any other case we don't yet know about. One thing to be careful of is that incheck_console_input()
we expect that if we saw aParseResult::SyntaxError
, then we can still just write the input to R's buffer, because we expect that R is going to error on that input "normally". I would not want to make a mistake there. But if we could do this then we'd support R 4.2 better too (I'm not too worried about it if we can't, just saying).attempt to use zero-length variable name
that we promote toParseResult::SyntaxError
, but that doesn't feel great either.