fix arg parse (#5754)

* fix arg parse

* add ut, fix clippy

* simplify code

* fmt code

crates/nu-parser/src/deparse.rs

@@ -1,6 +1,3 @@
-use std::fs::File;
-use std::io::{BufRead, BufReader};
-
 pub fn escape_quote_string(input: &str) -> String {
     let mut output = String::with_capacity(input.len() + 2);
     output.push('"');
@@ -16,97 +13,80 @@ pub fn escape_quote_string(input: &str) -> String {
     output
 }
 
-fn looks_like_flag(input: &str) -> bool {
+// Escape rules:
-    if !input.starts_with('-') {
+// input argument contains ' '(like abc def), we will convert it to `abc def`.
-        false
+// input argument contains --version='xx yy', we will convert it to --version=`'xx yy'`
-        // it does not start with  '-'
+// input argument contains " or \, we will try to escape input.
-    } else if !input.starts_with("--") {
+pub fn escape_for_script_arg(input: &str) -> String {
-        if input.len() > 2
+    // handle for flag, maybe we need to escape the value.
-            && input.chars().nth(2).expect("this should never trigger") != '='
+    if input.starts_with("--") {
-            && input.chars().nth(2).expect("this should never trigger") != ' '
+        if let Some((arg_name, arg_val)) = input.split_once('=') {
-        {
+            // only want to escape arg_val.
-            false
+            let arg_val = if arg_val.contains(' ') {
-            // while it start with '-', it is not of the form '-x=y' or '-x y'
+                format!("`{}`", arg_val)
-        } else {
+            } else if arg_val.contains('"') || arg_val.contains('\\') {
-            input.len() >= 2
+                escape_quote_string(arg_val)
+            } else {
+                arg_val.into()
+            };
+            return format!("{}={}", arg_name, arg_val);
         }
+    }
+
+    if input.contains(' ') {
+        format!("`{}`", input)
+    } else if input.contains('"') || input.contains('\\') {
+        escape_quote_string(input)
     } else {
-        input.len() > 2
+        input.to_string()
-        // it is either a flag --x or a '--'
     }
 }
 
-fn escape_quote_string_when_flags_are_unclear(input: &str) -> String {
+#[cfg(test)]
-    // internal use only. When reading the file for flags goes wrong, revert back to a manual check
+mod test {
-    // for flags.
+    use super::escape_for_script_arg;
-    let mut output = String::new();
-    if !looks_like_flag(input) {
-        output.push('"');
-        for c in input.chars() {
-            if c == '"' || c == '\\' {
-                output.push('\\');
-            }
-            output.push(c);
-        }
-        output.push('"');
-        output
-    } else if input.contains(' ') || input.contains('=') {
-        // this is a flag that requires delicate handling
-        let mut flag_tripped = false;
-        for c in input.chars() {
-            if c == '"' || c == '\\' {
-                output.push('\\');
-            }
-            output.push(c);
-            if (c == ' ' || c == '=') && !flag_tripped {
-                flag_tripped = true;
-                output.push('"');
-            }
-        }
-        output.push('"');
-        output
-    } else {
-        // this is a normal flag, aka "--x"
-        String::from(input)
-    }
-}
 
-pub fn escape_quote_string_with_file(input: &str, file: &str) -> String {
+    #[test]
-    // use when you want to cross-compare to a file to ensure flags are checked properly
+    fn test_not_extra_quote() {
-    let file = File::open(file);
+        // check for input arg like this:
-    match file {
+        // nu b.nu 8
-        Ok(f) => {
+        assert_eq!(escape_for_script_arg("8"), "8".to_string());
-            let lines = BufReader::new(f).lines();
+    }
-            for line in lines {
+
-                let mut flag_start = false;
+    #[test]
-                let mut word = String::new();
+    fn test_arg_with_flag() {
-                let line_or = line.unwrap_or_else(|_| String::from(" "));
+        // check for input arg like this:
-                if line_or.contains('-') {
+        // nu b.nu linux --version=v5.2
-                    for n in line_or.chars() {
+        assert_eq!(escape_for_script_arg("linux"), "linux".to_string());
-                        if n == '-' {
+        assert_eq!(
-                            flag_start = true;
+            escape_for_script_arg("--version=v5.2"),
-                        }
+            "--version=v5.2".to_string()
-                        if n == ' ' || n == ':' || n == ')' {
+        );
-                            flag_start = false;
+
-                        }
+        // check for input arg like this:
-                        if flag_start {
+        // nu b.nu linux --version v5.2
-                            word.push(n);
+        assert_eq!(escape_for_script_arg("--version"), "--version".to_string());
-                        }
+        assert_eq!(escape_for_script_arg("v5.2"), "v5.2".to_string());
-                    }
+    }
-                }
+
-                if word.contains(input) || {
+    #[test]
-                    let s: Vec<&str> = input.split('=').collect();
+    fn test_flag_arg_with_values_contains_space() {
-                    word.contains(s[0])
+        // check for input arg like this:
-                } {
+        // nu b.nu test_ver --version='xx yy' --arch=ghi
-                    return input.to_string();
+        assert_eq!(
-                }
+            escape_for_script_arg("--version='xx yy'"),
-            }
+            "--version=`'xx yy'`".to_string()
-            let mut final_word = String::new();
+        );
-            final_word.push('"');
+        assert_eq!(
-            final_word.push_str(input);
+            escape_for_script_arg("--arch=ghi"),
-            final_word.push('"');
+            "--arch=ghi".to_string()
-            final_word
+        );
-        }
+    }
-        _ => escape_quote_string_when_flags_are_unclear(input),
+
+    #[test]
+    fn test_escape() {
+        // check for input arg like this:
+        // nu b.nu '"'
+        assert_eq!(escape_for_script_arg(r#"""#), r#""\"""#.to_string());
     }
 }

crates/nu-parser/src/lib.rs

@@ -8,7 +8,7 @@ mod parse_keywords;
 mod parser;
 mod type_check;
 
-pub use deparse::{escape_quote_string, escape_quote_string_with_file};
+pub use deparse::{escape_for_script_arg, escape_quote_string};
 pub use errors::ParseError;
 pub use flatten::{flatten_block, flatten_expression, flatten_pipeline, FlatShape};
 pub use known_external::KnownExternal;

src/main.rs

@@ -17,7 +17,7 @@ use nu_cli::{
 };
 use nu_command::{create_default_context, BufferedReader};
 use nu_engine::{get_full_help, CallExt};
-use nu_parser::{escape_quote_string, escape_quote_string_with_file, parse};
+use nu_parser::{escape_for_script_arg, escape_quote_string, parse};
 use nu_protocol::{
     ast::{Call, Expr, Expression},
     engine::{Command, EngineState, Stack, StateWorkingSet},
@@ -93,7 +93,7 @@ fn main() -> Result<()> {
     let mut args = std::env::args().skip(1);
     while let Some(arg) = args.next() {
         if !script_name.is_empty() {
-            args_to_script.push(escape_quote_string_with_file(&arg, &script_name));
+            args_to_script.push(escape_for_script_arg(&arg));
         } else if arg.starts_with('-') {
             // Cool, it's a flag
             let flag_value = match arg.as_ref() {