From c147125ce96714d105985462aac76a0b52f2f545 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 9 Apr 2024 18:41:53 +0900 Subject: [PATCH] cli: migrate "cat" to matcher API, warn unmatched paths This is the last non-debug command that doesn't support file patterns. It wouldn't make much sense to "cat" multiple files (despite the command name), but doing that should be harmless. --- cli/src/commands/cat.rs | 63 ++++++++++++++++++++++++-------- cli/tests/cli-reference@.md.snap | 10 +++-- cli/tests/test_cat_command.rs | 46 +++++++++++++++++++++-- 3 files changed, 96 insertions(+), 23 deletions(-) diff --git a/cli/src/commands/cat.rs b/cli/src/commands/cat.rs index 373046c08..3a5c47a5e 100644 --- a/cli/src/commands/cat.rs +++ b/cli/src/commands/cat.rs @@ -15,25 +15,31 @@ use std::io::{self, Write}; use jj_lib::conflicts::{materialize_tree_value, MaterializedTreeValue}; +use jj_lib::fileset::{FilePattern, FilesetExpression}; use jj_lib::merge::MergedTreeValue; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; use pollster::FutureExt; use tracing::instrument; -use crate::cli_util::{CommandHelper, RevisionArg, WorkspaceCommandHelper}; +use crate::cli_util::{ + print_unmatched_explicit_paths, CommandHelper, RevisionArg, WorkspaceCommandHelper, +}; use crate::command_error::{user_error, CommandError}; use crate::ui::Ui; -/// Print contents of a file in a revision +/// Print contents of files in a revision +/// +/// If the given path is a directory, files in the directory will be visited +/// recursively. #[derive(clap::Args, Clone, Debug)] pub(crate) struct CatArgs { /// The revision to get the file contents from #[arg(long, short, default_value = "@")] revision: RevisionArg, - /// The file to print - #[arg(value_hint = clap::ValueHint::FilePath)] - path: String, + /// Paths to print + #[arg(required = true, value_hint = clap::ValueHint::FilePath)] + paths: Vec, } #[instrument(skip_all)] @@ -45,18 +51,46 @@ 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 value = tree.path_value(&path); - if value.is_absent() { - let ui_path = workspace_command.format_file_path(&path); - return Err(user_error(format!("No such path: {ui_path}"))); + // TODO: No need to add special case for empty paths when switching to + // parse_union_filesets(). paths = [] should be "none()" if supported. + let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?; + + // Try fast path for single file entry + if let Some(path) = get_single_path(&fileset_expression) { + let value = tree.path_value(path); + if value.is_absent() { + let ui_path = workspace_command.format_file_path(path); + return Err(user_error(format!("No such path: {ui_path}"))); + } + if !value.is_tree() { + ui.request_pager(); + write_tree_entries(ui, &workspace_command, [(path, value)])?; + return Ok(()); + } } + + let matcher = fileset_expression.to_matcher(); ui.request_pager(); - write_tree_entries(ui, &workspace_command, [(&path, value)])?; + write_tree_entries( + ui, + &workspace_command, + tree.entries_matching(matcher.as_ref()), + )?; + print_unmatched_explicit_paths(ui, &workspace_command, &fileset_expression, [&tree])?; Ok(()) } +fn get_single_path(expression: &FilesetExpression) -> Option<&RepoPath> { + match &expression { + FilesetExpression::Pattern(pattern) => match pattern { + // Not using pattern.as_path() because files-in: shouldn't + // select the literal itself. + FilePattern::FilePath(path) | FilePattern::PrefixPath(path) => Some(path), + }, + _ => None, + } +} + fn write_tree_entries>( ui: &Ui, workspace_command: &WorkspaceCommandHelper, @@ -73,15 +107,14 @@ fn write_tree_entries>( MaterializedTreeValue::Conflict { contents, .. } => { ui.stdout_formatter().write_all(&contents)?; } - MaterializedTreeValue::Symlink { .. } - | MaterializedTreeValue::Tree(_) - | MaterializedTreeValue::GitSubmodule(_) => { + MaterializedTreeValue::Symlink { .. } | MaterializedTreeValue::GitSubmodule(_) => { let ui_path = workspace_command.format_file_path(path.as_ref()); writeln!( ui.warning_default(), "Path exists but is not a file: {ui_path}" )?; } + MaterializedTreeValue::Tree(_) => panic!("entries should not contain trees"), } } Ok(()) diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 942063aa6..c3a50add6 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -108,7 +108,7 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d * `abandon` — Abandon a revision * `backout` — Apply the reverse of a revision on top of another revision * `branch` — Manage branches -* `cat` — Print contents of a file in a revision +* `cat` — Print contents of files in a revision * `chmod` — Sets or removes the executable bit for paths in the repo * `commit` — Update the description and create a new change on top * `config` — Manage config options @@ -384,13 +384,15 @@ A non-tracking remote branch is just a pointer to the last-fetched remote branch ## `jj cat` -Print contents of a file in a revision +Print contents of files in a revision -**Usage:** `jj cat [OPTIONS] ` +If the given path is a directory, files in the directory will be visited recursively. + +**Usage:** `jj cat [OPTIONS] ...` ###### **Arguments:** -* `` — The file to print +* `` — Paths to print ###### **Options:** diff --git a/cli/tests/test_cat_command.rs b/cli/tests/test_cat_command.rs index 5efd7a635..f15c6da55 100644 --- a/cli/tests/test_cat_command.rs +++ b/cli/tests/test_cat_command.rs @@ -61,11 +61,26 @@ fn test_cat() { Error: No such path: nonexistent "###); - // TODO: files under the directory will be printed - let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["cat", "dir"]); - insta::assert_snapshot!(stdout, @""); + // Can print files under the specified directory + let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "dir"]); + insta::assert_snapshot!(stdout, @r###" + c + "###); + + // Can print multiple files + let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "."]); + insta::assert_snapshot!(stdout, @r###" + c + b + "###); + + // Unmatched paths should generate warnings + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["cat", "file1", "non-existent"]); + insta::assert_snapshot!(stdout, @r###" + b + "###); insta::assert_snapshot!(stderr, @r###" - Warning: Path exists but is not a file: dir + Warning: No matching entries for paths: non-existent "###); // Can print a conflict @@ -83,3 +98,26 @@ fn test_cat() { >>>>>>> "###); } + +#[cfg(unix)] +#[test] +fn test_cat_symlink() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + std::fs::write(repo_path.join("file1"), "a\n").unwrap(); + std::fs::create_dir(repo_path.join("dir")).unwrap(); + std::fs::write(repo_path.join("dir").join("file2"), "c\n").unwrap(); + std::os::unix::fs::symlink("symlink1_target", repo_path.join("symlink1")).unwrap(); + + // Can print multiple files + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["cat", "."]); + insta::assert_snapshot!(stdout, @r###" + c + a + "###); + insta::assert_snapshot!(stderr, @r###" + Warning: Path exists but is not a file: symlink1 + "###); +}