From 74f62305b289498bb9a182f2b1185d769215ddd6 Mon Sep 17 00:00:00 2001 From: zc he Date: Sat, 15 Mar 2025 22:17:59 +0800 Subject: [PATCH] fix(completion): more quoting for file_completion/directory_completion (#15299) # Description Found inconsistent behaviors of `directory_completion` and `file_completion`, https://github.com/nushell/nushell/issues/13951 https://github.com/nushell/reedline/pull/886 Also there're failing cases with such file names/dir names `foo(`, `foo{`, `foo[`. I think it doesn't harm to be more conservative at adding quotes, even if it might be unnecessary for paired names like `foo{}`. # User-Facing Changes # Tests + Formatting Adjusted # After Submitting --- .../src/completions/completion_common.rs | 38 +++++++++---------- crates/nu-cli/tests/completions/mod.rs | 24 ++++++++++++ .../partial_completions/partial-d(/.gitkeep | 0 .../quoted_completions/curly-bracket_{.txt | 0 .../quoted_completions/semicolon_;.txt | 0 .../quoted_completions/square-bracket_[.txt | 0 6 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 tests/fixtures/partial_completions/partial-d(/.gitkeep create mode 100644 tests/fixtures/quoted_completions/curly-bracket_{.txt create mode 100644 tests/fixtures/quoted_completions/semicolon_;.txt create mode 100644 tests/fixtures/quoted_completions/square-bracket_[.txt diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index 31a4bdd75b..7d4cc670f8 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -276,7 +276,7 @@ pub fn complete_item( }); FileSuggestion { span, - path: escape_path(path, want_directory), + path: escape_path(path), style, is_dir, } @@ -285,30 +285,30 @@ pub fn complete_item( } // Fix files or folders with quotes or hashes -pub fn escape_path(path: String, dir: bool) -> String { +pub fn escape_path(path: String) -> String { // make glob pattern have the highest priority. - if nu_glob::is_glob(path.as_str()) { + if nu_glob::is_glob(path.as_str()) || path.contains('`') { + // expand home `~` for https://github.com/nushell/nushell/issues/13905 let pathbuf = nu_path::expand_tilde(path); let path = pathbuf.to_string_lossy(); - return if path.contains('\'') { - // decide to use double quote, also need to escape `"` in path - // or else users can't do anything with completed path either. - format!("\"{}\"", path.replace('"', r#"\""#)) + if path.contains('\'') { + // decide to use double quotes + // Path as Debug will do the escaping for `"`, `\` + format!("{:?}", path) } else { format!("'{path}'") - }; - } - - let filename_contaminated = !dir && path.contains(['\'', '"', ' ', '#', '(', ')']); - let dirname_contaminated = dir && path.contains(['\'', '"', ' ', '#']); - let maybe_flag = path.starts_with('-'); - let maybe_variable = path.starts_with('$'); - let maybe_number = path.parse::().is_ok(); - if filename_contaminated || dirname_contaminated || maybe_flag || maybe_variable || maybe_number - { - format!("`{path}`") + } } else { - path + let contaminated = + path.contains(['\'', '"', ' ', '#', '(', ')', '{', '}', '[', ']', '|', ';']); + let maybe_flag = path.starts_with('-'); + let maybe_variable = path.starts_with('$'); + let maybe_number = path.parse::().is_ok(); + if contaminated || maybe_flag || maybe_variable || maybe_number { + format!("`{path}`") + } else { + path + } } } diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index 5890aa21da..0326c9ba84 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -896,6 +896,7 @@ fn partial_completions() { folder(dir.join("partial-a")), folder(dir.join("partial-b")), folder(dir.join("partial-c")), + format!("`{}`", folder(dir.join("partial-d("))), ]; // Match the results @@ -938,6 +939,7 @@ fn partial_completions() { file(dir.join("partial-b").join("hello_b")), file(dir.join("partial-b").join("hi_b")), file(dir.join("partial-c").join("hello_c")), + format!("`{}`", file(dir.join("partial-d(").join(".gitkeep"))), ]; // Match the results @@ -985,6 +987,15 @@ fn partial_completions() { .join("final_partial") .join("somefile"), ), + format!( + "`{}`", + file( + dir.join("partial-d(") + .join("..") + .join("final_partial") + .join("somefile"), + ) + ), ]; // Match the results @@ -1065,6 +1076,16 @@ fn partial_completion_with_dot_expansions() { .join("final_partial") .join("somefile"), ), + format!( + "`{}`", + file( + dir.join("partial-d(") + .join("...") + .join("partial_completions") + .join("final_partial") + .join("somefile"), + ) + ), ]; // Match the results @@ -1389,6 +1410,9 @@ fn file_completion_quoted() { "`-inf`", "`4.2`", "\'[a] bc.txt\'", + "`curly-bracket_{.txt`", + "`semicolon_;.txt`", + "'square-bracket_[.txt'", "`te st.txt`", "`te#st.txt`", "`te'st.txt`", diff --git a/tests/fixtures/partial_completions/partial-d(/.gitkeep b/tests/fixtures/partial_completions/partial-d(/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/quoted_completions/curly-bracket_{.txt b/tests/fixtures/quoted_completions/curly-bracket_{.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/quoted_completions/semicolon_;.txt b/tests/fixtures/quoted_completions/semicolon_;.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/quoted_completions/square-bracket_[.txt b/tests/fixtures/quoted_completions/square-bracket_[.txt new file mode 100644 index 0000000000..e69de29bb2