Fix parsing of strings with special characters

crates/nu-parser/src/deparse.rs

@@ -1,3 +1,10 @@
+fn string_should_be_quoted(input: &str) -> bool {
+    input.starts_with('$')
+        || input
+            .chars()
+            .any(|c| c == ' ' || c == '(' || c == '\'' || c == '`' || c == '"' || c == '\\')
+}
+
 pub fn escape_quote_string(input: &str) -> String {
     let mut output = String::with_capacity(input.len() + 2);
     output.push('"');
@@ -14,37 +21,32 @@ pub fn escape_quote_string(input: &str) -> String {
 }
 
 // Escape rules:
-// input argument contains ' '(like abc def), we will convert it to `abc def`.
-// input argument contains --version='xx yy', we will convert it to --version=`'xx yy'`
-// input argument contains " or \, we will try to escape input.
+// input argument is not a flag, does not start with $ and doesn't contain special characters, it is passed as it is (foo -> foo)
+// input argument is not a flag and either starts with $ or contains special characters, quotes are added, " and \ are escaped (two \words -> "two \\words")
+// input argument is a flag without =, it's passed as it is (--foo -> --foo)
+// input argument is a flag with =, the first two points apply to the value (--foo=bar -> --foo=bar; --foo=bar' -> --foo="bar'")
+//
+// special characters are white space, (, ', `, ",and \
 pub fn escape_for_script_arg(input: &str) -> String {
     // handle for flag, maybe we need to escape the value.
     if input.starts_with("--") {
         if let Some((arg_name, arg_val)) = input.split_once('=') {
             // only want to escape arg_val.
-            let arg_val = if arg_val.contains(' ') {
-                format!("`{arg_val}`")
-            } else if arg_val.contains('"') || arg_val.contains('\\') {
+            let arg_val = if string_should_be_quoted(arg_val) {
                 escape_quote_string(arg_val)
-            } else if arg_val.is_empty() {
-                // return an empty string
-                "''".to_string()
             } else {
                 arg_val.into()
             };
+
             return format!("{arg_name}={arg_val}");
+        } else {
+            return input.into();
         }
     }
-
-    if input.contains(' ') {
-        format!("`{input}`")
-    } else if input.contains('"') || input.contains('\\') {
+    if string_should_be_quoted(input) {
         escape_quote_string(input)
-    } else if input.is_empty() {
-        // return an empty string
-        "''".to_string()
     } else {
-        input.to_string()
+        input.into()
     }
 }
 
@@ -55,15 +57,28 @@ mod test {
     #[test]
     fn test_not_extra_quote() {
         // check for input arg like this:
-        // nu b.nu 8
+        // nu b.nu word 8
+        assert_eq!(escape_for_script_arg("word"), "word".to_string());
         assert_eq!(escape_for_script_arg("8"), "8".to_string());
     }
 
+    #[test]
+    fn test_quote_special() {
+        // check for input arg like this:
+        // nu b.nu "two words" $nake "`123"
+        assert_eq!(
+            escape_for_script_arg("two words"),
+            r#""two words""#.to_string()
+        );
+        assert_eq!(escape_for_script_arg("$nake"), r#""$nake""#.to_string());
+        assert_eq!(escape_for_script_arg("`123"), r#""`123""#.to_string());
+    }
+
     #[test]
     fn test_arg_with_flag() {
         // check for input arg like this:
-        // nu b.nu linux --version=v5.2
-        assert_eq!(escape_for_script_arg("linux"), "linux".to_string());
+        // nu b.nu --linux --version=v5.2
+        assert_eq!(escape_for_script_arg("--linux"), "--linux".to_string());
         assert_eq!(
             escape_for_script_arg("--version=v5.2"),
             "--version=v5.2".to_string()
@@ -76,23 +91,27 @@ mod test {
     }
 
     #[test]
-    fn test_flag_arg_with_values_contains_space() {
+    fn test_flag_arg_with_values_contains_special() {
         // check for input arg like this:
-        // nu b.nu test_ver --version='xx yy' --arch=ghi
+        // nu b.nu test_ver --version='xx yy' --separator="`"
         assert_eq!(
             escape_for_script_arg("--version='xx yy'"),
-            "--version=`'xx yy'`".to_string()
+            r#"--version="'xx yy'""#.to_string()
         );
         assert_eq!(
-            escape_for_script_arg("--arch=ghi"),
-            "--arch=ghi".to_string()
+            escape_for_script_arg("--separator=`"),
+            r#"--separator="`""#.to_string()
         );
     }
 
     #[test]
     fn test_escape() {
         // check for input arg like this:
-        // nu b.nu '"'
-        assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string());
+        // nu b.nu \ --arg='"'
+        assert_eq!(escape_for_script_arg(r#"\"#), r#""\\""#.to_string());
+        assert_eq!(
+            escape_for_script_arg(r#"--arg=""#),
+            r#"--arg="\"""#.to_string()
+        );
     }
 }