Skip to content

Conversation

@christian-schilling
Copy link
Member

Change: from-filter

@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch 15 times, most recently from b8ff019 to d02579b Compare October 14, 2025 19:12
@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch 14 times, most recently from 1736730 to 9ead7f6 Compare November 2, 2025 09:56
@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch 2 times, most recently from 37ee5cd to 73287ab Compare November 2, 2025 17:59
Copy link

Copilot AI left a 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_from and filter_concat operations
  • Implemented parsing logic and the HistoryConcat operation in the filter core
  • Added test coverage for the new :from filter 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.

Comment on lines +65 to +72
filter_concat = {
CMD_START ~ "from" ~ "("
~ NEWLINE*
~ (rev ~ filter_spec)?
~ (CMD_SEP+ ~ (rev ~ filter_spec))*
~ NEWLINE*
~ ")"
}
Copy link

Copilot AI Nov 3, 2025

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\".

Copilot uses AI. Check for mistakes.
format!(":{}", parse::quote(m))
}
Op::HistoryConcat(r, filter) => {
format!(":concat({}{})", r.to_string(), spec(*filter))
Copy link

Copilot AI Nov 3, 2025

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.

Suggested change
format!(":concat({}{})", r.to_string(), spec(*filter))
format!(":concat({}:{})", r.to_string(), spec(*filter))

Copilot uses AI. Check for mistakes.
@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch 16 times, most recently from 99d88a7 to ecc50a7 Compare November 9, 2025 23:53
@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch 6 times, most recently from 2be9b20 to 7050edb Compare November 11, 2025 18:46
@christian-schilling christian-schilling force-pushed the @changes/master/christian.schilling.de@gmail.com/from-filter branch from 7050edb to 1d766b3 Compare November 11, 2025 19:15
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.

2 participants