From 8d88b4d358a7327eb14394178de81bed1390b9c8 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 13 Apr 2024 01:00:44 +0200 Subject: [PATCH] Support quoted escaping also when ' or \ is present Also, if there are more single quotes than double quotes and dollars, use double quotes for quoting. --- src/common.rs | 30 ++++++++++--- src/parse_util.rs | 86 +++++++++++++++++++++++++------------- src/reader.rs | 7 +++- tests/checks/alias.fish | 4 +- tests/checks/complete.fish | 2 +- tests/checks/nuls.fish | 2 +- tests/checks/string.fish | 3 ++ 7 files changed, 94 insertions(+), 40 deletions(-) diff --git a/src/common.rs b/src/common.rs index 6a44061a8..4aaab7976 100644 --- a/src/common.rs +++ b/src/common.rs @@ -9,6 +9,7 @@ use crate::future_feature_flags::{feature_test, FeatureFlag}; use crate::global_safety::AtomicRef; use crate::global_safety::RelaxedAtomicBool; use crate::libc::MB_CUR_MAX; +use crate::parse_util::parse_util_escape_string_with_quote; use crate::termsize::Termsize; use crate::wchar::{decode_byte_from_char, encode_byte_to_char, prelude::*}; use crate::wcstringutil::wcs2string_callback; @@ -190,6 +191,9 @@ fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { let mut need_escape = false; let mut need_complex_escape = false; + let mut double_quotes = 0; + let mut single_quotes = 0; + let mut dollars = 0; if !no_quoted && input.is_empty() { return L!("''").to_owned(); @@ -267,7 +271,9 @@ fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { } '\\' | '\'' => { need_escape = true; - need_complex_escape = true; + if c == '\'' { + single_quotes += 1; + } if escape_printables || (c == '\\' && !symbolic) { out.push('\\'); } @@ -286,6 +292,12 @@ fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { '&' | '$' | ' ' | '#' | '<' | '>' | '(' | ')' | '[' | ']' | '{' | '}' | '?' | '*' | '|' | ';' | '"' | '%' | '~' => { + if c == '"' { + double_quotes += 1; + } + if c == '$' { + dollars += 1; + } let char_is_normal = (c == '~' && no_tilde) || (c == '?' && no_qmark); if !char_is_normal { need_escape = true; @@ -327,12 +339,20 @@ fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { // Use quoted escaping if possible, since most people find it easier to read. if !no_quoted && need_escape && !need_complex_escape && escape_printables { - let single_quote = '\''; + let quote = if single_quotes > double_quotes + dollars { + '"' + } else { + '\'' + }; out.clear(); out.reserve(2 + input.len()); - out.push(single_quote); - out.push_utfstr(input); - out.push(single_quote); + out.push(quote); + out.push_utfstr(&parse_util_escape_string_with_quote( + input, + Some(quote), + EscapeFlags::empty(), + )); + out.push(quote); } out diff --git a/src/parse_util.rs b/src/parse_util.rs index c66345892..035ae4fca 100644 --- a/src/parse_util.rs +++ b/src/parse_util.rs @@ -722,14 +722,10 @@ fn get_quote(cmd_str: &wstr, len: usize) -> Option { pub fn parse_util_escape_string_with_quote( cmd: &wstr, quote: Option, - no_tilde: bool, + escape_flags: EscapeFlags, ) -> WString { let Some(quote) = quote else { - let mut flags = EscapeFlags::NO_QUOTED; - if no_tilde { - flags |= EscapeFlags::NO_TILDE; - } - return escape_string(cmd, EscapeStringStyle::Script(flags)); + return escape_string(cmd, EscapeStringStyle::Script(escape_flags)); }; // Here we are going to escape a string with quotes. // A few characters cannot be represented inside quotes, e.g. newlines. In that case, @@ -1863,40 +1859,70 @@ fn test_escape_quotes() { macro_rules! validate { ($cmd:expr, $quote:expr, $no_tilde:expr, $expected:expr) => { assert_eq!( - parse_util_escape_string_with_quote(L!($cmd), $quote, $no_tilde), + parse_util_escape_string_with_quote( + L!($cmd), + $quote, + if $no_tilde { + EscapeFlags::NO_TILDE + } else { + EscapeFlags::empty() + } + ), + L!($expected) + ); + }; + } + macro_rules! validate_no_quoted { + ($cmd:expr, $quote:expr, $no_tilde:expr, $expected:expr) => { + assert_eq!( + parse_util_escape_string_with_quote( + L!($cmd), + $quote, + EscapeFlags::NO_QUOTED + | if $no_tilde { + EscapeFlags::NO_TILDE + } else { + EscapeFlags::empty() + } + ), L!($expected) ); }; } - // These are "raw string literals" - validate!("abc", None, false, "abc"); - validate!("abc~def", None, false, "abc\\~def"); + validate!("abc~def", None, false, "'abc~def'"); validate!("abc~def", None, true, "abc~def"); - validate!("abc\\~def", None, false, "abc\\\\\\~def"); - validate!("abc\\~def", None, true, "abc\\\\~def"); - validate!("~abc", None, false, "\\~abc"); + validate!("~abc", None, false, "'~abc'"); validate!("~abc", None, true, "~abc"); - validate!("~abc|def", None, false, "\\~abc\\|def"); - validate!("|abc~def", None, false, "\\|abc\\~def"); - validate!("|abc~def", None, true, "\\|abc~def"); - validate!("foo\nbar", None, false, "foo\\nbar"); + + // These are "raw string literals" + validate_no_quoted!("abc", None, false, "abc"); + validate_no_quoted!("abc~def", None, false, "abc\\~def"); + validate_no_quoted!("abc~def", None, true, "abc~def"); + validate_no_quoted!("abc\\~def", None, false, "abc\\\\\\~def"); + validate_no_quoted!("abc\\~def", None, true, "abc\\\\~def"); + validate_no_quoted!("~abc", None, false, "\\~abc"); + validate_no_quoted!("~abc", None, true, "~abc"); + validate_no_quoted!("~abc|def", None, false, "\\~abc\\|def"); + validate_no_quoted!("|abc~def", None, false, "\\|abc\\~def"); + validate_no_quoted!("|abc~def", None, true, "\\|abc~def"); + validate_no_quoted!("foo\nbar", None, false, "foo\\nbar"); // Note tildes are not expanded inside quotes, so no_tilde is ignored with a quote. - validate!("abc", Some('\''), false, "abc"); - validate!("abc\\def", Some('\''), false, "abc\\\\def"); - validate!("abc'def", Some('\''), false, "abc\\'def"); - validate!("~abc'def", Some('\''), false, "~abc\\'def"); - validate!("~abc'def", Some('\''), true, "~abc\\'def"); - validate!("foo\nba'r", Some('\''), false, "foo'\\n'ba\\'r"); - validate!("foo\\\\bar", Some('\''), false, "foo\\\\\\\\bar"); + validate_no_quoted!("abc", Some('\''), false, "abc"); + validate_no_quoted!("abc\\def", Some('\''), false, "abc\\\\def"); + validate_no_quoted!("abc'def", Some('\''), false, "abc\\'def"); + validate_no_quoted!("~abc'def", Some('\''), false, "~abc\\'def"); + validate_no_quoted!("~abc'def", Some('\''), true, "~abc\\'def"); + validate_no_quoted!("foo\nba'r", Some('\''), false, "foo'\\n'ba\\'r"); + validate_no_quoted!("foo\\\\bar", Some('\''), false, "foo\\\\\\\\bar"); - validate!("abc", Some('"'), false, "abc"); - validate!("abc\\def", Some('"'), false, "abc\\\\def"); - validate!("~abc'def", Some('"'), false, "~abc'def"); - validate!("~abc'def", Some('"'), true, "~abc'def"); - validate!("foo\nba'r", Some('"'), false, "foo\"\\n\"ba'r"); - validate!("foo\\\\bar", Some('"'), false, "foo\\\\\\\\bar"); + validate_no_quoted!("abc", Some('"'), false, "abc"); + validate_no_quoted!("abc\\def", Some('"'), false, "abc\\\\def"); + validate_no_quoted!("~abc'def", Some('"'), false, "~abc'def"); + validate_no_quoted!("~abc'def", Some('"'), true, "~abc'def"); + validate_no_quoted!("foo\nba'r", Some('"'), false, "foo\"\\n\"ba'r"); + validate_no_quoted!("foo\\\\bar", Some('"'), false, "foo\\\\\\\\bar"); } #[test] diff --git a/src/reader.rs b/src/reader.rs index ca8d01a89..167d65c81 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -5091,6 +5091,11 @@ pub fn completion_apply_to_command_line( return replace_line_at_cursor(command_line, inout_cursor_pos, val_str); } + let mut escape_flags = EscapeFlags::NO_QUOTED; + if no_tilde { + escape_flags.insert(EscapeFlags::NO_TILDE); + } + if do_replace_token { let mut move_cursor; let mut range = 0..0; @@ -5157,7 +5162,7 @@ pub fn completion_apply_to_command_line( } } - parse_util_escape_string_with_quote(val_str, quote, no_tilde) + parse_util_escape_string_with_quote(val_str, quote, escape_flags) } else { val_str.to_owned() }; diff --git a/tests/checks/alias.fish b/tests/checks/alias.fish index de725979d..71b1ee2ee 100644 --- a/tests/checks/alias.fish +++ b/tests/checks/alias.fish @@ -17,7 +17,7 @@ alias foo '"a b" c d e' # framework and we can't predict the definition. alias | grep -Ev '^alias (fish_indent|fish_key_reader) ' # CHECK: alias a-2 'echo "hello there"' -# CHECK: alias a-3 echo\\\ hello\\\\\\\ there +# CHECK: alias a-3 'echo hello\\\\ there' # CHECK: alias foo '"a b" c d e' # CHECK: alias my_alias 'foo; and echo foo ran' @@ -28,7 +28,7 @@ alias l. "ls -d .*" alias d "'/mnt/c/Program Files (x86)/devenv.exe' /Edit" functions d # CHECK: # Defined via `source` -# CHECK: function d --wraps=\'/mnt/c/Program\ Files\ \(x86\)/devenv.exe\'\ /Edit --description alias\ d\ \'/mnt/c/Program\ Files\ \(x86\)/devenv.exe\'\ /Edit +# CHECK: function d --wraps="'/mnt/c/Program Files (x86)/devenv.exe' /Edit" --description "alias d '/mnt/c/Program Files (x86)/devenv.exe' /Edit" # CHECK: '/mnt/c/Program Files (x86)/devenv.exe' /Edit $argv # CHECK: end diff --git a/tests/checks/complete.fish b/tests/checks/complete.fish index 1270d1713..b7de71103 100644 --- a/tests/checks/complete.fish +++ b/tests/checks/complete.fish @@ -62,7 +62,7 @@ complete # CHECK: complete --force-files t -l fileoption # CHECK: complete --no-files t -a '(t)' # CHECK: complete -p '/complete test/beta1' -s Z -d 'desc, desc' -# CHECK: complete --require-parameter 'complete test beta2' -d desc\ \'\ desc2\ \[ -a 'foo bar' +# CHECK: complete --require-parameter 'complete test beta2' -d "desc ' desc2 [" -a 'foo bar' # CHECK: complete --exclusive complete_test_beta2 -o test -n false # CHECK: complete {{.*}} # CHECK: complete {{.*}} diff --git a/tests/checks/nuls.fish b/tests/checks/nuls.fish index fa2194787..75620d18b 100644 --- a/tests/checks/nuls.fish +++ b/tests/checks/nuls.fish @@ -9,4 +9,4 @@ echo foo\x00bar | string escape # CHECK: foo # This one is just escaped echo foo\\x00bar | string escape -# CHECK: foo\\x00bar +# CHECK: 'foo\\x00bar' diff --git a/tests/checks/string.fish b/tests/checks/string.fish index b24e5c706..2d5a08a6c 100644 --- a/tests/checks/string.fish +++ b/tests/checks/string.fish @@ -260,6 +260,9 @@ echo \x07 | string escape # CHECK: \cg string escape --style=script 'a b#c"\'d' +# CHECK: 'a b#c"\'d' + +string escape --no-quoted --style=script 'a b#c"\'d' # CHECK: a\ b\#c\"\'d string escape --style=url 'a b#c"\'d'