Revert "nu-cli/completions: better fix for files with special characters (#5254)" (#5372)

This reverts commit 3cf3329e49.

Cargo.lock

@@ -3716,7 +3716,7 @@ dependencies = [
 [[package]]
 name = "reedline"
 version = "0.4.0"
-source = "git+https://github.com/nushell/reedline?branch=main#229c729898ef91bf9cb83a5420a6fe7b4c8d1045"
+source = "git+https://github.com/nushell/reedline?branch=main#2e2bdc54621643e7bee5ba2e2d9bf28e757074e0"
 dependencies = [
  "chrono",
  "crossterm",

crates/nu-cli/src/completions/file_completions.rs

@@ -134,8 +134,14 @@ pub fn file_path_completion(
                             file_name.push(SEP);
                         }
 
-                        // Escape path string if necessary
+                        if path.contains(' ') {
-                        path = escape_path_str(path);
+                            path = format!("\'{}\'", path);
+                        }
+
+                        // Fix files or folders with quotes
+                        if path.contains('\'') || path.contains('"') {
+                            path = format!("`{}`", path);
+                        }
 
                         Some((span, path))
                     } else {
@@ -152,85 +158,3 @@ pub fn file_path_completion(
 pub fn matches(partial: &str, from: &str, match_algorithm: MatchAlgorithm) -> bool {
     match_algorithm.matches_str(&from.to_ascii_lowercase(), &partial.to_ascii_lowercase())
 }
-
-// escape paths that contains some special characters
-pub fn escape_path_str(path: String) -> String {
-    let mut path = path;
-
-    // List of special characters that need to be escaped
-    let special_characters = b"\\\'\"";
-    let replacements = [b"\\\\", b"\\\'", b"\\\""];
-
-    // Check if path needs to be escaped
-    let needs_escape = path.bytes().fold(false, |acc, x| {
-        acc
-        || x == b'\\' // 0x5c
-        || x == b'`' // 0x60
-        || x == b'"'
-        || x == b' '
-        || x == b'\''
-    });
-
-    if needs_escape {
-        let mut result: Vec<u8> = vec![b'\"'];
-
-        // Walk through the path characters
-        for b in path.bytes() {
-            // Basically the equivalent of str.find(), but expanded
-            if let Some(idx) = special_characters.iter().enumerate().fold(None, |idx, c| {
-                if *c.1 == b {
-                    Some(c.0)
-                } else {
-                    idx
-                }
-            }) {
-                for rb in replacements[idx] {
-                    result.push(*rb);
-                }
-            } else {
-                result.push(b);
-            }
-        }
-
-        // Final quote
-        result.push(b'\"');
-
-        // Update path
-        path = String::from_utf8(result).unwrap_or(path);
-    }
-
-    path
-}
-
-mod test {
-    #[test]
-    fn escape_path() {
-        // Vec of (path, expected escape)
-        let cases: Vec<(&str, &str)> = vec![
-            ("/home/nushell/filewith`", "\"/home/nushell/filewith`\""),
-            (
-                "/home/nushell/folder with spaces",
-                "\"/home/nushell/folder with spaces\"",
-            ),
-            (
-                "/home/nushell/folder\"withquotes",
-                "\"/home/nushell/folder\\\"withquotes\"",
-            ),
-            (
-                "C:\\windows\\system32\\escape path",
-                "\"C:\\\\windows\\\\system32\\\\escape path\"",
-            ),
-            (
-                "/home/nushell/shouldnt/be/escaped",
-                "/home/nushell/shouldnt/be/escaped",
-            ),
-        ];
-
-        for item in cases.into_iter() {
-            assert_eq!(
-                crate::completions::escape_path_str(item.0.to_string()),
-                item.1.to_string()
-            )
-        }
-    }
-}

crates/nu-cli/src/completions/mod.rs

@@ -16,6 +16,6 @@ pub use completion_options::{CompletionOptions, MatchAlgorithm, SortBy};
 pub use custom_completions::CustomCompletion;
 pub use directory_completions::DirectoryCompletion;
 pub use dotnu_completions::DotNuCompletion;
-pub use file_completions::{escape_path_str, file_path_completion, partial_from, FileCompletion};
+pub use file_completions::{file_path_completion, partial_from, FileCompletion};
 pub use flag_completions::FlagCompletion;
 pub use variable_completions::VariableCompletion;

crates/nu-cli/tests/test_completions.rs

@@ -8,7 +8,6 @@ use reedline::{Completer, Suggestion};
 const SEP: char = std::path::MAIN_SEPARATOR;
 
 #[test]
-#[cfg(not(target_os = "windows"))]
 fn file_completions() {
     // Create a new engine
     let (dir, dir_str, engine) = new_engine();
@@ -24,11 +23,12 @@ fn file_completions() {
 
     // Create the expected values
     let expected_paths: Vec<String> = vec![
-        folder(dir.clone().join("test_a")),
+        file(dir.join("nushell")),
-        folder(dir.clone().join("test_b")),
+        folder(dir.join("test_a")),
-        folder(dir.clone().join("another")),
+        folder(dir.join("test_b")),
-        file(dir.clone().join(".hidden_file")),
+        folder(dir.join("another")),
-        folder(dir.clone().join(".hidden_folder")),
+        file(dir.join(".hidden_file")),
+        folder(dir.join(".hidden_folder")),
     ];
 
     // Match the results
@@ -46,7 +46,6 @@ fn file_completions() {
 }
 
 #[test]
-#[cfg(not(target_os = "windows"))]
 fn folder_completions() {
     // Create a new engine
     let (dir, dir_str, engine) = new_engine();
@@ -88,26 +87,9 @@ pub fn new_engine() -> (PathBuf, String, EngineState) {
 }
 
 // match a list of suggestions with the expected values
-// skipping the order (for now) due to some issue with sorting behaving
+pub fn match_suggestions(expected: Vec<String>, suggestions: Vec<Suggestion>) {
-// differently for each OS
+    expected.iter().zip(suggestions).for_each(|it| {
-fn match_suggestions(expected: Vec<String>, suggestions: Vec<Suggestion>) {
+        assert_eq!(it.0, &it.1.value);
-    suggestions.into_iter().for_each(|it| {
-        let items = expected.clone();
-        let result = items.into_iter().find(|x| {
-            let mut current_item = x.clone();
-
-            // For windows the expected should also escape "\"
-            if cfg!(windows) {
-                current_item = current_item.replace("\\", "\\\\");
-            }
-
-            &current_item == &it.value
-        });
-
-        match result {
-            Some(val) => assert_eq!(val, it.value),
-            None => panic!("the path {} is not expected", it.value),
-        }
     });
 }