From 6600b3edfbd0d7ac58e604b14b4336f69dcbea3b Mon Sep 17 00:00:00 2001
From: Anton Sagel <76914086+Ancient77@users.noreply.github.com>
Date: Mon, 9 Sep 2024 20:39:18 +0200
Subject: [PATCH] Expand multiple dots in path in completions (#13725)

# Description
This is my first PR, and I'm looking for feedback to help me improve!

This PR fixes #13380 by expanding the path prior to parsing it.
Also I've removed some unused code in
[completion_common.rs](https://github.com/Ancient77/nushell/blob/84e92bb02c78d4bb71a045fbbe73b9351ab8d16a/crates/nu-cli/src/completions/completion_common.rs
)
# User-Facing Changes

Auto-completion for "cd .../" now works by expanding to "cd ../../".

# Tests + Formatting

Formatted and added 2 tests for triple dots in the middle of a path and
at the end.
Also added a test for the expand_ndots() function.
---
 .../src/completions/completion_common.rs      |  66 ++++-
 crates/nu-cli/tests/completions/mod.rs        | 240 +++++++++++++++++-
 crates/nu-path/src/dots.rs                    |  11 +
 .../folder_inside_folder/myfile               |   0
 4 files changed, 303 insertions(+), 14 deletions(-)
 create mode 100644 tests/fixtures/completions/directory_completion/folder_inside_folder/myfile

diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs
index 8aae72b4f3..77ae165c73 100644
--- a/crates/nu-cli/src/completions/completion_common.rs
+++ b/crates/nu-cli/src/completions/completion_common.rs
@@ -1,3 +1,4 @@
+use super::MatchAlgorithm;
 use crate::{
     completions::{matches, CompletionOptions},
     SemanticSuggestion,
@@ -5,6 +6,7 @@ use crate::{
 use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher};
 use nu_ansi_term::Style;
 use nu_engine::env_to_string;
+use nu_path::dots::expand_ndots;
 use nu_path::{expand_to_real_path, home_dir};
 use nu_protocol::{
     engine::{EngineState, Stack, StateWorkingSet},
@@ -13,8 +15,6 @@ use nu_protocol::{
 use nu_utils::get_ls_colors;
 use std::path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP};
 
-use super::MatchAlgorithm;
-
 #[derive(Clone, Default)]
 pub struct PathBuiltFromString {
     parts: Vec<String>,
@@ -41,7 +41,7 @@ pub fn complete_rec(
     let mut completions = vec![];
 
     if let Some((&base, rest)) = partial.split_first() {
-        if (base == "." || base == "..") && (isdir || !rest.is_empty()) {
+        if base.chars().all(|c| c == '.') && (isdir || !rest.is_empty()) {
             let mut built = built.clone();
             built.parts.push(base.to_string());
             built.isdir = true;
@@ -156,16 +156,25 @@ pub fn complete_item(
     engine_state: &EngineState,
     stack: &Stack,
 ) -> Vec<(nu_protocol::Span, String, Option<Style>)> {
-    let partial = surround_remove(partial);
-    let isdir = partial.ends_with(is_separator);
+    let cleaned_partial = surround_remove(partial);
+    let isdir = cleaned_partial.ends_with(is_separator);
+    let expanded_partial = expand_ndots(Path::new(&cleaned_partial));
+    let should_collapse_dots = expanded_partial != Path::new(&cleaned_partial);
+    let mut partial = expanded_partial.to_string_lossy().to_string();
 
     #[cfg(unix)]
     let path_separator = SEP;
     #[cfg(windows)]
-    let path_separator = partial
+    let path_separator = cleaned_partial
         .chars()
         .rfind(|c: &char| is_separator(*c))
         .unwrap_or(SEP);
+
+    // Handle the trailing dot case
+    if cleaned_partial.ends_with(&format!("{path_separator}.")) {
+        partial.push_str(&format!("{path_separator}."));
+    }
+
     let cwd_pathbuf = Path::new(cwd).to_path_buf();
     let ls_colors = (engine_state.config.completions.use_ls_colors
         && engine_state.config.use_ansi_coloring)
@@ -185,16 +194,11 @@ pub fn complete_item(
     match components.peek().cloned() {
         Some(c @ Component::Prefix(..)) => {
             // windows only by definition
-            components.next();
-            if let Some(Component::RootDir) = components.peek().cloned() {
-                components.next();
-            };
             cwd = [c, Component::RootDir].iter().collect();
             prefix_len = c.as_os_str().len();
             original_cwd = OriginalCwd::Prefix(c.as_os_str().to_string_lossy().into_owned());
         }
         Some(c @ Component::RootDir) => {
-            components.next();
             // This is kind of a hack. When joining an empty string with the rest,
             // we add the slash automagically
             cwd = PathBuf::from(c.as_os_str());
@@ -202,7 +206,6 @@ pub fn complete_item(
             original_cwd = OriginalCwd::Prefix(String::new());
         }
         Some(Component::Normal(home)) if home.to_string_lossy() == "~" => {
-            components.next();
             cwd = home_dir().map(Into::into).unwrap_or(cwd_pathbuf);
             prefix_len = 1;
             original_cwd = OriginalCwd::Home;
@@ -227,7 +230,10 @@ pub fn complete_item(
         isdir,
     )
     .into_iter()
-    .map(|p| {
+    .map(|mut p| {
+        if should_collapse_dots {
+            p = collapse_ndots(p);
+        }
         let path = original_cwd.apply(p, path_separator);
         let style = ls_colors.as_ref().map(|lsc| {
             lsc.style_for_path_with_metadata(
@@ -340,3 +346,37 @@ pub fn sort_completions<T>(
 
     items
 }
+
+/// Collapse multiple ".." components into n-dots.
+///
+/// It performs the reverse operation of `expand_ndots`, collapsing sequences of ".." into n-dots,
+/// such as "..." and "....".
+///
+/// The resulting path will use platform-specific path separators, regardless of what path separators were used in the input.
+fn collapse_ndots(path: PathBuiltFromString) -> PathBuiltFromString {
+    let mut result = PathBuiltFromString {
+        parts: Vec::with_capacity(path.parts.len()),
+        isdir: path.isdir,
+    };
+
+    let mut dot_count = 0;
+
+    for part in path.parts {
+        if part == ".." {
+            dot_count += 1;
+        } else {
+            if dot_count > 0 {
+                result.parts.push(".".repeat(dot_count + 1));
+                dot_count = 0;
+            }
+            result.parts.push(part);
+        }
+    }
+
+    // Add any remaining dots
+    if dot_count > 0 {
+        result.parts.push(".".repeat(dot_count + 1));
+    }
+
+    result
+}
diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs
index 3dc4443bc3..2c0a98688c 100644
--- a/crates/nu-cli/tests/completions/mod.rs
+++ b/crates/nu-cli/tests/completions/mod.rs
@@ -339,7 +339,7 @@ fn file_completions() {
     match_suggestions(&expected_paths, &suggestions);
 
     // Test completions for hidden files
-    let target_dir = format!("ls {}{MAIN_SEPARATOR}.", folder(dir.join(".hidden_folder")));
+    let target_dir = format!("ls {}", file(dir.join(".hidden_folder").join(".")));
     let suggestions = completer.complete(&target_dir, target_dir.len());
 
     let expected_paths: Vec<String> =
@@ -564,6 +564,58 @@ fn partial_completions() {
     match_suggestions(&expected_paths, &suggestions);
 }
 
+#[test]
+fn partial_completion_with_dot_expansions() {
+    let (dir, _, engine, stack) = new_partial_engine();
+
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+
+    let dir_str = file(
+        dir.join("par")
+            .join("...")
+            .join("par")
+            .join("fi")
+            .join("so"),
+    );
+    let target_dir = format!("rm {dir_str}");
+    let suggestions = completer.complete(&target_dir, target_dir.len());
+
+    // Create the expected values
+    let expected_paths: Vec<String> = vec![
+        file(
+            dir.join("partial")
+                .join("...")
+                .join("partial_completions")
+                .join("final_partial")
+                .join("somefile"),
+        ),
+        file(
+            dir.join("partial-a")
+                .join("...")
+                .join("partial_completions")
+                .join("final_partial")
+                .join("somefile"),
+        ),
+        file(
+            dir.join("partial-b")
+                .join("...")
+                .join("partial_completions")
+                .join("final_partial")
+                .join("somefile"),
+        ),
+        file(
+            dir.join("partial-c")
+                .join("...")
+                .join("partial_completions")
+                .join("final_partial")
+                .join("somefile"),
+        ),
+    ];
+
+    // Match the results
+    match_suggestions(&expected_paths, &suggestions);
+}
+
 #[test]
 fn command_ls_with_filecompletion() {
     let (_, _, engine, stack) = new_engine();
@@ -953,6 +1005,192 @@ fn folder_with_directorycompletions() {
     match_suggestions(&expected_paths, &suggestions);
 }
 
+#[test]
+fn folder_with_directorycompletions_with_dots() {
+    // Create a new engine
+    let (dir, _, engine, stack) = new_engine();
+    let dir_str = dir
+        .join("directory_completion")
+        .join("folder_inside_folder")
+        .into_os_string()
+        .into_string()
+        .unwrap();
+
+    // Instantiate a new completer
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+
+    // Test completions for the current folder
+    let target_dir = format!("cd {dir_str}{MAIN_SEPARATOR}..{MAIN_SEPARATOR}");
+    let suggestions = completer.complete(&target_dir, target_dir.len());
+
+    // Create the expected values
+    let expected_paths: Vec<String> = vec![folder(
+        dir.join("directory_completion")
+            .join("folder_inside_folder")
+            .join("..")
+            .join("folder_inside_folder"),
+    )];
+
+    #[cfg(windows)]
+    {
+        let target_dir = format!("cd {dir_str}/../");
+        let slash_suggestions = completer.complete(&target_dir, target_dir.len());
+
+        let expected_slash_paths: Vec<String> = expected_paths
+            .iter()
+            .map(|s| s.replace('\\', "/"))
+            .collect();
+
+        match_suggestions(&expected_slash_paths, &slash_suggestions);
+    }
+
+    // Match the results
+    match_suggestions(&expected_paths, &suggestions);
+}
+
+#[test]
+fn folder_with_directorycompletions_with_three_trailing_dots() {
+    // Create a new engine
+    let (dir, _, engine, stack) = new_engine();
+    let dir_str = dir
+        .join("directory_completion")
+        .join("folder_inside_folder")
+        .into_os_string()
+        .into_string()
+        .unwrap();
+
+    // Instantiate a new completer
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+
+    // Test completions for the current folder
+    let target_dir = format!("cd {dir_str}{MAIN_SEPARATOR}...{MAIN_SEPARATOR}");
+    let suggestions = completer.complete(&target_dir, target_dir.len());
+
+    // Create the expected values
+    let expected_paths: Vec<String> = vec![
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("...")
+                .join("another"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("...")
+                .join("directory_completion"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("...")
+                .join("test_a"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("...")
+                .join("test_b"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("...")
+                .join(".hidden_folder"),
+        ),
+    ];
+
+    #[cfg(windows)]
+    {
+        let target_dir = format!("cd {dir_str}/.../");
+        let slash_suggestions = completer.complete(&target_dir, target_dir.len());
+
+        let expected_slash_paths: Vec<String> = expected_paths
+            .iter()
+            .map(|s| s.replace('\\', "/"))
+            .collect();
+
+        match_suggestions(&expected_slash_paths, &slash_suggestions);
+    }
+
+    // Match the results
+    match_suggestions(&expected_paths, &suggestions);
+}
+
+#[test]
+fn folder_with_directorycompletions_do_not_collapse_dots() {
+    // Create a new engine
+    let (dir, _, engine, stack) = new_engine();
+    let dir_str = dir
+        .join("directory_completion")
+        .join("folder_inside_folder")
+        .into_os_string()
+        .into_string()
+        .unwrap();
+
+    // Instantiate a new completer
+    let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
+
+    // Test completions for the current folder
+    let target_dir = format!("cd {dir_str}{MAIN_SEPARATOR}..{MAIN_SEPARATOR}..{MAIN_SEPARATOR}");
+    let suggestions = completer.complete(&target_dir, target_dir.len());
+
+    // Create the expected values
+    let expected_paths: Vec<String> = vec![
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("..")
+                .join("..")
+                .join("another"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("..")
+                .join("..")
+                .join("directory_completion"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("..")
+                .join("..")
+                .join("test_a"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("..")
+                .join("..")
+                .join("test_b"),
+        ),
+        folder(
+            dir.join("directory_completion")
+                .join("folder_inside_folder")
+                .join("..")
+                .join("..")
+                .join(".hidden_folder"),
+        ),
+    ];
+
+    #[cfg(windows)]
+    {
+        let target_dir = format!("cd {dir_str}/../../");
+        let slash_suggestions = completer.complete(&target_dir, target_dir.len());
+
+        let expected_slash_paths: Vec<String> = expected_paths
+            .iter()
+            .map(|s| s.replace('\\', "/"))
+            .collect();
+
+        match_suggestions(&expected_slash_paths, &slash_suggestions);
+    }
+
+    // Match the results
+    match_suggestions(&expected_paths, &suggestions);
+}
+
 #[test]
 fn variables_completions() {
     // Create a new engine
diff --git a/crates/nu-path/src/dots.rs b/crates/nu-path/src/dots.rs
index 6f46bb09c3..8d6341cb46 100644
--- a/crates/nu-path/src/dots.rs
+++ b/crates/nu-path/src/dots.rs
@@ -152,6 +152,17 @@ mod test_expand_ndots {
         };
         assert_path_eq!(expand_ndots(path), expected);
     }
+
+    #[test]
+    fn trailing_dots() {
+        let path = Path::new("/foo/bar/..");
+        let expected = if cfg!(windows) {
+            r"\foo\bar\.."
+        } else {
+            "/foo/bar/.."
+        };
+        assert_path_eq!(expand_ndots(path), expected);
+    }
 }
 
 #[cfg(test)]
diff --git a/tests/fixtures/completions/directory_completion/folder_inside_folder/myfile b/tests/fixtures/completions/directory_completion/folder_inside_folder/myfile
new file mode 100644
index 0000000000..e69de29bb2