-
Notifications
You must be signed in to change notification settings - Fork 67
Implement :from filter #1519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Implement :from filter #1519
Conversation
b8ff019 to
d02579b
Compare
1736730 to
9ead7f6
Compare
37ee5cd to
73287ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds two new filter operations, :from and :concat, to enable starting history filtering from a specific commit. The :from filter preserves original history up to a specified commit and applies filtering from that point onwards, while :concat is the underlying operation used internally.
Key changes:
- Added grammar rules for
filter_fromandfilter_concatoperations - Implemented parsing logic and the
HistoryConcatoperation in the filter core - Added test coverage for the new
:fromfilter functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| josh-core/src/filter/grammar.pest | Added grammar rules for filter_from and filter_concat |
| josh-core/src/filter/parse.rs | Implemented parsing logic for the new filter operations |
| josh-core/src/filter/mod.rs | Added HistoryConcat operation with support for lazy refs, spec generation, tree representation, and commit application |
| tests/filter/concat.t | Added test cases demonstrating :from filter behavior |
| tests/proxy/workspace_errors.t | Updated error message to include new filter types |
| docs/src/reference/filters.md | Added documentation for the new :from filter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| filter_concat = { | ||
| CMD_START ~ "from" ~ "(" | ||
| ~ NEWLINE* | ||
| ~ (rev ~ filter_spec)? | ||
| ~ (CMD_SEP+ ~ (rev ~ filter_spec))* | ||
| ~ NEWLINE* | ||
| ~ ")" | ||
| } |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter_concat rule incorrectly uses \"from\" as the command keyword instead of \"concat\". This causes the parser to be unable to distinguish between :from() and :concat() operations. Change line 66 to use \"concat\".
| format!(":{}", parse::quote(m)) | ||
| } | ||
| Op::HistoryConcat(r, filter) => { | ||
| format!(":concat({}{})", r.to_string(), spec(*filter)) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing separator between the reference and the filter spec. The format should be :concat({}{}) with a colon separator, similar to how Op::Rev and Op::Join format their output on lines 593 and 601. This should be format!(\"{}{}\", r.to_string(), spec(*filter)) to match the input format expected by the parser.
| format!(":concat({}{})", r.to_string(), spec(*filter)) | |
| format!(":concat({}:{})", r.to_string(), spec(*filter)) |
99d88a7 to
ecc50a7
Compare
2be9b20 to
7050edb
Compare
7050edb to
1d766b3
Compare
Change: from-filter