feat(error): offset-based span tracking#50
Conversation
Use snapbox for snapshot testing for Display. This would be useful for span-aware errors.
Add an `offset` field accumulating byte length of every line consumed. This lays groundwork for attaching byte-offset spans to parse errors.
|
|
||
| /// Creates an error with a specific offset as span. | ||
| fn error_at(&self, kind: ParsePatchErrorKind, offset: usize) -> ParsePatchError { | ||
| ParsePatchError::new(kind, offset..offset) |
There was a problem hiding this comment.
While we track spans here for room for future, currently just use span.start for everything for simplicity, e.g., error parsing patch at byte 28: unable to parse hunk header.
There was a problem hiding this comment.
We could enhance and annotate the header when we have better infra for richer error display.
src/patch/error.rs
Outdated
| pub fn span(&self) -> Option<Range<usize>> { | ||
| self.span.clone() | ||
| } |
There was a problem hiding this comment.
Do we want this to be public? I suppose eventually we do want some sort of rich error data to be able to be pulled out.
There was a problem hiding this comment.
Just want your thoughts on this, and then i can get this PR merged.
There was a problem hiding this comment.
Removed. We should add it when we need it
| anstyle = { version = "1.0.13", optional = true } | ||
|
|
||
| [dev-dependencies] | ||
| snapbox = "0.6.24" |
There was a problem hiding this comment.
I haven't used snapbox before, but have used tools like cargo-insta before. I wonder if it would make sense to eventually migrate more of these tests to be snapshot based vs trying to encode these things in strs.
There was a problem hiding this comment.
Not something that needs to be tackled as a part of this PR, i'm more just considering things we could do in the future.
There was a problem hiding this comment.
I chose it because
- I am more familiar with snapbox
- It doesn't require any other CLI tool. Just
env SNAPSHOTS=overwrite cargo test. Though that also means you don't have fancy snapshot reviews. Can cut over if needed :)
New methods `error()` and `error_at()` attach the current offset so callers get location info in error messages.
2 participants
This is pretty much weihanglo#38 but without richer display.
The richer display there was modeled after the
tomlcrate, though I just found it didn't handle multi-bytes UTF-8 well (including CJK of course). I'd like to just drop it now and maybe that can behind a Cargo feature or an optional config in the future.