From e97a4fab710ed591f7aad05a6e9073f4bebf3859 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Fri, 19 Apr 2024 14:49:35 +0200 Subject: [PATCH] Escape : and = in file completions This is similar to f7dac82ed (Escape separators (colon and equals) to improve completion, 2019-08-23) except we only escape : and = if they are the result of file completions. This way we avoid issues with custom completions like dd. This also means that it won't work for things like __fish_complete_suffix [*] but that can be fixed later, once we can serialize the DONT_ESCAPE flag. By moving the escaping step earlier, this causes some unit test changes which should not result in actual behavior change. See also #6099 [*]: The new \: and \= does not leak from "complete -C" because that command unescapes its output -- unless --escape is given. --- CHANGELOG.rst | 4 ++ src/common.rs | 10 +++++ src/complete.rs | 81 ++++++++++++++++++++++++++++------ src/tests/complete.rs | 100 +++++++++++++++++++++++++++++------------- 4 files changed, 151 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b3f10a51f..4e8e0ffdc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -99,6 +99,10 @@ Interactive improvements - Command-specific tab completions may now offer results whose first character is a period. For example, it is now possible to tab-complete ``git add`` for files with leading periods. The default file completions hide these files, unless the token itself has a leading period (:issue:`3707`). - Option completion now uses fuzzy subsequence filtering, as non-option completion does. This means that ``--fb`` may be completed to ``--foobar`` if there is no better match. - Completions that insert an entire token now use quotes instead of backslashes to escape special characters (:issue:`5433`). +- File name completion usually starts at the last ``:`` or ``=`` within a token. + If these characters are actually part of the filename, they will be escaped as ``\:`` and ``\=``, + and no longer get this special treatment. + This matches Bash's behavior. - Autosuggestions were sometimes not shown after recalling a line from history, which has been fixed (:issue:`10287`). - Nonprintable ASCII control characters are now rendered using symbols from Unicode's Control Pictures block (:issue:`5274`). - Up-arrow search matches are no longer highlighted with low contrast. diff --git a/src/common.rs b/src/common.rs index 0d46a862b..dd0fedeb3 100644 --- a/src/common.rs +++ b/src/common.rs @@ -120,6 +120,8 @@ bitflags! { const NO_TILDE = 1 << 2; /// Replace non-printable control characters with Unicode symbols. const SYMBOLIC = 1 << 3; + /// Escape : and = + const SEPARATORS = 1 << 4; } } @@ -180,6 +182,7 @@ pub fn escape_string(s: &wstr, style: EscapeStringStyle) -> WString { /// Escape a string in a fashion suitable for using in fish script. fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { let escape_printables = !flags.contains(EscapeFlags::NO_PRINTABLES); + let escape_separators = flags.contains(EscapeFlags::SEPARATORS); let no_quoted = flags.contains(EscapeFlags::NO_QUOTED); let no_tilde = flags.contains(EscapeFlags::NO_TILDE); let no_qmark = feature_test(FeatureFlag::qmark_noglob); @@ -290,6 +293,13 @@ fn escape_string_script(input: &wstr, flags: EscapeFlags) -> WString { ANY_STRING_RECURSIVE => { out += L!("**"); } + ':' | '=' => { + if escape_separators { + need_escape = true; + out.push('\\'); + } + out.push(c); + } '&' | '$' | ' ' | '#' | '<' | '>' | '(' | ')' | '[' | ']' | '{' | '}' | '?' | '*' | '|' | ';' | '"' | '%' | '~' => { diff --git a/src/complete.rs b/src/complete.rs index 63c9ac4cf..d8f604cff 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -10,7 +10,7 @@ use std::{ }; use crate::{ - common::charptr2wcstring, + common::{charptr2wcstring, escape_string, EscapeFlags, EscapeStringStyle}, reader::{get_quote, is_backslashed}, util::wcsfilecmp, }; @@ -727,11 +727,12 @@ impl<'ctx> Completer<'ctx> { if cmd_tok.location_in_or_at_end_of_source_range(cursor_pos) { let equals_sign_pos = variable_assignment_equals_pos(current_token); - if equals_sign_pos.is_some() { + if let Some(pos) = equals_sign_pos { self.complete_param_expand( - current_token, - true, /* do_file */ - false, /* handle_as_special_cd */ + ¤t_token[..pos + 1], + ¤t_token[pos + 1..], + /*do_file=*/ true, + /*handle_as_special_cd=*/ false, ); return; } @@ -831,10 +832,7 @@ impl<'ctx> Completer<'ctx> { } // This function wants the unescaped string. - self.complete_param_expand(current_argument, do_file, handle_as_special_cd); - - // Escape '[' in the argument before completing it. - self.escape_opening_brackets(current_argument); + self.complete_param_expand(L!(""), current_argument, do_file, handle_as_special_cd); // Lastly mark any completions that appear to already be present in arguments. self.mark_completions_duplicating_arguments(&cmdline, current_token, tokens); @@ -1501,7 +1499,13 @@ impl<'ctx> Completer<'ctx> { } /// Perform generic (not command-specific) expansions on the specified string. - fn complete_param_expand(&mut self, s: &wstr, do_file: bool, handle_as_special_cd: bool) { + fn complete_param_expand( + &mut self, + variable_override_prefix: &wstr, + s: &wstr, + do_file: bool, + handle_as_special_cd: bool, + ) { if self.ctx.check_cancel() { return; } @@ -1534,7 +1538,8 @@ impl<'ctx> Completer<'ctx> { // foo=bar => expand the whole thing, and also just bar // // We also support colon separator (#2178). If there's more than one, prefer the last one. - let sep_index = if get_quote(s, s.len()).is_some() { + let quoted = get_quote(s, s.len()).is_some(); + let sep_index = if quoted { None } else { let mut end = s.len(); @@ -1570,6 +1575,14 @@ impl<'ctx> Completer<'ctx> { // Any COMPLETE_REPLACES_TOKEN will also stomp the separator. We need to "repair" them by // inserting our separator and prefix. + Self::escape_opening_brackets(&mut local_completions, s); + Self::escape_separators( + &mut local_completions, + variable_override_prefix, + self.flags.autosuggestion, + true, + quoted, + ); let prefix_with_sep = s.as_char_slice()[..sep_index + 1].into(); for comp in &mut local_completions { comp.prepend_token_prefix(prefix_with_sep); @@ -1586,14 +1599,56 @@ impl<'ctx> Completer<'ctx> { flags -= ExpandFlags::FUZZY_MATCH; } + let first = self.completions.len(); if expand_to_receiver(s.to_owned(), &mut self.completions, flags, self.ctx, None).result == ExpandResultCode::error { FLOGF!(complete, "Error while expanding string '%ls'", s); } + Self::escape_opening_brackets(&mut self.completions[first..], s); + let have_token = !s.is_empty(); + Self::escape_separators( + &mut self.completions[first..], + variable_override_prefix, + self.flags.autosuggestion, + have_token, + quoted, + ); } } + fn escape_separators( + completions: &mut [Completion], + variable_override_prefix: &wstr, + append_only: bool, + have_token: bool, + is_quoted: bool, + ) { + for c in completions { + if is_quoted && !c.replaces_token() { + continue; + } + // clone of completion_apply_to_command_line + let add_space = !c.flags.contains(CompleteFlags::NO_SPACE); + let no_tilde = c.flags.contains(CompleteFlags::DONT_ESCAPE_TILDES); + let mut escape_flags = EscapeFlags::SEPARATORS; + if append_only || !add_space || (!c.replaces_token() && have_token) { + escape_flags.insert(EscapeFlags::NO_QUOTED); + } + if no_tilde { + escape_flags.insert(EscapeFlags::NO_TILDE); + } + if c.replaces_token() { + c.completion = variable_override_prefix.to_owned() + + &escape_string(&c.completion, EscapeStringStyle::Script(escape_flags))[..]; + } else { + c.completion = + escape_string(&c.completion, EscapeStringStyle::Script(escape_flags)); + } + assert!(!c.flags.contains(CompleteFlags::DONT_ESCAPE)); + c.flags |= CompleteFlags::DONT_ESCAPE; + } + } /// Complete the specified string as an environment variable. /// Returns `true` if this was a variable, so we should stop completion. fn complete_variable(&mut self, s: &wstr, start_offset: usize) -> bool { @@ -2008,7 +2063,7 @@ impl<'ctx> Completer<'ctx> { /// /// Check if there is any unescaped, unquoted '['; if yes, make the completions replace the entire /// argument instead of appending, so '[' will be escaped. - fn escape_opening_brackets(&mut self, argument: &wstr) { + fn escape_opening_brackets(completions: &mut [Completion], argument: &wstr) { let mut have_unquoted_unescaped_bracket = false; let mut quote = None; let mut escaped = false; @@ -2040,7 +2095,7 @@ impl<'ctx> Completer<'ctx> { ) else { return; }; - for comp in self.completions.get_list_mut() { + for comp in completions { if comp.flags.contains(CompleteFlags::REPLACES_TOKEN) { continue; } diff --git a/src/tests/complete.rs b/src/tests/complete.rs index 6780348d4..785a48d71 100644 --- a/src/tests/complete.rs +++ b/src/tests/complete.rs @@ -95,8 +95,13 @@ fn test_complete() { std::fs::create_dir_all("test/complete_test").unwrap(); std::fs::write("test/complete_test/has space", []).unwrap(); std::fs::write("test/complete_test/bracket[abc]", []).unwrap(); - #[cfg(not(windows))] // Square brackets are not legal path characters on WIN32/CYGWIN std::fs::write(r"test/complete_test/gnarlybracket\[abc]", []).unwrap(); + #[cfg(not(windows))] // Square brackets are not legal path characters on WIN32/CYGWIN + { + std::fs::write(r"test/complete_test/gnarlybracket\[abc]", []).unwrap(); + std::fs::write(r"test/complete_test/colon:abc", []).unwrap(); + } + std::fs::write(r"test/complete_test/equal=abc", []).unwrap(); std::fs::write("test/complete_test/testfile", []).unwrap(); let testfile = CString::new("test/complete_test/testfile").unwrap(); assert_eq!(unsafe { libc::chmod(testfile.as_ptr(), 0o700,) }, 0); @@ -158,21 +163,45 @@ fn test_complete() { // Brackets - see #5831 unique_completion_applies_as!( "touch test/complete_test/bracket[", - "test/complete_test/bracket[abc]", + r"'test/complete_test/bracket[abc]'", "touch 'test/complete_test/bracket[abc]' ", ); unique_completion_applies_as!( "echo (ls test/complete_test/bracket[", - "test/complete_test/bracket[abc]", + r"'test/complete_test/bracket[abc]'", "echo (ls 'test/complete_test/bracket[abc]' ", ); #[cfg(not(windows))] // Square brackets are not legal path characters on WIN32/CYGWIN { unique_completion_applies_as!( r"touch test/complete_test/gnarlybracket\\[", - r"test/complete_test/gnarlybracket\[abc]", + r"'test/complete_test/gnarlybracket\\[abc]'", r"touch 'test/complete_test/gnarlybracket\\[abc]' ", ); + unique_completion_applies_as!( + r"a=test/complete_test/bracket[", + r"a='test/complete_test/bracket[abc]'", + r"a='test/complete_test/bracket[abc]' ", + ); + } + + #[cfg(not(windows))] + { + unique_completion_applies_as!( + r"touch test/complete_test/colon", + r"\:abc", + r"touch test/complete_test/colon\:abc ", + ); + unique_completion_applies_as!( + r"touch test/complete_test/colon\:", + r"abc", + r"touch test/complete_test/colon\:abc ", + ); + unique_completion_applies_as!( + r#"touch "test/complete_test/colon:"#, + r"abc", + r#"touch "test/complete_test/colon:abc" "#, + ); } // #8820 @@ -427,8 +456,9 @@ fn test_autosuggest_suggest_special() { let _cleanup = test_init(); macro_rules! perform_one_autosuggestion_cd_test { ($command:literal, $expected:literal, $vars:expr) => { + let command = L!($command); let mut comps = complete( - L!($command), + command, CompletionRequestOptions::autosuggest(), &OperationContext::background($vars, EXPANSION_LIMIT_BACKGROUND), ) @@ -440,7 +470,15 @@ fn test_autosuggest_suggest_special() { if !expects_error { sort_and_prioritize(&mut comps, CompletionRequestOptions::default()); let suggestion = &comps[0]; - assert_eq!(suggestion.completion, L!($expected)); + let mut cursor = command.len(); + let newcmdline = completion_apply_to_command_line( + &suggestion.completion, + suggestion.flags, + command, + &mut cursor, + /*append_only=*/ true, + ); + assert_eq!(newcmdline.strip_prefix(command).unwrap(), L!($expected)); } }; } @@ -512,24 +550,24 @@ fn test_autosuggest_suggest_special() { perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/0", "foobar/", &vars); - perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/1", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/1", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/1", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/2", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/2", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/2", "foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/1", r"foo\ bar/", &vars); + perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/1", r"foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/1", r"foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/2", r"foo\ \ bar/", &vars); + perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/2", r"foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/2", r"foo bar/", &vars); #[cfg(not(windows))] { - perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/3", "foo\\bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/3", "foo\\bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/3", "foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/3", r"foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/3", r"foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/3", r"foo\\bar/", &vars); } - perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/4", "foo'bar/", &vars); + perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/4", r"foo\'bar/", &vars); perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/4", "foo'bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/4", "foo'bar/", &vars); - perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/5", "foo\"bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/5", "foo\"bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/5", "foo\"bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/4", r"foo\'bar/", &vars); + perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/5", r#"foo\"bar/"#, &vars); + perform_one_autosuggestion_cd_test!("cd \"test/autosuggest_test/5", r#"foo\"bar/"#, &vars); + perform_one_autosuggestion_cd_test!("cd 'test/autosuggest_test/5", r#"foo"bar/"#, &vars); vars.parent .vars @@ -549,24 +587,24 @@ fn test_autosuggest_suggest_special() { perform_one_autosuggestion_cd_test!("cd 0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd \"0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd '0", "foobar/", &vars); - perform_one_autosuggestion_cd_test!("cd 1", "foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 1", r"foo\ bar/", &vars); perform_one_autosuggestion_cd_test!("cd \"1", "foo bar/", &vars); perform_one_autosuggestion_cd_test!("cd '1", "foo bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 2", "foo bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 2", r"foo\ \ bar/", &vars); perform_one_autosuggestion_cd_test!("cd \"2", "foo bar/", &vars); perform_one_autosuggestion_cd_test!("cd '2", "foo bar/", &vars); #[cfg(not(windows))] { - perform_one_autosuggestion_cd_test!("cd 3", "foo\\bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"3", "foo\\bar/", &vars); - perform_one_autosuggestion_cd_test!("cd '3", "foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 3", r"foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd \"3", r"foo\\bar/", &vars); + perform_one_autosuggestion_cd_test!("cd '3", r"foo\\bar/", &vars); } - perform_one_autosuggestion_cd_test!("cd 4", "foo'bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"4", "foo'bar/", &vars); - perform_one_autosuggestion_cd_test!("cd '4", "foo'bar/", &vars); - perform_one_autosuggestion_cd_test!("cd 5", "foo\"bar/", &vars); - perform_one_autosuggestion_cd_test!("cd \"5", "foo\"bar/", &vars); - perform_one_autosuggestion_cd_test!("cd '5", "foo\"bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 4", r"foo\'bar/", &vars); + perform_one_autosuggestion_cd_test!("cd \"4", r"foo'bar/", &vars); + perform_one_autosuggestion_cd_test!("cd '4", r"foo\'bar/", &vars); + perform_one_autosuggestion_cd_test!("cd 5", r#"foo\"bar/"#, &vars); + perform_one_autosuggestion_cd_test!("cd \"5", r#"foo\"bar/"#, &vars); + perform_one_autosuggestion_cd_test!("cd '5", r#"foo"bar/"#, &vars); // A single quote should defeat tilde expansion. perform_one_autosuggestion_cd_test!("cd '~/test_autosuggest_suggest_specia'", "", &vars);