diff --git a/Cargo.lock b/Cargo.lock index 2f4f2eb3bf..3f6b7851d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1826,6 +1826,7 @@ dependencies = [ "futures 0.3.31", "git2", "gix", + "glob", "indexmap", "indoc", "insta", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 57be178226..a2d3400a91 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -64,6 +64,7 @@ dunce = { workspace = true } futures = { workspace = true } git2 = { workspace = true } gix = { workspace = true } +glob = { workspace = true } indexmap = { workspace = true } indoc = { workspace = true } itertools = { workspace = true } diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index 2e79432958..52f75a4441 100644 --- a/cli/src/commands/commit.rs +++ b/cli/src/commands/commit.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use jj_lib::backend::Signature; use jj_lib::object_id::ObjectId; use jj_lib::repo::Repo; @@ -44,7 +44,7 @@ pub(crate) struct CommitArgs { /// Put these paths in the first commit #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::modified_files), + add = ArgValueCompleter::new(complete::modified_files), )] paths: Vec, /// Reset the author to the configured user diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 0f79aa707c..b3a1c28409 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -13,6 +13,7 @@ // limitations under the License. use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use itertools::Itertools; use jj_lib::copies::CopyRecords; use jj_lib::repo::Repo; @@ -59,7 +60,7 @@ pub(crate) struct DiffArgs { /// Restrict the diff to these paths #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::modified_revision_or_range_files), + add = ArgValueCompleter::new(complete::modified_revision_or_range_files), )] paths: Vec, #[command(flatten)] diff --git a/cli/src/commands/interdiff.rs b/cli/src/commands/interdiff.rs index dbe2db66b3..d577e22bdf 100644 --- a/cli/src/commands/interdiff.rs +++ b/cli/src/commands/interdiff.rs @@ -16,6 +16,7 @@ use std::slice; use clap::ArgGroup; use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use tracing::instrument; use crate::cli_util::CommandHelper; @@ -44,7 +45,7 @@ pub(crate) struct InterdiffArgs { /// Restrict the diff to these paths #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::interdiff_files), + add = ArgValueCompleter::new(complete::interdiff_files), )] paths: Vec, #[command(flatten)] diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index 76bd8315a3..bd29479ab1 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -15,6 +15,7 @@ use std::io::Write; use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use itertools::Itertools; use jj_lib::object_id::ObjectId; use tracing::instrument; @@ -64,7 +65,7 @@ pub(crate) struct ResolveArgs { // TODO: Find the conflict we can resolve even if it's not the first one. #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::revision_conflicted_files), + add = ArgValueCompleter::new(complete::revision_conflicted_files), )] paths: Vec, } diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index 11ec1bfdf7..5262dd6e3d 100644 --- a/cli/src/commands/restore.rs +++ b/cli/src/commands/restore.rs @@ -15,6 +15,7 @@ use std::io::Write; use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use jj_lib::object_id::ObjectId; use jj_lib::rewrite::restore_tree; use tracing::instrument; @@ -47,7 +48,7 @@ pub(crate) struct RestoreArgs { /// Restore only these paths (instead of all paths) #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::modified_range_files), + add = ArgValueCompleter::new(complete::modified_range_files), )] paths: Vec, /// Revision to restore from (source) diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index e750af8705..25d2f01cf3 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -14,6 +14,7 @@ use std::io::Write; use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use jj_lib::object_id::ObjectId; use jj_lib::repo::Repo; use tracing::instrument; @@ -68,7 +69,7 @@ pub(crate) struct SplitArgs { /// Put these paths in the first commit #[arg( value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::modified_revision_files), + add = ArgValueCompleter::new(complete::modified_revision_files), )] paths: Vec, } diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 79f13fba31..462fec1e8c 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -13,6 +13,7 @@ // limitations under the License. use clap_complete::ArgValueCandidates; +use clap_complete::ArgValueCompleter; use itertools::Itertools as _; use jj_lib::commit::Commit; use jj_lib::commit::CommitIteratorExt; @@ -93,7 +94,7 @@ pub(crate) struct SquashArgs { #[arg( conflicts_with_all = ["interactive", "tool"], value_hint = clap::ValueHint::AnyPath, - add = ArgValueCandidates::new(complete::squash_revision_files), + add = ArgValueCompleter::new(complete::squash_revision_files), )] paths: Vec, /// The source revision will not be abandoned diff --git a/cli/src/complete.rs b/cli/src/complete.rs index ed829a50b2..bad1e81504 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -448,7 +448,23 @@ pub fn leaf_config_keys() -> Vec { config_keys_impl(true) } -fn all_files_from_rev(rev: String) -> Vec { +fn dir_prefix_from<'a>(path: &'a str, current: &str) -> Option<&'a str> { + path[current.len()..] + .split_once(std::path::MAIN_SEPARATOR) + .map(|(next, _)| path.split_at(current.len() + next.len() + 1).0) +} + +fn current_prefix_to_fileset(current: &str) -> String { + let cur_esc = glob::Pattern::escape(current); + let dir_pat = format!("{cur_esc}*/**"); + let path_pat = format!("{cur_esc}*"); + format!("glob:{dir_pat:?} | glob:{path_pat:?}") +} + +fn all_files_from_rev(rev: String, current: &std::ffi::OsStr) -> Vec { + let Some(current) = current.to_str() else { + return Vec::new(); + }; with_jj(|jj, _| { let mut child = jj .build() @@ -456,6 +472,9 @@ fn all_files_from_rev(rev: String) -> Vec { .arg("list") .arg("--revision") .arg(rev) + .arg("--config-toml") + .arg("ui.allow-filesets = true") + .arg(current_prefix_to_fileset(current)) .stdout(std::process::Stdio::piped()) .spawn() .map_err(user_error)?; @@ -465,7 +484,13 @@ fn all_files_from_rev(rev: String) -> Vec { .lines() .take(1_000) .map_while(Result::ok) - .map(CompletionCandidate::new) + .map(|path| { + if let Some(dir_path) = dir_prefix_from(&path, current) { + return CompletionCandidate::new(dir_path); + } + CompletionCandidate::new(path) + }) + .dedup() // directories may occur multiple times .collect()) }) } @@ -473,8 +498,16 @@ fn all_files_from_rev(rev: String) -> Vec { fn modified_files_from_rev_with_jj_cmd( rev: (String, Option), mut cmd: std::process::Command, + current: &std::ffi::OsStr, ) -> Result, CommandError> { - cmd.arg("diff").arg("--summary"); + let Some(current) = current.to_str() else { + return Ok(Vec::new()); + }; + cmd.arg("diff") + .arg("--summary") + .arg("--config-toml") + .arg("ui.allow-filesets = true") + .arg(current_prefix_to_fileset(current)); match rev { (rev, None) => cmd.arg("--revision").arg(rev), (from, Some(to)) => cmd.arg("--from").arg(from).arg("--to").arg(to), @@ -488,6 +521,11 @@ fn modified_files_from_rev_with_jj_cmd( let (mode, path) = line .split_once(' ') .expect("diff --summary should contain a space between mode and path"); + + if let Some(dir_path) = dir_prefix_from(path, current) { + return CompletionCandidate::new(dir_path); + } + let help = match mode { "M" => "Modified".into(), "D" => "Deleted".into(), @@ -498,14 +536,21 @@ fn modified_files_from_rev_with_jj_cmd( }; CompletionCandidate::new(path).help(Some(help.into())) }) + .dedup() // directories may occur multiple times .collect()) } -fn modified_files_from_rev(rev: (String, Option)) -> Vec { - with_jj(|jj, _| modified_files_from_rev_with_jj_cmd(rev, jj.build())) +fn modified_files_from_rev( + rev: (String, Option), + current: &std::ffi::OsStr, +) -> Vec { + with_jj(|jj, _| modified_files_from_rev_with_jj_cmd(rev, jj.build(), current)) } -fn conflicted_files_from_rev(rev: &str) -> Vec { +fn conflicted_files_from_rev(rev: &str, current: &std::ffi::OsStr) -> Vec { + let Some(current) = current.to_str() else { + return Vec::new(); + }; with_jj(|jj, _| { let output = jj .build() @@ -513,59 +558,69 @@ fn conflicted_files_from_rev(rev: &str) -> Vec { .arg("--list") .arg("--revision") .arg(rev) + .arg("--config-toml") + .arg("ui.allow-filesets = true") + .arg(current_prefix_to_fileset(current)) .output() .map_err(user_error)?; let stdout = String::from_utf8_lossy(&output.stdout); Ok(stdout .lines() - .filter_map(|line| line.split_whitespace().next()) - .map(CompletionCandidate::new) + .map(|line| { + let path = line + .split_whitespace() + .next() + .expect("resolve --list should contain whitespace after path"); + + if let Some(dir_path) = dir_prefix_from(path, current) { + return CompletionCandidate::new(dir_path); + } + CompletionCandidate::new(path) + }) + .dedup() // directories may occur multiple times .collect()) }) } -pub fn modified_files() -> Vec { - modified_files_from_rev(("@".into(), None)) +pub fn modified_files(current: &std::ffi::OsStr) -> Vec { + modified_files_from_rev(("@".into(), None), current) } pub fn all_revision_files(current: &std::ffi::OsStr) -> Vec { - // TODO: Use `current` once `jj file list` gains the ability to list only - // the content of the "current" directory. - let _ = current; - all_files_from_rev(parse::revision_or_wc()) + all_files_from_rev(parse::revision_or_wc(), current) } -pub fn modified_revision_files() -> Vec { - modified_files_from_rev((parse::revision_or_wc(), None)) +pub fn modified_revision_files(current: &std::ffi::OsStr) -> Vec { + modified_files_from_rev((parse::revision_or_wc(), None), current) } -pub fn modified_range_files() -> Vec { +pub fn modified_range_files(current: &std::ffi::OsStr) -> Vec { match parse::range() { - Some((from, to)) => modified_files_from_rev((from, Some(to))), - None => modified_files_from_rev(("@".into(), None)), + Some((from, to)) => modified_files_from_rev((from, Some(to)), current), + None => modified_files_from_rev(("@".into(), None), current), } } -pub fn modified_revision_or_range_files() -> Vec { +pub fn modified_revision_or_range_files(current: &std::ffi::OsStr) -> Vec { if let Some(rev) = parse::revision() { - return modified_files_from_rev((rev, None)); + return modified_files_from_rev((rev, None), current); } - modified_range_files() + modified_range_files(current) } -pub fn revision_conflicted_files() -> Vec { - conflicted_files_from_rev(&parse::revision_or_wc()) +pub fn revision_conflicted_files(current: &std::ffi::OsStr) -> Vec { + conflicted_files_from_rev(&parse::revision_or_wc(), current) } /// Specific function for completing file paths for `jj squash` -pub fn squash_revision_files() -> Vec { +pub fn squash_revision_files(current: &std::ffi::OsStr) -> Vec { let rev = parse::squash_revision().unwrap_or_else(|| "@".into()); - modified_files_from_rev((rev, None)) + modified_files_from_rev((rev, None), current) } /// Specific function for completing file paths for `jj interdiff` -pub fn interdiff_files() -> Vec { +pub fn interdiff_files(current: &std::ffi::OsStr) -> Vec { let Some((from, to)) = parse::range() else { return Vec::new(); }; @@ -573,8 +628,12 @@ pub fn interdiff_files() -> Vec { // files that are the same in both, which is a false positive. This approach // is more lightweight than actually doing a temporary rebase here. with_jj(|jj, _| { - let mut res = modified_files_from_rev_with_jj_cmd((from, None), jj.build())?; - res.extend(modified_files_from_rev_with_jj_cmd((to, None), jj.build())?); + let mut res = modified_files_from_rev_with_jj_cmd((from, None), jj.build(), current)?; + res.extend(modified_files_from_rev_with_jj_cmd( + (to, None), + jj.build(), + current, + )?); Ok(res) }) } diff --git a/cli/tests/test_completion.rs b/cli/tests/test_completion.rs index 409a3f691f..6f8da88795 100644 --- a/cli/tests/test_completion.rs +++ b/cli/tests/test_completion.rs @@ -550,6 +550,9 @@ fn create_commit( test_env.jj_cmd_ok(repo_path, &args); } for (name, content) in files { + if let Some((dir, _)) = name.rsplit_once('/') { + std::fs::create_dir_all(repo_path.join(dir)).unwrap(); + } match content { Some(content) => std::fs::write(repo_path.join(name), content).unwrap(), None => std::fs::remove_file(repo_path.join(name)).unwrap(), @@ -564,6 +567,10 @@ fn test_files() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); let repo_path = test_env.env_root().join("repo"); + // Completions for files use filesets internally. + // Ensure they still work if the user has them disabled. + test_env.add_config("ui.allow-filesets = false"); + create_commit( &test_env, &repo_path, @@ -588,6 +595,9 @@ fn test_files() { ("f_renamed", Some("renamed\n")), ("f_deleted", None), ("f_added", Some("added\n")), + ("f_dir/dir_file_1", Some("foo\n")), + ("f_dir/dir_file_2", Some("foo\n")), + ("f_dir/dir_file_3", Some("foo\n")), ], ); @@ -600,6 +610,9 @@ fn test_files() { &[ ("f_modified", Some("modified_again\n")), ("f_added_2", Some("added_2\n")), + ("f_dir/dir_file_1", Some("bar\n")), + ("f_dir/dir_file_2", Some("bar\n")), + ("f_dir/dir_file_3", Some("bar\n")), ], ); test_env.jj_cmd_ok(&repo_path, &["rebase", "-r=@", "-d=first"]); @@ -639,20 +652,26 @@ fn test_files() { ); let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "all()", "--summary"]); - insta::assert_snapshot!(stdout, @r" - @ wqnwkozp test.user@example.com 2001-02-03 08:05:20 working_copy 89d772f3 + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" + @ wqnwkozp test.user@example.com 2001-02-03 08:05:20 working_copy 45c3a621 │ working_copy │ A f_added_2 │ M f_modified - ○ zsuskuln test.user@example.com 2001-02-03 08:05:11 second 12ffc2f7 + ○ zsuskuln test.user@example.com 2001-02-03 08:05:11 second 77a99380 │ second │ A f_added │ D f_deleted + │ A f_dir/dir_file_1 + │ A f_dir/dir_file_2 + │ A f_dir/dir_file_3 │ M f_modified │ A f_renamed - │ × royxmykx test.user@example.com 2001-02-03 08:05:14 conflicted 14453858 conflict + │ × royxmykx test.user@example.com 2001-02-03 08:05:14 conflicted 23eb154d conflict ├─╯ conflicted │ A f_added_2 + │ A f_dir/dir_file_1 + │ A f_dir/dir_file_2 + │ A f_dir/dir_file_3 │ M f_modified ○ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 first 2a2f433c │ first @@ -676,9 +695,10 @@ fn test_files() { let test_env = test_env; let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "file", "show", "f_"]); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_added f_added_2 + f_dir/ f_modified f_not_yet_renamed f_renamed @@ -687,27 +707,47 @@ fn test_files() { let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "file", "annotate", "-r@-", "f_"]); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_added + f_dir/ f_modified f_not_yet_renamed f_renamed f_unchanged "); - let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "diff", "-r", "@-", "f_"]); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_added Added f_deleted Deleted + f_dir/ f_modified Modified f_renamed Added "); + + let stdout = test_env.jj_cmd_success( + &repo_path, + &[ + "--", + "jj", + "diff", + "-r", + "@-", + &format!("f_dir{}", std::path::MAIN_SEPARATOR), + ], + ); + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" + f_dir/dir_file_1 Added + f_dir/dir_file_2 Added + f_dir/dir_file_3 Added + "); + let stdout = test_env.jj_cmd_success( &repo_path, &["--", "jj", "diff", "--from", "root()", "--to", "@-", "f_"], ); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_added Added + f_dir/ f_modified Added f_not_yet_renamed Added f_renamed Added @@ -726,7 +766,7 @@ fn test_files() { "f_", ], ); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_interdiff_only_from Added f_interdiff_same Added f_interdiff_only_to Added @@ -735,7 +775,7 @@ fn test_files() { // squash has a different behavior with --from and --to flags let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "squash", "-f=first", "f_"]); - insta::assert_snapshot!(stdout, @r" + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" f_deleted Added f_modified Added f_not_yet_renamed Added @@ -744,5 +784,8 @@ fn test_files() { let stdout = test_env.jj_cmd_success(&repo_path, &["--", "jj", "resolve", "-r=conflicted", "f_"]); - insta::assert_snapshot!(stdout, @"f_modified"); + insta::assert_snapshot!(stdout.replace('\\', "/"), @r" + f_dir/ + f_modified + "); }