Repository navigation
tgrep-core: implement Error::source() and add tests for error module - #178
Merged
Shengyu Fu (shengyfu) merged 1 commit intoOct 6, 2026
Merged
Conversation
The public Error type used 'impl std::error::Error for Error {}' (empty),
so source() returned None for every variant — losing the inner cause for
Io and Json, which wrap std::io::Error and serde_json::Error respectively.
The other three error types in the crate (GenerationError, WorktreeError,
MoveStagedFilesError) already implement source() correctly. This brings
Error in line with the existing pattern.
Adds 13 unit tests covering Display for every variant, source() returning
the inner error for Io/Json and None for the four String variants, From
conversions preserving the inner error, and Result<T> being Send + Sync.
Contributor
Author
|
@microsoft-github-policy-service agree |
Shengyu Fu (shengyfu)
approved these changes
Oct 6, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation matches existing crate patterns and the tests adequately cover the changed behavior.
Review effort: Balanced
Findings: None
What changed in this PR
Implements error chaining for wrapped I/O and JSON errors and adds comprehensive unit coverage.
Changes:
- Exposes underlying causes through
Error::source(). - Tests display formatting, conversions, sources, result aliases, and thread-safety bounds.
| File | Description |
|---|---|
tgrep-core/src/error.rs |
Implements error sources and adds unit tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or 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
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.
Summary
The public
Errortype intgrep-core/src/error.rsusedimpl std::error::Error for Error {}(empty), sosource()returnedNonefor every variant. This loses the inner cause for theIoandJsonvariants, which wrapstd::io::Errorandserde_json::Errorrespectively.The other three error types in the crate (
GenerationError,WorktreeError,MoveStagedFilesError) already implementsource()correctly. This bringsErrorin line with the existing pattern.Changes
Error::source()returningSome(error)forIoandJson,Nonefor the fourStringvariants (IndexNotFound,IndexCorrupted,Regex,Server).#[cfg(test)] mod testsblock with 13 unit tests covering:source()returning the inner error forIo/JsonandNonefor the String variantsFrom<io::Error>andFrom<serde_json::Error>preserving the inner errorResult<T>alias round-trip andSend + Syncbounds (compile-time, sinceResult<T>crosses thread boundaries inwalker.rsand the serve runtime)Why this is safe
source()is not called anywhere in production code (grep -rn "\.source()" tgrep-core/src/ tgrep-cli/src/returns 0 hits outside tests).source()goes fromNone(default) toSome(error)for the wrapping variants. Any code that depended onNonewas silently losing the cause chain — this fixes that.GenerationError::source()intgrep-core/src/generations/mod.rs:70-79.Validation
cargo fmt --all -- --check✅cargo clippy --workspace -- -D warnings✅cargo test -p tgrep-core --lib error✅ (17 tests: 13 new + 4 pre-existing in the same module)