[DISCARDED] RFC: WIP: More detailed error reporting (reporting full failed expressions) #100
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
| # ########################################################################## | ||
| # WIP: We have doubts about this behavior. To our understanding, | ||
| # the parsing of OrderedChoice should not throw. | ||
| # ########################################################################## |
There was a problem hiding this comment.
Thanks for the contribution @mettta and @stanislaw. I think you are heading in the right direction but there is an issue that you have noticed (and was reported recently in #96) that needs to be handled before your work is integrated. I'm working on it and will do a detailed review of this PR when finished.
There was a problem hiding this comment.
Hi, I just finished the work I've mentioned. Please rebase both your PRs on the master to see if everything works.
There was a problem hiding this comment.
We will rebase in just a few minutes. We are preparing one small commit on top of this patch that makes all previous tests pass as they were in the master branch (reporting only the non-matched expressions in a non-matched expression, not the whole expression). We are fixing the remaining tests and don't know yet how long this should take.
There was a problem hiding this comment.
Excellent. Thanks a lot! Just take your time. Nothing urgent here :)
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
9 tasks
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 1, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 2, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 2, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 2, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 2, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 6, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 7, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 7, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 7, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 8, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 8, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 9, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 9, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 9, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
changed the title
|
Closing this in favour of the more advanced (and correct) #102. |
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 9, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 9, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
added a commit
to stanislaw/Arpeggio
that referenced
this pull request
Apr 10, 2023
This patch is more advanced compared to textX#100 The NoMatch class is made smarter and tracks the exact match errors. With this design, reporting an error translates to printing the diagnostics of all failed subexpressions of a failed expressions. For example, for `Sequence`, the `SequenceNoMatch` class collects the failed expression as well as all previously matched (but failed) `Optional` expressions. The naming of `*NoMatch` certainly introduces some redundancy in the class names, however, without this measure I had hard time visualizing the internal structure of the failed expression to be printed. Open question: - Is this desirable that in case of ordered choices, we could print not only the winning match but all matches that are equally possible, as in this example: ``` assert ( "Expected '4' at position (1, 15)" " or " "'five' at position (1, 20)" in str(e.value) ) ``` The advantage of this approach would be that a user sees all alternatives that are possible in cases like this. The cost: the implementation has to control whether several `at position` strings have to be presented instead of a single `at position` at the end of the inspection message.
stanislaw
changed the title
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.
Hello,
This is our (@mettta and @stanislaw) attempt to make a step towards more precise error reporting.
This slightly more simple implementation at first sacrifices some of the previous capabilities, such as "not reporting the whole failed expression, but only it's failed part". However, these capabilities can be implemented outside the
_nm_raisemethod.I would be curious to hear from you if you consider this to be going to the right direction. In the meantime, I would like to focus on implementing this capability given the direction taken in this patch:
Code review checklist
CHANGELOG.md, no needto update for typo fixes and such).