Insert some completions with quotes instead of backslashes

File names that have lots of spaces look quite ugly when inserted as
completions because every space will have a backslash.

Add an initial heuristic to decide when to use quotes instead of
backslash escapes.

Quote when
1. it's not an autosuggestion
2. we replace the token or insert a fresh one
3. we will add a space at the end

In future we could relax some of these requirements.

Requirement 2 means we don't quote when appending to an existing token.
Need to find a natural behavior here.

Re 3, if the completion adds no space, users will probably want to add more
characters, which looks a bit weird if the token has a trailing quote.
We could relax this requirement for directory completions, so «ls so»
completes to «ls 'some dir with spaces'/».

Closes #5433

CHANGELOG.rst

@@ -95,6 +95,7 @@ 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`).
 - 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`).
 - When a command like ``fg %2`` fails to find the given job, it no longer behaves as if no job spec was given (:issue:`9835`).

src/parse_util.rs

@@ -674,48 +674,6 @@ fn error_for_character(c: char) -> WString {
     }
 }
 
-/// Calculates information on the parameter at the specified index.
-///
-/// \param cmd The command to be analyzed
-/// \param pos An index in the string which is inside the parameter
-/// \return the type of quote used by the parameter: either ' or " or \0.
-pub fn parse_util_get_quote_type(cmd: &wstr, pos: usize) -> Option<char> {
-    let mut tok = Tokenizer::new(cmd, TOK_ACCEPT_UNFINISHED);
-    while let Some(token) = tok.next() {
-        if token.type_ == TokenType::string && token.location_in_or_at_end_of_source_range(pos) {
-            return get_quote(tok.text_of(&token), pos - token.offset());
-        }
-    }
-    None
-}
-
-fn get_quote(cmd_str: &wstr, len: usize) -> Option<char> {
-    let cmd = cmd_str.as_char_slice();
-    let mut i = 0;
-    while i < cmd.len() {
-        if cmd[i] == '\\' {
-            i += 1;
-            if i == cmd_str.len() {
-                return None;
-            }
-            i += 1;
-        } else if cmd[i] == '\'' || cmd[i] == '"' {
-            match quote_end(cmd_str, i, cmd[i]) {
-                Some(end) => {
-                    if end > len {
-                        return Some(cmd[i]);
-                    }
-                    i = end + 1;
-                }
-                None => return Some(cmd[i]),
-            }
-        } else {
-            i += 1;
-        }
-    }
-    None
-}
-
 /// Attempts to escape the string 'cmd' using the given quote type, as determined by the quote
 /// character. The quote can be a single quote or double quote, or L'\0' to indicate no quoting (and
 /// thus escaping should be with backslashes). Optionally do not escape tildes.

src/reader.rs

@@ -5119,7 +5119,10 @@ 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;
+    let mut escape_flags = EscapeFlags::empty();
+    if append_only || !add_space {
+        escape_flags.insert(EscapeFlags::NO_QUOTED);
+    }
     if no_tilde {
         escape_flags.insert(EscapeFlags::NO_TILDE);
     }
@@ -5132,17 +5135,7 @@ pub fn completion_apply_to_command_line(
         let mut sb = command_line[..range.start].to_owned();
 
         if do_escape {
-            let escaped = escape_string(
-                val_str,
-                EscapeStringStyle::Script(
-                    EscapeFlags::NO_QUOTED
-                        | if no_tilde {
-                            EscapeFlags::NO_TILDE
-                        } else {
-                            EscapeFlags::empty()
-                        },
-                ),
-            );
+            let escaped = escape_string(val_str, EscapeStringStyle::Script(escape_flags));
             sb.push_utfstr(&escaped);
             move_cursor = escaped.len();
         } else {
@@ -5168,8 +5161,10 @@ pub fn completion_apply_to_command_line(
         let mut tok = 0..0;
         parse_util_token_extent(command_line, cursor_pos, &mut tok, None);
         // Find the last quote in the token to complete.
+        let mut have_token = false;
         if tok.contains(&cursor_pos) || cursor_pos == tok.end {
             quote = get_quote(&command_line[tok.clone()], cursor_pos - tok.start);
+            have_token = !tok.is_empty();
         }
 
         // If the token is reported as unquoted, but ends with a (unescaped) quote, and we can
@@ -5185,6 +5180,10 @@ pub fn completion_apply_to_command_line(
             }
         }
 
+        if have_token {
+            escape_flags.insert(EscapeFlags::NO_QUOTED);
+        }
+
         parse_util_escape_string_with_quote(val_str, quote, escape_flags)
     } else {
         val_str.to_owned()

src/tests/complete.rs

@@ -159,7 +159,7 @@ fn test_complete() {
         &mut cursor,
         false,
     );
-    assert_eq!(newcmdline, L!("touch test/complete_test/bracket\\[abc\\] "));
+    assert_eq!(newcmdline, L!("touch 'test/complete_test/bracket[abc]' "));
 
     // #8820
     let mut cursor_pos = 11;
@@ -191,7 +191,7 @@ fn test_complete() {
         );
         assert_eq!(
             newcmdline,
-            L!(r"touch test/complete_test/gnarlybracket\\\[abc\] ")
+            L!(r"touch 'test/complete_test/gnarlybracket\\[abc]' ")
         );
     }
 

src/tokenizer.rs

@@ -52,6 +52,7 @@ pub enum TokenizerError {
     expected_bclose_found_pclose,
 }
 
+#[derive(Debug)]
 pub struct Tok {
     // Offset of the token.
     pub offset: u32,

tests/checks/complete.fish

@@ -10,7 +10,7 @@ complete -c complete_test_alpha3 --no-files -w 'complete_test_alpha2 extra2'
 complete -C'complete_test_alpha1 arg1 '
 # CHECK: complete_test_alpha1 arg1
 complete --escape -C'complete_test_alpha1 arg1 '
-# CHECK: complete_test_alpha1\ arg1\
+# CHECK: 'complete_test_alpha1 arg1 '
 complete -C'complete_test_alpha2 arg2 '
 # CHECK: complete_test_alpha1 extra1 arg2
 complete -C'complete_test_alpha3 arg3 '

tests/pexpects/commandline.py

@@ -53,7 +53,7 @@ expect_prompt("foo")
 sendline("complete -c foo -xa '(commandline)'")
 expect_prompt()
 send("foo bar \t")
-expect_str("foo bar foo\ bar\ ")
+expect_str("foo bar 'foo bar '")
 send("\b" * 64)
 
 # Commandline works when run on its own (#8807).