diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 473824675..8af5c8c23 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -235,11 +235,12 @@ Completions - Option completion now uses fuzzy subsequence filtering, just like non-option completion (:issue:`830`). 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`). -- Historically, file name completions are provided after the last ``:`` or ``=`` within a token. +- Normally, file name completions start after the last ``:`` or ``=`` in a token. This helps commands like ``rsync --files-from=``. - If the ``=`` or ``:`` is actually part of the filename, it will be escaped as ``\:`` and ``\=``, - and no longer get this special treatment. + This special meaning can now disabled by escaping these separators as ``\:`` and ``\=``. This matches Bash's behavior. + Note that this escaping is usually not necessary since the completion engine already tries + to guess whether the separator is actually part of a file name. - Various new completion scripts and numerous updates to existing ones. - Generated completions are now stored in ``$XDG_CACHE_HOME/fish`` or ``~/.cache/fish`` by default (:issue:`10369`) diff --git a/src/common.rs b/src/common.rs index c7769b458..7e81c505b 100644 --- a/src/common.rs +++ b/src/common.rs @@ -119,10 +119,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; /// Escape , - const COMMA = 1 << 5; + const COMMA = 1 << 4; } } @@ -183,7 +181,6 @@ 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 escape_comma = flags.contains(EscapeFlags::COMMA); let no_quoted = flags.contains(EscapeFlags::NO_QUOTED); let no_tilde = flags.contains(EscapeFlags::NO_TILDE); @@ -295,13 +292,6 @@ 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); - } ',' => { if escape_comma { need_escape = true; diff --git a/src/complete.rs b/src/complete.rs index 5639c42bc..1a23c1116 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -10,7 +10,7 @@ use std::{ }; use crate::{ - common::{charptr2wcstring, escape_string, EscapeFlags, EscapeStringStyle}, + common::charptr2wcstring, reader::{get_quote, is_backslashed}, util::wcsfilecmp, wutil::sprintf, @@ -96,7 +96,7 @@ pub const PROG_COMPLETE_SEP: char = '\t'; bitflags! { #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] - pub struct CompleteFlags: u8 { + pub struct CompleteFlags: u16 { /// Do not insert space afterwards if this is the only completion. (The default is to try insert /// a space). const NO_SPACE = 1 << 0; @@ -115,6 +115,8 @@ bitflags! { const DUPLICATES_ARGUMENT = 1 << 6; /// This completes not just a token but replaces an entire line. const REPLACES_LINE = 1 << 7; + /// If replacing the entire token, keep the "foo=" prefix. + const KEEP_VARIABLE_OVERRIDE_PREFIX = 1 << 8; } } @@ -737,13 +739,16 @@ 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 let Some(pos) = equals_sign_pos { + let first = self.completions.len(); self.complete_param_expand( - ¤t_token[..pos + 1], ¤t_token[pos + 1..], /*do_file=*/ true, /*handle_as_special_cd=*/ false, cur_tok.is_unterminated_brace, ); + for c in &mut self.completions[first..] { + c.flags |= CompleteFlags::KEEP_VARIABLE_OVERRIDE_PREFIX; + } return; } // Complete command filename. @@ -842,7 +847,6 @@ impl<'ctx> Completer<'ctx> { // This function wants the unescaped string. self.complete_param_expand( - L!(""), current_argument, do_file, handle_as_special_cd, @@ -1524,7 +1528,6 @@ impl<'ctx> Completer<'ctx> { /// Perform generic (not command-specific) expansions on the specified string. fn complete_param_expand( &mut self, - variable_override_prefix: &wstr, s: &wstr, do_file: bool, handle_as_special_cd: bool, @@ -1565,8 +1568,7 @@ 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 quoted = get_quote(s, s.len()).is_some(); - let sep_index = if quoted { + let sep_index = if get_quote(s, s.len()).is_some() { None } else { let mut end = s.len(); @@ -1584,6 +1586,7 @@ impl<'ctx> Completer<'ctx> { }; let complete_from_start = sep_index.is_none() || !string_prefixes_string(L!("-"), s); + let first_from_start = self.completions.len(); if complete_from_start { let mut flags = flags; // Don't do fuzzy matching for files if the string begins with a dash (issue #568). We could @@ -1592,7 +1595,6 @@ impl<'ctx> Completer<'ctx> { flags -= ExpandFlags::FUZZY_MATCH; } - let first = self.completions.len(); if matches!( expand_to_receiver(s.to_owned(), &mut self.completions, flags, self.ctx, None) .result, @@ -1600,20 +1602,25 @@ impl<'ctx> Completer<'ctx> { ) { 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, - ); + Self::escape_opening_brackets(&mut self.completions[first_from_start..], s); } let Some(sep_index) = sep_index else { return; }; + + // We generally expand both, the whole token ("foo=bar") and also just the "bar" + // suffix. If the whole token is a valid path prefix, completions of just the suffix + // are probably false positives, and are confusing when I'm using completions to list + // directory contents. Apply a wonky heuristic to work around the most visible case -- + // the empty suffix -- where all files in $PWD are completed/autosuggested. + if self.completions[first_from_start..] + .iter() + .any(|c| !c.replaces_token()) + && sep_index + 1 == s.len() + { + return; + } let sep_string = s.slice_from(sep_index + 1); let mut local_completions = Vec::new(); if matches!( @@ -1630,16 +1637,9 @@ impl<'ctx> Completer<'ctx> { FLOGF!(complete, "Error while expanding string '%ls'", sep_string); } + Self::escape_opening_brackets(&mut local_completions, s); // 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); @@ -1649,38 +1649,6 @@ impl<'ctx> Completer<'ctx> { } } - 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 { diff --git a/src/reader.rs b/src/reader.rs index 00dbf1abb..259fdfdc3 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -131,6 +131,7 @@ use crate::threads::{ Debounce, }; use crate::tokenizer::quote_end; +use crate::tokenizer::variable_assignment_equals_pos; use crate::tokenizer::{ tok_command, MoveWordStateMachine, MoveWordStyle, TokenType, Tokenizer, TOK_ACCEPT_UNFINISHED, TOK_SHOW_COMMENTS, @@ -5953,6 +5954,7 @@ pub fn completion_apply_to_command_line( let do_replace_line = flags.contains(CompleteFlags::REPLACES_LINE); let do_escape = !flags.contains(CompleteFlags::DONT_ESCAPE); let no_tilde = flags.contains(CompleteFlags::DONT_ESCAPE_TILDES); + let keep_variable_override = flags.contains(CompleteFlags::KEEP_VARIABLE_OVERRIDE_PREFIX); let cursor_pos = *inout_cursor_pos; let mut back_into_trailing_quote = false; @@ -5978,19 +5980,27 @@ pub fn completion_apply_to_command_line( } if do_replace_token { - let mut move_cursor; + let mut move_cursor = 0; let mut range = 0..0; parse_util_token_extent(command_line, cursor_pos, &mut range, None); let mut sb = command_line[..range.start].to_owned(); + if keep_variable_override { + let tok = &command_line[range.clone()]; + let separator_pos = variable_assignment_equals_pos(tok).unwrap(); + let key = &tok[..=separator_pos]; + sb.push_utfstr(&key); + move_cursor += key.len(); + } + if do_escape { let escaped = escape_string(val_str, EscapeStringStyle::Script(escape_flags)); sb.push_utfstr(&escaped); - move_cursor = escaped.len(); + move_cursor += escaped.len(); } else { sb.push_utfstr(val_str); - move_cursor = val_str.len(); + move_cursor += val_str.len(); } if add_space { diff --git a/src/tests/complete.rs b/src/tests/complete.rs index 891e41c08..21f267e30 100644 --- a/src/tests/complete.rs +++ b/src/tests/complete.rs @@ -137,6 +137,13 @@ fn test_complete() { assert_eq!(completions.len(), 1); assert_eq!(completions[0].completion, L!("space")); + completions = do_complete( + L!(": test/complete_test/colon:"), + CompletionRequestOptions::default(), + ); + assert_eq!(completions.len(), 1); + assert_eq!(completions[0].completion, L!("abc")); + macro_rules! unique_completion_applies_as { ( $cmdline:expr, $completion_result:expr, $applied:expr $(,)? ) => { let cmdline = L!($cmdline); @@ -168,24 +175,24 @@ fn test_complete() { // Brackets - see #5831 unique_completion_applies_as!( "touch test/complete_test/bracket[", - r"'test/complete_test/bracket[abc]'", + "test/complete_test/bracket[abc]", "touch 'test/complete_test/bracket[abc]' ", ); unique_completion_applies_as!( "echo (ls test/complete_test/bracket[", - r"'test/complete_test/bracket[abc]'", + "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"test/complete_test/bracket[abc]", r"a='test/complete_test/bracket[abc]' ", ); } @@ -194,13 +201,13 @@ fn test_complete() { { unique_completion_applies_as!( r"touch test/complete_test/colon", - r"\:abc", - r"touch test/complete_test/colon\:abc ", + r":abc", + r"touch test/complete_test/colon:abc ", ); unique_completion_applies_as!( - r"touch test/complete_test/colon\:", + r"touch test/complete_test/colon:", r"abc", - r"touch test/complete_test/colon\:abc ", + r"touch test/complete_test/colon:abc ", ); unique_completion_applies_as!( r#"touch "test/complete_test/colon:"#, @@ -462,9 +469,8 @@ fn test_autosuggest_suggest_special() { let parser = TestParser::new(); macro_rules! perform_one_autosuggestion_cd_test { ($command:literal, $expected:literal, $vars:expr) => { - let command = L!($command); let mut comps = complete( - command, + L!($command), CompletionRequestOptions::autosuggest(), &OperationContext::background($vars, EXPANSION_LIMIT_BACKGROUND), ) @@ -476,15 +482,7 @@ fn test_autosuggest_suggest_special() { if !expects_error { sort_and_prioritize(&mut comps, CompletionRequestOptions::default()); let suggestion = &comps[0]; - 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)); + assert_eq!(suggestion.completion, L!($expected)); } }; } @@ -552,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", 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); + 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); #[cfg(not(windows))] { - 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/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/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/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); + 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); vars.parent .vars @@ -589,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", 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 '1", "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); perform_one_autosuggestion_cd_test!("cd '2", "foo bar/", &vars); #[cfg(not(windows))] { - 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 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 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); + 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); // A single quote should defeat tilde expansion. perform_one_autosuggestion_cd_test!("cd '~/test_autosuggest_suggest_specia'", "", &vars);