diff --git a/CHANGELOG.md b/CHANGELOG.md index fc19e69a62..202aacf5eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,10 @@ edits the change twice in some cases. * Fixed parsing of `files(expr)` revset expression including parentheses. [#7747](https://github.com/jj-vcs/jj/issues/7747) +* Shell completion now works with non‑normalized paths, fixing the previous + panic and allowing prefixes containing `.` or `..` to be completed correctly. + [#6861](https://github.com/jj-vcs/jj/issues/6861) + ## [0.34.0] - 2025-10-01 ### Release highlights diff --git a/cli/src/complete.rs b/cli/src/complete.rs index 34162c79e5..5d9e454dea 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -22,6 +22,7 @@ use clap_complete::CompletionCandidate; use indoc::indoc; use itertools::Itertools as _; use jj_lib::config::ConfigNamePathBuf; +use jj_lib::file_util::normalize_path; use jj_lib::settings::UserSettings; use jj_lib::workspace::DefaultWorkspaceLoaderFactory; use jj_lib::workspace::WorkspaceLoaderFactory as _; @@ -739,10 +740,57 @@ pub fn branch_name_equals_any_revision(current: &std::ffi::OsStr) -> Vec(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 is_path_separator(c: char) -> bool { + c == '/' || c == std::path::MAIN_SEPARATOR +} + +fn path_completion_candidate_from( + current_prefix: &str, + path: &str, + mode: Option<&str>, +) -> Option { + // The user-provided completion string might not prefix `path` unless cast in + // normal form. + let normalized_prefix_path = normalize_path(Path::new(current_prefix)); + let normalized_prefix = match normalized_prefix_path.to_str() { + None => current_prefix, + Some(".") => "", // `.` cannot be normalized further, but doesn't prefix `path`. + Some(normalized_prefix) => normalized_prefix, + }; + + let mut remainder = path.strip_prefix(normalized_prefix)?; + + // Trailing slash might have been normalized away in which case we need to strip + // the leading slash in the remainder away, or else the slash would appear + // twice. + if current_prefix.ends_with(is_path_separator) { + remainder = remainder + .strip_prefix(is_path_separator) + .unwrap_or(remainder); + } + + let (completion, help) = match remainder.split_inclusive(is_path_separator).at_most_one() { + // Completed component is the final component in `path`, so we're completing the file to + // which `mode` refers. + Ok(file_completion) => ( + file_completion?, + mode.map(|mode| match mode { + "modified" => "Modified".into(), + "removed" => "Deleted".into(), + "added" => "Added".into(), + "renamed" => "Renamed".into(), + "copied" => "Copied".into(), + _ => format!("unknown mode: '{mode}'").into(), + }), + ), + + // Omit `mode` when completing only up to the next directory. + Err(mut components) => (components.next().unwrap(), None), + }; + + // Completion candidate splices normalized completion onto the user-provided + // prefix. + Some(CompletionCandidate::new(format!("{current_prefix}{completion}")).help(help)) } fn current_prefix_to_fileset(current: &str) -> String { @@ -774,12 +822,7 @@ fn all_files_from_rev(rev: String, current: &std::ffi::OsStr) -> Vec CompletionCandidate { - if let Some(dir_path) = dir_prefix_from(path, current) { - return CompletionCandidate::new(dir_path); + let mut candidates: Vec<_> = stdout + .lines() + .filter_map(|line| line.split_once(' ')) + .filter_map(|(mode, path)| { + if mode == "renamed.source" { + path_completion_candidate_from(current, path, Some("renamed")) + .inspect(|_| include_renames |= true) + } else { + path_completion_candidate_from(current, path, Some(mode)) } + }) + .collect(); - let help = match mode { - "modified" => "Modified".into(), - "removed" => "Deleted".into(), - "added" => "Added".into(), - "renamed" => "Renamed".into(), - "copied" => "Copied".into(), - _ => format!("unknown mode: '{mode}'"), - }; - CompletionCandidate::new(path).help(Some(help.into())) - } - - if mode == "renamed.source" { - if !path.starts_with(current) { - continue; - } - candidates.push(path_to_candidate(current, "renamed", path)); - include_renames |= true; - } else { - candidates.push(path_to_candidate(current, mode, path)); - } - } if include_renames { candidates.sort_unstable_by(|a, b| Path::new(a.get_value()).cmp(Path::new(b.get_value()))); } @@ -874,16 +901,13 @@ fn conflicted_files_from_rev(rev: &str, current: &std::ffi::OsStr) -> Vec