diff --git a/CHANGELOG.md b/CHANGELOG.md index b26e7da4f..35af57788 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * New `tracked_remote_branches()` and `untracked_remote_branches()` revset functions can be used to select tracked/untracked remote branches. +* The `file()` revset function now accepts fileset as argument. + ### Fixed bugs * `jj diff --git` no longer shows the contents of binary files. diff --git a/cli/tests/test_revset_output.rs b/cli/tests/test_revset_output.rs index 766f84f6e..0c564343e 100644 --- a/cli/tests/test_revset_output.rs +++ b/cli/tests/test_revset_output.rs @@ -140,42 +140,61 @@ fn test_bad_function_call() { = Function "file": Expected at least 1 arguments "###); - let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(a, not@a-string)"]); + let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(not::a-fileset)"]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: Expected expression of file pattern - Caused by: --> 1:9 + Error: Failed to parse revset: Invalid fileset expression + Caused by: + 1: --> 1:6 | - 1 | file(a, not@a-string) - | ^----------^ + 1 | file(not::a-fileset) + | ^------------^ | - = Expected expression of file pattern + = Invalid fileset expression + 2: --> 1:5 + | + 1 | not::a-fileset + | ^--- + | + = expected , , or "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"file(foo:"bar")"#]); insta::assert_snapshot!(stderr, @r###" - Error: Failed to parse revset: Invalid file pattern + Error: Failed to parse revset: Invalid fileset expression Caused by: 1: --> 1:6 | 1 | file(foo:"bar") | ^-------^ | + = Invalid fileset expression + 2: --> 1:1 + | + 1 | foo:"bar" + | ^-------^ + | = Invalid file pattern - 2: Invalid file pattern kind "foo:" + 3: Invalid file pattern kind "foo:" "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"file(a, "../out")"#]); insta::assert_snapshot!(stderr.replace('\\', "/"), @r###" - Error: Failed to parse revset: Invalid file pattern + Error: Failed to parse revset: Invalid fileset expression Caused by: 1: --> 1:9 | 1 | file(a, "../out") | ^------^ | + = Invalid fileset expression + 2: --> 1:1 + | + 1 | "../out" + | ^------^ + | = Invalid file pattern - 2: Path "../out" is not in the repo "." - 3: Invalid component ".." in repo-relative path "../out" + 3: Path "../out" is not in the repo "." + 4: Invalid component ".." in repo-relative path "../out" "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "branches(bad:pattern)"]); diff --git a/docs/revsets.md b/docs/revsets.md index 023f645ac..56b755927 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -268,8 +268,8 @@ given [string pattern](#string-patterns). * `empty()`: Commits modifying no files. This also includes `merges()` without user modifications and `root()`. -* `file(pattern[, pattern]...)`: Commits modifying paths matching one of the - given [file patterns](filesets.md#file-patterns). +* `file(expression)`: Commits modifying paths matching the given [fileset + expression](filesets.md). Paths are relative to the directory `jj` was invoked from. A directory name will match all files in that directory and its subdirectories. @@ -277,6 +277,9 @@ given [string pattern](#string-patterns). For example, `file(foo)` will match files `foo`, `foo/bar`, `foo/bar/baz`. It will *not* match `foobar` or `bar/foo`. + Some file patterns might need quoting because the `expression` must also be + parsable as a revset. For example, `.` has to be quoted in `file(".")`. + * `conflict()`: Commits with conflicts. * `present(x)`: Same as `x`, but evaluated to `none()` if any of the commits diff --git a/lib/src/fileset.rs b/lib/src/fileset.rs index acffdbf94..07b522583 100644 --- a/lib/src/fileset.rs +++ b/lib/src/fileset.rs @@ -446,6 +446,16 @@ fn resolve_expression( } } +/// Parses text into `FilesetExpression` without bare string fallback. +pub fn parse( + text: &str, + path_converter: &RepoPathUiConverter, +) -> FilesetParseResult { + let node = fileset_parser::parse_program(text)?; + // TODO: add basic tree substitution pass to eliminate redundant expressions + resolve_expression(path_converter, &node) +} + /// Parses text into `FilesetExpression` with bare string fallback. /// /// If the text can't be parsed as a fileset expression, and if it doesn't diff --git a/lib/src/fileset_parser.rs b/lib/src/fileset_parser.rs index 68e7bf74b..884c6d687 100644 --- a/lib/src/fileset_parser.rs +++ b/lib/src/fileset_parser.rs @@ -305,7 +305,6 @@ fn parse_expression_node(pair: Pair) -> FilesetParseResult } /// Parses text into expression tree. No name resolution is made at this stage. -#[cfg(test)] // TODO: alias will be parsed with no bare_string fallback pub fn parse_program(text: &str) -> FilesetParseResult { let mut pairs = FilesetParser::parse(Rule::program, text)?; let first = pairs.next().unwrap(); diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 262e4aa08..db4ba985f 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -29,7 +29,7 @@ use thiserror::Error; use crate::backend::{BackendError, BackendResult, ChangeId, CommitId}; use crate::commit::Commit; use crate::dsl_util::{collect_similar, AliasExpandError as _}; -use crate::fileset::{FilePattern, FilesetExpression}; +use crate::fileset::FilesetExpression; use crate::graph::GraphEdge; use crate::hex_util::to_forward_hex; use crate::id_prefix::IdPrefixContext; @@ -43,7 +43,7 @@ pub use crate::revset_parser::{ }; use crate::store::Store; use crate::str_util::StringPattern; -use crate::{dsl_util, revset_parser}; +use crate::{dsl_util, fileset, revset_parser}; /// Error occurred during symbol resolution. #[derive(Debug, Error)] @@ -706,10 +706,10 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: function.args_span, // TODO: better to use name_span? ) })?; + // TODO: emit deprecation warning if multiple arguments are passed let ([arg], args) = function.expect_some_arguments()?; let file_expressions = itertools::chain([arg], args) - .map(|arg| expect_file_pattern(arg, ctx.path_converter)) - .map_ok(FilesetExpression::pattern) + .map(|arg| expect_fileset_expression(arg, ctx.path_converter)) .try_collect()?; let expr = FilesetExpression::union_all(file_expressions); Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr))) @@ -726,15 +726,19 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: map }); -pub fn expect_file_pattern( +/// Parses the given `node` as a fileset expression. +pub fn expect_fileset_expression( node: &ExpressionNode, path_converter: &RepoPathUiConverter, -) -> Result { - let parse_pattern = |value: &str, kind: Option<&str>| match kind { - Some(kind) => FilePattern::from_str_kind(path_converter, value, kind), - None => FilePattern::cwd_prefix_path(path_converter, value), - }; - revset_parser::expect_pattern_with("file pattern", node, parse_pattern) +) -> Result { + // Alias handling is a bit tricky. The outermost expression `alias` is + // substituted, but inner expressions `x & alias` aren't. If this seemed + // weird, we can either transform AST or turn off revset aliases completely. + revset_parser::expect_expression_with(node, |node| { + fileset::parse(node.span.as_str(), path_converter).map_err(|err| { + RevsetParseError::expression("Invalid fileset expression", node.span).with_source(err) + }) + }) } pub fn expect_string_pattern(node: &ExpressionNode) -> Result { @@ -2567,9 +2571,28 @@ mod tests { insta::assert_debug_snapshot!( parse_with_workspace("file(foo)", &WorkspaceId::default()).unwrap(), @r###"Filter(File(Pattern(PrefixPath("foo"))))"###); + insta::assert_debug_snapshot!( + parse_with_workspace("file(all())", &WorkspaceId::default()).unwrap(), + @"Filter(File(All))"); insta::assert_debug_snapshot!( parse_with_workspace(r#"file(file:"foo")"#, &WorkspaceId::default()).unwrap(), @r###"Filter(File(Pattern(FilePath("foo"))))"###); + insta::assert_debug_snapshot!( + parse_with_workspace("file(foo|bar&baz)", &WorkspaceId::default()).unwrap(), @r###" + Filter( + File( + UnionAll( + [ + Pattern(PrefixPath("foo")), + Intersection( + Pattern(PrefixPath("bar")), + Pattern(PrefixPath("baz")), + ), + ], + ), + ), + ) + "###); insta::assert_debug_snapshot!( parse_with_workspace("file(foo, bar, baz)", &WorkspaceId::default()).unwrap(), @r###" Filter( diff --git a/lib/src/revset_parser.rs b/lib/src/revset_parser.rs index 742f4b1bb..234aa0eda 100644 --- a/lib/src/revset_parser.rs +++ b/lib/src/revset_parser.rs @@ -804,7 +804,7 @@ pub fn expect_literal( /// Applies the give function to the innermost `node` by unwrapping alias /// expansion nodes. -fn expect_expression_with( +pub(super) fn expect_expression_with( node: &ExpressionNode, f: impl FnOnce(&ExpressionNode) -> Result, ) -> Result { diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 75716ed1e..13f3d3e21 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -2986,6 +2986,15 @@ fn test_evaluate_expression_file() { ), vec![commit1.id().clone()] ); + assert_eq!( + resolve_commit_ids_in_workspace( + mut_repo, + r#"file("added_clean_clean"|"added_modified_clean")"#, + &test_workspace.workspace, + Some(test_workspace.workspace.workspace_root()), + ), + vec![commit2.id().clone(), commit1.id().clone()] + ); assert_eq!( resolve_commit_ids_in_workspace( mut_repo,