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.
This commit is contained in:
Johannes Altmanninger 2024-04-19 14:49:35 +02:00
parent db365b5ef8
commit e97a4fab71
4 changed files with 151 additions and 44 deletions

View File

@ -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.

View File

@ -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);
}
'&' | '$' | ' ' | '#' | '<' | '>' | '(' | ')' | '[' | ']' | '{' | '}' | '?' | '*'
| '|' | ';' | '"' | '%' | '~' => {

View File

@ -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 */
&current_token[..pos + 1],
&current_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;
}

View File

@ -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'", "<error>", &vars);