From 8df979c7235d03242e3124eb47c22e14eff9639b Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 6 Apr 2024 16:46:24 +0900 Subject: [PATCH] cli: add support for file kind:pattern syntax This is basically the same as string kind:pattern syntax in CLI. This will hopefully be superseded by filesets, but I'm not sure if that will work out. A file name is more likely to contain whitespaces, which will have to be quoted as '"Documents and Settings"'. --- CHANGELOG.md | 3 +++ cli/src/cli_util.rs | 35 ++++++++++++++++++++++++++--------- cli/src/command_error.rs | 4 ++-- cli/src/commands/cat.rs | 1 + cli/src/commands/chmod.rs | 1 + cli/src/commands/log.rs | 17 ++++++----------- cli/tests/test_log_command.rs | 20 ++++++++++++++++++++ 7 files changed, 59 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d78ff840e..8931d73442 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj status` now supports filtering by paths. For example, `jj status .` will only list changed files that are descendants of the current directory. +* File path arguments now support [file pattern + syntax](docs/filesets.md#file-patterns). + ### Fixed bugs ## [0.16.0] - 2024-04-03 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 1d22946733..f0a0c1b4e4 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -36,12 +36,12 @@ 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::fileset::{FilePattern, FilesetExpression, FilesetParseContext}; use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile}; use jj_lib::hex_util::to_reverse_hex; use jj_lib::id_prefix::IdPrefixContext; -use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher}; +use jj_lib::matchers::Matcher; use jj_lib::merge::MergedTreeValue; use jj_lib::merged_tree::MergedTree; use jj_lib::object_id::ObjectId; @@ -646,19 +646,36 @@ impl WorkspaceCommandHelper { RepoPathBuf::parse_fs_path(&self.cwd, self.workspace_root(), input) } - pub fn matcher_from_values(&self, values: &[String]) -> Result, CommandError> { + /// Parses the given strings as file patterns. + pub fn parse_file_patterns( + &self, + values: &[String], + ) -> Result { + // TODO: This function might be superseded by parse_union_filesets(), + // but it would be weird if parse_union_*() had a special case for the + // empty arguments. if values.is_empty() { - Ok(Box::new(EverythingMatcher)) + Ok(FilesetExpression::all()) } else { - // TODO: Add support for globs and other formats - let paths: Vec<_> = values + let ctx = FilesetParseContext { + cwd: &self.cwd, + workspace_root: self.workspace.workspace_root(), + }; + let expressions = values .iter() - .map(|v| self.parse_file_path(v)) - .try_collect()?; - Ok(Box::new(PrefixMatcher::new(paths))) + .map(|v| FilePattern::parse(&ctx, v)) + .map_ok(FilesetExpression::pattern) + .try_collect() + .map_err(user_error)?; + Ok(FilesetExpression::union_all(expressions)) } } + pub fn matcher_from_values(&self, values: &[String]) -> Result, CommandError> { + let expr = self.parse_file_patterns(values)?; + Ok(expr.to_matcher()) + } + #[instrument(skip_all)] pub fn base_ignores(&self) -> Result, GitIgnoreError> { fn get_excludes_file_path(config: &gix::config::File) -> Option { diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 4aa87af6a1..5be11648e6 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -487,8 +487,6 @@ impl From for CommandError { impl From for CommandError { fn from(err: FsPathParseError) -> Self { - // TODO: implement pattern prefix like "root:" or "--cwd" option, - // and suggest it if the user input looks like repo-relative path #3216. user_error(err) } } @@ -522,6 +520,8 @@ impl From for CommandError { fn find_source_parse_error_hint(err: &dyn error::Error) -> Option { let source = err.source()?; + // TODO: For FilePatternParseError, suggest "root:" if the user + // input looks like repo-relative path #3216. if let Some(source) = source.downcast_ref() { string_pattern_parse_error_hint(source) } else { diff --git a/cli/src/commands/cat.rs b/cli/src/commands/cat.rs index 128ed73a2c..c0d78562d1 100644 --- a/cli/src/commands/cat.rs +++ b/cli/src/commands/cat.rs @@ -43,6 +43,7 @@ pub(crate) fn cmd_cat( let workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; let tree = commit.tree()?; + // TODO: migrate to .parse_file_patterns()?.to_matcher()? let path = workspace_command.parse_file_path(&args.path)?; let repo = workspace_command.repo(); let value = tree.path_value(&path); diff --git a/cli/src/commands/chmod.rs b/cli/src/commands/chmod.rs index 6f2d4af7dc..a63fdb9627 100644 --- a/cli/src/commands/chmod.rs +++ b/cli/src/commands/chmod.rs @@ -60,6 +60,7 @@ pub(crate) fn cmd_chmod( }; let mut workspace_command = command.workspace_helper(ui)?; + // TODO: migrate to .parse_file_patterns()?.to_matcher() let repo_paths: Vec<_> = args .paths .iter() diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index e925b0f366..3b62362b1e 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -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::{ @@ -78,6 +76,7 @@ pub(crate) fn cmd_log( ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; + let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?; let revset_expression = { // only use default revset if neither revset nor path are specified let mut expression = if args.revisions.is_empty() && args.paths.is_empty() { @@ -90,20 +89,16 @@ pub(crate) fn cmd_log( workspace_command.attach_revset_evaluator(RevsetExpression::all())? }; if !args.paths.is_empty() { - let file_expressions: Vec<_> = args - .paths - .iter() - .map(|path_arg| workspace_command.parse_file_path(path_arg)) - .map_ok(FilesetExpression::prefix_path) - .try_collect()?; - let expr = FilesetExpression::union_all(file_expressions); - expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File(expr))); + // Beware that args.paths = ["root:."] is not identical to []. The + // former will filter out empty commits. + let predicate = RevsetFilterPredicate::File(fileset_expression.clone()); + expression.intersect_with(&RevsetExpression::filter(predicate)); } expression }; let repo = workspace_command.repo(); - let matcher = workspace_command.matcher_from_values(&args.paths)?; + let matcher = fileset_expression.to_matcher(); let revset = revset_expression.evaluate()?; let store = repo.store(); diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 0d52f0ddf9..14dd87ee30 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -822,6 +822,26 @@ fn test_log_filtered_by_path() { A file2 "###); + // "root:" is resolved relative to the workspace root. + let stdout = test_env.jj_cmd_success( + test_env.env_root(), + &[ + "log", + "-R", + repo_path.to_str().unwrap(), + "-Tdescription", + "-s", + "root:file1", + ], + ); + insta::assert_snapshot!(stdout.replace('\\', "/"), @r###" + @ second + │ M repo/file1 + ◉ first + │ A repo/file1 + ~ + "###); + // file() revset doesn't filter the diff. let stdout = test_env.jj_cmd_success( &repo_path,