Teach parse_util_escape_string_with_quote about tildes

Properly escape literal tildes in tab completion results. Currently we
always escape tildes in unquoted arguments; in the future we may escape
only leading tildes.

Fixes #2274

CHANGELOG.md

@@ -37,6 +37,7 @@ This section is for changes merged to the `major` branch that are not also merge
 - A new input binding `pager-toggle-search` toggles the search field in the completions pager on and off. By default this is bound to control-s.
 - Slicing $history (in particular, `$history[1]` for the last executed command) is much faster.
 - The pager will now show the full command instead of just its last line if the number of completions is large (#4702).
+- Tildes in file names are now properly escaped in completions (#2274)
 
 ## Other significant changes
 - Command substitution output is now limited to 10 MB by default (#3822).

src/fish_tests.cpp

@@ -358,6 +358,33 @@ static void test_escape_crazy() {
     }
 }
 
+static void test_escape_quotes() {
+    say(L"Testing escaping with quotes");
+    // These are "raw string literals"
+    do_test(parse_util_escape_string_with_quote(L"abc", L'\0') == L"abc");
+    do_test(parse_util_escape_string_with_quote(L"abc~def", L'\0') == L"abc\\~def");
+    do_test(parse_util_escape_string_with_quote(L"abc~def", L'\0', true) == L"abc~def");
+    do_test(parse_util_escape_string_with_quote(L"abc\\~def", L'\0') == L"abc\\\\\\~def");
+    do_test(parse_util_escape_string_with_quote(L"abc\\~def", L'\0', true) == L"abc\\\\~def");
+    do_test(parse_util_escape_string_with_quote(L"~abc", L'\0') == L"\\~abc");
+    do_test(parse_util_escape_string_with_quote(L"~abc", L'\0', true) == L"~abc");
+    do_test(parse_util_escape_string_with_quote(L"~abc|def", L'\0') == L"\\~abc\\|def");
+    do_test(parse_util_escape_string_with_quote(L"|abc~def", L'\0') == L"\\|abc\\~def");
+    do_test(parse_util_escape_string_with_quote(L"|abc~def", L'\0', true) == L"\\|abc~def");
+
+    // Note tildes are not expanded inside quotes, so no_tilde is ignored with a quote.
+    do_test(parse_util_escape_string_with_quote(L"abc", L'\'') == L"abc");
+    do_test(parse_util_escape_string_with_quote(L"abc\\def", L'\'') == L"abc\\def");
+    do_test(parse_util_escape_string_with_quote(L"abc'def", L'\'') == L"abc\\'def");
+    do_test(parse_util_escape_string_with_quote(L"~abc'def", L'\'') == L"~abc\\'def");
+    do_test(parse_util_escape_string_with_quote(L"~abc'def", L'\'', true) == L"~abc\\'def");
+
+    do_test(parse_util_escape_string_with_quote(L"abc", L'"') == L"abc");
+    do_test(parse_util_escape_string_with_quote(L"abc\\def", L'"') == L"abc\\def");
+    do_test(parse_util_escape_string_with_quote(L"~abc'def", L'"') == L"~abc'def");
+    do_test(parse_util_escape_string_with_quote(L"~abc'def", L'"', true) == L"~abc'def");
+}
+
 static void test_format(void) {
     say(L"Testing formatting functions");
     struct {
@@ -4435,6 +4462,7 @@ int main(int argc, char **argv) {
     if (should_test_function("error_messages")) test_error_messages();
     if (should_test_function("escape")) test_unescape_sane();
     if (should_test_function("escape")) test_escape_crazy();
+    if (should_test_function("escape")) test_escape_quotes();
     if (should_test_function("format")) test_format();
     if (should_test_function("convert")) test_convert();
     if (should_test_function("convert_nulls")) test_convert_nulls();

src/parse_util.cpp

@@ -511,10 +511,11 @@ void parse_util_get_parameter_info(const wcstring &cmd, const size_t pos, wchar_
     free(cmd_tmp);
 }
 
-wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote) {
+wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote, bool no_tilde) {
     wcstring result;
     if (quote == L'\0') {
-        result = escape_string(cmd, ESCAPE_ALL | ESCAPE_NO_QUOTED | ESCAPE_NO_TILDE);
+        escape_flags_t flags = ESCAPE_ALL | ESCAPE_NO_QUOTED | (no_tilde ? ESCAPE_NO_TILDE : 0);
+        result = escape_string(cmd, flags);
     } else {
         bool unescapable = false;
         for (size_t i = 0; i < cmd.size(); i++) {
@@ -660,9 +661,8 @@ std::vector<int> parse_util_compute_indents(const wcstring &src) {
     // foo ; cas', we get an invalid parse tree (since 'cas' is not valid) but we indent it as if it
     // were a case item list.
     parse_node_tree_t tree;
-    parse_tree_from_string(src,
-                           parse_flag_continue_after_error | parse_flag_include_comments |
-                               parse_flag_accept_incomplete_tokens,
+    parse_tree_from_string(src, parse_flag_continue_after_error | parse_flag_include_comments |
+                                    parse_flag_accept_incomplete_tokens,
                            &tree, NULL /* errors */);
 
     // Start indenting at the first node. If we have a parse error, we'll have to start indenting

src/parse_util.h

@@ -114,8 +114,9 @@ void parse_util_get_parameter_info(const wcstring &cmd, const size_t pos, wchar_
 
 /// 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).
-wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote);
+/// thus escaping should be with backslashes). Optionally do not escape tildes.
+wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote,
+                                             bool no_tilde = false);
 
 /// Given a string, parse it as fish code and then return the indents. The return value has the same
 /// size as the string.

src/reader.cpp

@@ -678,7 +678,8 @@ void reader_write_title(const wcstring &cmd, bool reset_cursor_position) {
         fish_title_command = L"fish_title";
         if (!cmd.empty()) {
             fish_title_command.append(L" ");
-            fish_title_command.append(parse_util_escape_string_with_quote(cmd, L'\0'));
+            fish_title_command.append(
+                escape_string(cmd, ESCAPE_ALL | ESCAPE_NO_QUOTED | ESCAPE_NO_TILDE));
         }
     }
 
@@ -1018,9 +1019,10 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
                                           const wcstring &command_line, size_t *inout_cursor_pos,
                                           bool append_only) {
     const wchar_t *val = val_str.c_str();
-    bool add_space = !static_cast<bool>(flags & COMPLETE_NO_SPACE);
-    bool do_replace = static_cast<bool>(flags & COMPLETE_REPLACES_TOKEN);
-    bool do_escape = !static_cast<bool>(flags & COMPLETE_DONT_ESCAPE);
+    bool add_space = !bool(flags & COMPLETE_NO_SPACE);
+    bool do_replace = bool(flags & COMPLETE_REPLACES_TOKEN);
+    bool do_escape = !bool(flags & COMPLETE_DONT_ESCAPE);
+    bool no_tilde = bool(flags & COMPLETE_DONT_ESCAPE_TILDES);
 
     const size_t cursor_pos = *inout_cursor_pos;
     bool back_into_trailing_quote = false;
@@ -1036,8 +1038,6 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
         wcstring sb(buff, begin - buff);
 
         if (do_escape) {
-            // Respect COMPLETE_DONT_ESCAPE_TILDES.
-            bool no_tilde = static_cast<bool>(flags & COMPLETE_DONT_ESCAPE_TILDES);
             wcstring escaped = escape_string(
                 val, ESCAPE_ALL | ESCAPE_NO_QUOTED | (no_tilde ? ESCAPE_NO_TILDE : 0));
             sb.append(escaped);
@@ -1061,9 +1061,6 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
     wchar_t quote = L'\0';
     wcstring replaced;
     if (do_escape) {
-        // Note that we ignore COMPLETE_DONT_ESCAPE_TILDES here. We get away with this because
-        // unexpand_tildes only operates on completions that have COMPLETE_REPLACES_TOKEN set,
-        // but we ought to respect them.
         parse_util_get_parameter_info(command_line, cursor_pos, &quote, NULL, NULL);
 
         // If the token is reported as unquoted, but ends with a (unescaped) quote, and we can
@@ -1079,7 +1076,7 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
             }
         }
 
-        replaced = parse_util_escape_string_with_quote(val_str, quote);
+        replaced = parse_util_escape_string_with_quote(val_str, quote, no_tilde);
     } else {
         replaced = val;
     }