diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 5e3b6504d..1d2294673 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -36,6 +36,7 @@ use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue}; use jj_lib::commit::Commit; +use jj_lib::fileset::FilesetExpression; use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile}; use jj_lib::hex_util::to_reverse_hex; @@ -1223,8 +1224,9 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin // are millions of commits added to the repo, assuming the revset engine can // efficiently skip non-conflicting commits. Filter out empty commits mostly so // `jj new ` doesn't result in a message about new conflicts. - let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict) - .intersection(&RevsetExpression::filter(RevsetFilterPredicate::File(None))); + let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict).intersection( + &RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all())), + ); let removed_conflicts_expr = new_heads.range(&old_heads).intersection(&conflicts); let added_conflicts_expr = old_heads.range(&new_heads).intersection(&conflicts); diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index ee11cf4bc..e925b0f36 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -14,6 +14,7 @@ use itertools::Itertools; use jj_lib::backend::CommitId; +use jj_lib::fileset::FilesetExpression; use jj_lib::repo::Repo; use jj_lib::revset::{self, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; use jj_lib::revset_graph::{ @@ -89,14 +90,14 @@ pub(crate) fn cmd_log( workspace_command.attach_revset_evaluator(RevsetExpression::all())? }; if !args.paths.is_empty() { - let repo_paths: Vec<_> = args + let file_expressions: Vec<_> = args .paths .iter() .map(|path_arg| workspace_command.parse_file_path(path_arg)) + .map_ok(FilesetExpression::prefix_path) .try_collect()?; - expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File( - Some(repo_paths), - ))); + let expr = FilesetExpression::union_all(file_expressions); + expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File(expr))); } expression }; diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 0737d869f..452c1a4d1 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -28,7 +28,7 @@ use super::rev_walk::{EagerRevWalk, PeekableRevWalk, RevWalk, RevWalkBuilder}; use super::revset_graph_iterator::RevsetGraphWalk; use crate::backend::{ChangeId, CommitId, MillisSinceEpoch}; use crate::default_index::{AsCompositeIndex, CompositeIndex, IndexEntry, IndexPosition}; -use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit}; +use crate::matchers::{Matcher, Visit}; use crate::repo_path::RepoPath; use crate::revset::{ ResolvedExpression, ResolvedPredicateExpression, Revset, RevsetEvaluationError, @@ -1038,13 +1038,8 @@ fn build_predicate_fn( || pattern.matches(&commit.committer().email) }) } - RevsetFilterPredicate::File(paths) => { - // TODO: Add support for globs and other formats - let matcher: Rc = if let Some(paths) = paths { - Rc::new(PrefixMatcher::new(paths)) - } else { - Rc::new(EverythingMatcher) - }; + RevsetFilterPredicate::File(expr) => { + let matcher: Rc = expr.to_matcher().into(); box_pure_predicate_fn(move |index, pos| { let entry = index.entry_by_pos(pos); has_diff_from_parent(&store, index, &entry, matcher.as_ref()) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index dec825ba2..a89603f48 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -33,6 +33,7 @@ use thiserror::Error; use crate::backend::{BackendError, BackendResult, ChangeId, CommitId}; use crate::commit::Commit; +use crate::fileset::FilesetExpression; use crate::git; use crate::hex_util::to_forward_hex; use crate::object_id::{HexPrefix, PrefixResolution}; @@ -318,8 +319,8 @@ pub enum RevsetFilterPredicate { Author(StringPattern), /// Commits with committer's name or email containing the needle. Committer(StringPattern), - /// Commits modifying the paths specified by the pattern. - File(Option>), // TODO: embed matcher expression? + /// Commits modifying the paths specified by the fileset. + File(FilesetExpression), /// Commits with conflicts HasConflict, } @@ -1330,12 +1331,15 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: }); map.insert("empty", |name, arguments_pair, _state| { expect_no_arguments(name, arguments_pair)?; - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(None)).negated()) + Ok( + RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all())) + .negated(), + ) }); map.insert("file", |name, arguments_pair, state| { let arguments_span = arguments_pair.as_span(); if let Some(ctx) = state.workspace_ctx { - let paths: Vec<_> = arguments_pair + let file_expressions: Vec<_> = arguments_pair .into_inner() .map(|arg| -> Result<_, RevsetParseError> { let span = arg.as_span(); @@ -1345,19 +1349,18 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: RevsetParseError::invalid_arguments(name, "Invalid file pattern", span) .with_source(e) })?; - Ok(path) + Ok(FilesetExpression::prefix_path(path)) }) .try_collect()?; - if paths.is_empty() { + if file_expressions.is_empty() { Err(RevsetParseError::invalid_arguments( name, "Expected at least 1 argument", arguments_span, )) } else { - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some( - paths, - )))) + let expr = FilesetExpression::union_all(file_expressions); + Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr))) } } else { Err(RevsetParseError::with_span( @@ -2955,9 +2958,9 @@ mod tests { StringPattern::Substring("arg1".to_string()) )) .minus(&RevsetExpression::filter(RevsetFilterPredicate::File( - Some(vec![ - RepoPathBuf::from_internal_string("arg1"), - RepoPathBuf::from_internal_string("arg2"), + FilesetExpression::union_all(vec![ + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("arg1")), + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("arg2")), ]) ))) .minus(&RevsetExpression::visible_heads())) @@ -3317,25 +3320,28 @@ mod tests { ); assert_eq!( parse_with_workspace("empty()", &WorkspaceId::default()), - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(None)).negated()) + Ok( + RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all())) + .negated() + ) ); assert!(parse_with_workspace("empty(foo)", &WorkspaceId::default()).is_err()); assert!(parse_with_workspace("file()", &WorkspaceId::default()).is_err()); assert_eq!( parse_with_workspace("file(foo)", &WorkspaceId::default()), - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some( - vec![RepoPathBuf::from_internal_string("foo")] - )))) + Ok(RevsetExpression::filter(RevsetFilterPredicate::File( + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("foo")) + ))) ); assert_eq!( parse_with_workspace("file(foo, bar, baz)", &WorkspaceId::default()), - Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some( - vec![ - RepoPathBuf::from_internal_string("foo"), - RepoPathBuf::from_internal_string("bar"), - RepoPathBuf::from_internal_string("baz"), - ] - )))) + Ok(RevsetExpression::filter(RevsetFilterPredicate::File( + FilesetExpression::union_all(vec![ + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("foo")), + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("bar")), + FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("baz")), + ]) + ))) ); } @@ -4023,7 +4029,7 @@ mod tests { insta::assert_debug_snapshot!(optimize(parse("~empty()").unwrap()), @r###" Filter( File( - None, + All, ), ) "###); @@ -4260,10 +4266,10 @@ mod tests { ), Filter( File( - Some( - [ + Pattern( + PrefixPath( "bar", - ], + ), ), ), ), @@ -4282,10 +4288,10 @@ mod tests { ), Filter( File( - Some( - [ + Pattern( + PrefixPath( "bar", - ], + ), ), ), ), @@ -4315,10 +4321,10 @@ mod tests { ), Filter( File( - Some( - [ + Pattern( + PrefixPath( "bar", - ], + ), ), ), ), diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index e1a831400..278206c04 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -18,6 +18,7 @@ use assert_matches::assert_matches; use itertools::Itertools; use jj_lib::backend::{CommitId, MillisSinceEpoch, Signature, Timestamp}; use jj_lib::commit::Commit; +use jj_lib::fileset::FilesetExpression; use jj_lib::git; use jj_lib::git_backend::GitBackend; use jj_lib::object_id::ObjectId; @@ -2671,10 +2672,9 @@ fn test_evaluate_expression_file() { let resolve = |file_path: &RepoPath| -> Vec { let mut_repo = &*mut_repo; - let expression = - RevsetExpression::filter(RevsetFilterPredicate::File(Some( - vec![file_path.to_owned()], - ))); + let expression = RevsetExpression::filter(RevsetFilterPredicate::File( + FilesetExpression::prefix_path(file_path.to_owned()), + )); let revset = expression.evaluate_programmatic(mut_repo).unwrap(); revset.iter().collect() };