Errors once #152
Merged
Errors once #152
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… the same time
The current implemnetation of the parser calls the token matcher multiple times
to identify the same input line (e.g. in a 'lookahead' or as part of the state
machine ('match_token_at_*'). When an invocation causes an error (e.g. because
there are tags with embedded spaces, or the selected language isn't available),
the parser always stashes the error in the list of errors. Since the matcher
may be invoked multiple times, the error may get stashed multiple times. To
fix the user experience, Ruby, Perl, Python and others de-duplicate the reported
errors by error message.
This change moves stashing of errors to the point where the token is accepted into
the AST, which means that multiples of the same error are discarded when that makes
sense. E.g. the lookahead discards the errors and *only* looks at whether there
is a match with the desired token type.
Note: The structure of returning a boolean accepting the line and a second element
indicating an error, adds the currently-missing capability of saying 'yes, this is
what you're looking for, but there are problems with it'.
Move the responsibility to check that there even *is* an input line to match, to the token matcher. Now that the parser largely isn't responsible for it anymore, we can do away with a whole slew of equally-named wrapper functions in the parser that already exist in the token matcher. Not having two sets of functions by the same name helps to navigate the code.
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🤔 What's changed?
The Perl token matcher gained the ability to accept a token and at the same time indicate a token parser error. Before - due to the use of exceptions to indicate parser errors - it would only be possible to achieve one or the other.
⚡️ What's your motivation?
By making this change, the Perl implementation is able to report the parsing error exactly once, instead of - as it does now - requiring to de-duplicate errors which might be reported multiple times (this can happen due to the fact that the lookahead function of the parser triggers a parser error.
🏷️ What kind of change is this?
🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)
Since the error of the duplicates has already been fixed (the fast way), PR is more of a maintenance type of PR.
♻️ Anything particular you want feedback on?
During the weekly meeting, we assessed that exceptions are not the way to return data from a function.
📋 Checklist: