Print the actual reason a --changed-before/--changed-within date fails to parse - #2094
Open
MsfPablo wants to merge 1 commit into
Open
Print the actual reason a --changed-before/--changed-within date fails to parse#2094MsfPablo wants to merge 1 commit into
MsfPablo wants to merge 1 commit into
Conversation
TimeFilter::before/after now return Result<_, String> instead of Option, so the underlying jiff parse error reaches the user. An input like 2025-11-31 now says the day is out of range for that month rather than only that the value is invalid. Closes sharkdp#2053
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.
Closes #2053.
--changed-before/--changed-withincurrently swallow the parse error and print the same generic message for every bad input:As the issue points out, that reads like a formatting problem, when the real problem is that November has 30 days.
TimeFilter::from_strdiscarded thejifferrors because it returnedOption<SystemTime>. This changesTimeFilter::before/afterto returnResult<TimeFilter, String>and passes the reason through to the user:The
@-timestamp branch was also moved up so that an input starting with@reports a timestamp error rather than falling through to the date parser and complaining about years.Accepted inputs are unchanged; only the error paths differ. Added a unit test asserting the new messages, and updated the existing tests plus the CHANGELOG.
Verification (clean, and also clean on a
git stashbaseline):AI disclosure
Per CONTRIBUTING.md item 4: I used an AI coding assistant (Claude Code) for this change. It was used to locate the code path, draft the
Option->Resultrefactor insrc/filter/time.rsand the matching call sites insrc/main.rs, and to draft the unit test. I reviewed the diff, ran the commands above myself, and checked the resulting messages against the cases in the issue. I understand the change and can answer questions or revise it. The PR text is my own; AI was not used to write this description beyond assembling the command output above.