From 98ab31e15e56a18910c70216c6abe51334fd8c30 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Mon, 2 May 2022 06:37:20 +1200 Subject: [PATCH] Move uses of trim_quotes to unescape for filenames (#5398) * Move uses of trim_quotes to unescape for filenames * Fix Windows tests --- .../src/completions/command_completions.rs | 9 ++-- crates/nu-command/tests/commands/cp.rs | 6 +-- crates/nu-parser/src/parse_keywords.rs | 54 ++++++++++--------- crates/nu-parser/src/parser.rs | 15 +++--- 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index 1531fc63db..bcd3360220 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -1,7 +1,7 @@ use crate::completions::{ file_completions::file_path_completion, Completer, CompletionOptions, MatchAlgorithm, SortBy, }; -use nu_parser::{trim_quotes, FlatShape}; +use nu_parser::{unescape_unquote_string, FlatShape}; use nu_protocol::{ engine::{EngineState, StateWorkingSet}, Span, @@ -237,9 +237,10 @@ impl Completer for CommandCompletion { .map(move |x| { if self.flat_idx == 0 { // We're in the command position - if x.1.starts_with('"') && !matches!(preceding_byte.get(0), Some(b'^')) { - let trimmed = trim_quotes(x.1.as_bytes()); - let trimmed = String::from_utf8_lossy(trimmed).to_string(); + if (x.1.starts_with('"') || x.1.starts_with('\'') || x.1.starts_with('`')) + && !matches!(preceding_byte.get(0), Some(b'^')) + { + let (trimmed, _) = unescape_unquote_string(x.1.as_bytes(), span); let expanded = nu_path::canonicalize_with(trimmed, &cwd); if let Ok(expanded) = expanded { diff --git a/crates/nu-command/tests/commands/cp.rs b/crates/nu-command/tests/commands/cp.rs index 81fe7c1e36..e49f519ef1 100644 --- a/crates/nu-command/tests/commands/cp.rs +++ b/crates/nu-command/tests/commands/cp.rs @@ -8,7 +8,7 @@ fn copies_a_file() { Playground::setup("cp_test_1", |dirs, _| { nu!( cwd: dirs.root(), - "cp \"{}\" cp_test_1/sample.ini", + "cp `{}` cp_test_1/sample.ini", dirs.formats().join("sample.ini") ); @@ -171,13 +171,13 @@ fn copies_same_file_twice() { Playground::setup("cp_test_8", |dirs, _| { nu!( cwd: dirs.root(), - "cp \"{}\" cp_test_8/sample.ini", + "cp `{}` cp_test_8/sample.ini", dirs.formats().join("sample.ini") ); nu!( cwd: dirs.root(), - "cp \"{}\" cp_test_8/sample.ini", + "cp `{}` cp_test_8/sample.ini", dirs.formats().join("sample.ini") ); diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index b460918f6a..a843619a99 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -23,7 +23,7 @@ use crate::{ parse_internal_call, parse_multispan_value, parse_signature, parse_string, parse_var_with_opt_type, trim_quotes, }, - ParseError, + unescape_unquote_string, ParseError, }; pub fn parse_def_predecl( @@ -1312,9 +1312,10 @@ pub fn parse_use( } else { // TODO: Do not close over when loading module from file // It could be a file - if let Ok(module_filename) = - String::from_utf8(trim_quotes(&import_pattern.head.name).to_vec()) - { + + let (module_filename, err) = + unescape_unquote_string(&import_pattern.head.name, import_pattern.head.span); + if err.is_none() { if let Some(module_path) = find_in_dirs(&module_filename, working_set, &cwd, LIB_DIRS_ENV) { @@ -1816,8 +1817,8 @@ pub fn parse_source( // Command and one file name if spans.len() >= 2 { let name_expr = working_set.get_span_contents(spans[1]); - let name_expr = trim_quotes(name_expr); - if let Ok(filename) = String::from_utf8(name_expr.to_vec()) { + let (filename, err) = unescape_unquote_string(name_expr, spans[1]); + if err.is_none() { if let Some(path) = find_in_dirs(&filename, working_set, &cwd, LIB_DIRS_ENV) { if let Ok(contents) = std::fs::read(&path) { // This will load the defs from the file into the @@ -1967,26 +1968,27 @@ pub fn parse_register( .map(|expr| { let name_expr = working_set.get_span_contents(expr.span); - let name_expr = trim_quotes(name_expr); - String::from_utf8(name_expr.to_vec()) - .map_err(|_| ParseError::NonUtf8(expr.span)) - .and_then(|name| { - if let Some(p) = find_in_dirs(&name, working_set, &cwd, PLUGIN_DIRS_ENV) { - Ok(p) - } else { - Err(ParseError::RegisteredFileNotFound(name, expr.span)) - } - }) - .and_then(|path| { - if path.exists() & path.is_file() { - Ok(path) - } else { - Err(ParseError::RegisteredFileNotFound( - format!("{:?}", path), - expr.span, - )) - } - }) + let (name, err) = unescape_unquote_string(name_expr, expr.span); + + if let Some(err) = err { + Err(err) + } else { + let path = if let Some(p) = find_in_dirs(&name, working_set, &cwd, PLUGIN_DIRS_ENV) + { + p + } else { + return Err(ParseError::RegisteredFileNotFound(name, expr.span)); + }; + + if path.exists() & path.is_file() { + Ok(path) + } else { + Err(ParseError::RegisteredFileNotFound( + format!("{:?}", path), + expr.span, + )) + } + } }) .expect("required positional has being checked") .and_then(|path| { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3c322079df..d9bc86d1f9 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1928,10 +1928,10 @@ pub fn parse_directory( span: Span, ) -> (Expression, Option) { let bytes = working_set.get_span_contents(span); - let bytes = trim_quotes(bytes); + let (token, err) = unescape_unquote_string(bytes, span); trace!("parsing: directory"); - if let Ok(token) = String::from_utf8(bytes.into()) { + if err.is_none() { trace!("-- found {}", token); ( Expression { @@ -1955,10 +1955,10 @@ pub fn parse_filepath( span: Span, ) -> (Expression, Option) { let bytes = working_set.get_span_contents(span); - let bytes = trim_quotes(bytes); + let (token, err) = unescape_unquote_string(bytes, span); trace!("parsing: filepath"); - if let Ok(token) = String::from_utf8(bytes.into()) { + if err.is_none() { trace!("-- found {}", token); ( Expression { @@ -2267,12 +2267,11 @@ pub fn parse_glob_pattern( working_set: &mut StateWorkingSet, span: Span, ) -> (Expression, Option) { + let bytes = working_set.get_span_contents(span); + let (token, err) = unescape_unquote_string(bytes, span); trace!("parsing: glob pattern"); - let bytes = working_set.get_span_contents(span); - let bytes = trim_quotes(bytes); - - if let Ok(token) = String::from_utf8(bytes.into()) { + if err.is_none() { trace!("-- found {}", token); ( Expression {