Skip to content

Comments

Location aware error context#410

Draft
urso wants to merge 3 commits intowinnow-rs:mainfrom
urso:loc-ctx-error
Draft

Location aware error context#410
urso wants to merge 3 commits intowinnow-rs:mainfrom
urso:loc-ctx-error

Conversation

@urso
Copy link

@urso urso commented Jan 1, 2024

Enhancement Request: #409

This PR implements the LocationContextError. The implementation follows the ContextError type:

pub struct LocationContextError<L = usize, C = StrContext> {
   ...
}

Main differences to ContextError:

  • we add a pos field to the LocationContextError.
  • ParserError.or will choose the context based on the longest parse. We merge the contexts into a common Vec in case both errors did reach the same position in the input stream
  • add_context will discard context details based on the input location

Although Location.location returns an usize, this type is not always appropriate in all situations. For example in one of my parsers I have a lexer phase and some parse steps that just recognize a sequence of tokens. The actual parsing happens later. For that reason I'm having a struct Span(Range<u32>) as location.

In order to make the location type configurable we allow users to set the location type via L.

Note: I didn't want to modify the Location trait to make the output type configurable yet. Although I think that would improve the usability of LocationContextError. This is why ParserError and AddContext are implemented for usize only. But their implementations can easily be copied for custom location types. The helper methods to merge/add context information only require L: Ord.

Testing:
I didn't find tests for the other error types. So I didn't add new ones.

@urso urso mentioned this pull request Jan 1, 2024
2 tasks
@urso
Copy link
Author

urso commented Jan 1, 2024

Update:
I removed the L location type parameter from LocationContextError. We store the position as usize and require the input stream to implement Location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant