From 180290f3a8b4e117f044218e71f64be1457c9548 Mon Sep 17 00:00:00 2001 From: Jason Gedge Date: Thu, 2 Jul 2020 19:29:28 -0400 Subject: [PATCH] Remove custom escaping for external args. (#2095) Our own custom escaping unfortunately is far too simple to cover all cases. Instead, the parser will now do no transforms on the args passed to an external command, letting the process spawning library deal with doing the appropriate escaping. --- .../src/commands/classified/external.rs | 17 +++---------- crates/nu-parser/src/parse.rs | 24 +++++++++++++++++-- tests/shell/pipeline/commands/external.rs | 9 +++++++ 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/crates/nu-cli/src/commands/classified/external.rs b/crates/nu-cli/src/commands/classified/external.rs index 38b56c34d4..554a394ed6 100644 --- a/crates/nu-cli/src/commands/classified/external.rs +++ b/crates/nu-cli/src/commands/classified/external.rs @@ -118,29 +118,18 @@ async fn run_with_stdin( let value = evaluate_baseline_expr(arg, &context.registry, &scope.it, &scope.vars, &scope.env) .await?; + // Skip any arguments that don't really exist, treating them as optional // FIXME: we may want to preserve the gap in the future, though it's hard to say // what value we would put in its place. if value.value.is_none() { continue; } + // Do the cleanup that we need to do on any argument going out: let trimmed_value_string = value.as_string()?.trim_end_matches('\n').to_string(); - let value_string; - #[cfg(not(windows))] - { - value_string = trimmed_value_string - .replace('$', "\\$") - .replace('"', "\\\"") - .to_string() - } - #[cfg(windows)] - { - value_string = trimmed_value_string - } - - command_args.push(value_string); + command_args.push(trimmed_value_string); } let process_args = command_args diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs index 39024f983e..ca5b22fae9 100644 --- a/crates/nu-parser/src/parse.rs +++ b/crates/nu-parser/src/parse.rs @@ -512,6 +512,26 @@ fn parse_interpolated_string( (call, error) } +/// Parses the given argument using the shape as a guide for how to correctly parse the argument +fn parse_external_arg( + registry: &dyn SignatureRegistry, + lite_arg: &Spanned, +) -> (SpannedExpression, Option) { + if lite_arg.item.starts_with('$') { + return parse_full_column_path(&lite_arg, registry); + } + + if lite_arg.item.starts_with('`') && lite_arg.item.len() > 1 && lite_arg.item.ends_with('`') { + // This is an interpolated string + parse_interpolated_string(registry, &lite_arg) + } else { + ( + SpannedExpression::new(Expression::string(lite_arg.item.clone()), lite_arg.span), + None, + ) + } +} + /// Parses the given argument using the shape as a guide for how to correctly parse the argument fn parse_arg( expected_type: SyntaxShape, @@ -1237,7 +1257,7 @@ fn classify_pipeline( args.push(name); for lite_arg in &lite_cmd.args { - let (expr, err) = parse_arg(SyntaxShape::String, registry, lite_arg); + let (expr, err) = parse_external_arg(registry, lite_arg); if error.is_none() { error = err; } @@ -1313,7 +1333,7 @@ fn classify_pipeline( args.push(name); for lite_arg in &lite_cmd.args { - let (expr, err) = parse_arg(SyntaxShape::String, registry, lite_arg); + let (expr, err) = parse_external_arg(registry, lite_arg); if error.is_none() { error = err; } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 14e64f0bbf..4d1a17f4c7 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -192,6 +192,15 @@ mod external_words { assert_eq!(actual.out, "joturner@foo.bar.baz"); } + + #[test] + fn no_escaping_for_single_quoted_strings() { + let actual = nu!(cwd: ".", r#" + nu --testbin cococo 'test "things"' + "#); + + assert_eq!(actual.out, "test \"things\""); + } } mod nu_commands {