Teach case-insensitive completions about tildes. Fixes https://github.com/fish-shell/fish-shell/issues/647

7 files changed, 132 insertions, 82 deletions

diff --git a/common.cpp b/common.cpp
index 3fb64a24..75a44ceb 100644
--- a/common.cpp
+++ b/common.cpp
@@ -554,12 +554,8 @@ wchar_t *quote_end(const wchar_t *pos)
 wcstring wsetlocale(int category, const wchar_t *locale)
 {
 
-    char *lang = NULL;
-    if (locale)
-    {
-        lang = wcs2str(locale);
-    }
-    char * res = setlocale(category,lang);
+    char *lang = locale ? wcs2str(locale) : NULL;
+    char *res = setlocale(category, lang);
     free(lang);
 
     /*
@@ -572,7 +568,7 @@ wcstring wsetlocale(int category, const wchar_t *locale)
 
     // U+23CE is the "return" character
     omitted_newline_char = unicode ? L'\x23CE' : L'~';
-
+    
     if (!res)
         return wcstring();
     else
diff --git a/complete.h b/complete.h
index 478facc1..72bcd8c1 100644
--- a/complete.h
+++ b/complete.h
@@ -88,7 +88,10 @@ enum
     COMPLETE_AUTO_SPACE = 1 << 3,
 
     /** This completion should be inserted as-is, without escaping. */
-    COMPLETE_DONT_ESCAPE = 1 << 4
+    COMPLETE_DONT_ESCAPE = 1 << 4,
+    
+    /** If you do escape, don't escape tildes */
+    COMPLETE_DONT_ESCAPE_TILDES = 1 << 5
 };
 typedef int complete_flags_t;
 
diff --git a/expand.cpp b/expand.cpp
index 886087f6..69be2b03 100644
--- a/expand.cpp
+++ b/expand.cpp
@@ -1463,44 +1463,50 @@ static wcstring expand_unescape_string(const wcstring &in, int escape_special)
     return tmp;
 }
 
-/**
-   Attempts tilde expansion of the string specified, modifying it in place.
-*/
-static void expand_home_directory(wcstring &input)
+/* Given that input[0] is HOME_DIRECTORY or tilde (ugh), return the user's name. Return the empty string if it is just a tilde. Also return by reference the index of the first character of the remaining part of the string (e.g. the subsequent slash) */
+static wcstring get_home_directory_name(const wcstring &input, size_t *out_tail_idx)
 {
     const wchar_t * const in = input.c_str();
-    if (in[0] == HOME_DIRECTORY)
+    assert(in[0] == HOME_DIRECTORY || in[0] == L'~');
+    size_t tail_idx;
+
+    const wchar_t *name_end = wcschr(in, L'/');
+    if (name_end)
+    {
+        tail_idx = name_end - in;
+    }
+    else
+    {
+        tail_idx = wcslen(in);
+    }
+    *out_tail_idx = tail_idx;
+    return input.substr(1, tail_idx - 1);
+}
+
+/** Attempts tilde expansion of the string specified, modifying it in place. */
+static void expand_home_directory(wcstring &input)
+{
+    if (! input.empty() && input.at(0) == HOME_DIRECTORY)
     {
-        int tilde_error = 0;
         size_t tail_idx;
+        wcstring username = get_home_directory_name(input, &tail_idx);
+        
+        bool tilde_error = false;
         wcstring home;
-
-        if (in[1] == '/' || in[1] == '\0')
+        if (username.empty())
         {
             /* Current users home directory */
-
             home = env_get_string(L"HOME");
             tail_idx = 1;
         }
         else
         {
             /* Some other users home directory */
-            const wchar_t *name_end = wcschr(in, L'/');
-            if (name_end)
-            {
-                tail_idx = name_end - in;
-            }
-            else
-            {
-                tail_idx = wcslen(in);
-            }
-            wcstring name_str = input.substr(1, tail_idx - 1);
-            std::string name_cstr = wcs2string(name_str);
+            std::string name_cstr = wcs2string(username);
             struct passwd *userinfo = getpwnam(name_cstr.c_str());
-
             if (userinfo == NULL)
             {
-                tilde_error = 1;
+                tilde_error = true;
                 input[0] = L'~';
             }
             else
@@ -1518,13 +1524,59 @@ static void expand_home_directory(wcstring &input)
 
 void expand_tilde(wcstring &input)
 {
-    if (! input.empty() && input.at(0) == L'~')
+    // Avoid needless COW behavior by ensuring we use const at
+    const wcstring &tmp = input;
+    if (! tmp.empty() && tmp.at(0) == L'~')
     {
         input.at(0) = HOME_DIRECTORY;
         expand_home_directory(input);
     }
 }
 
+static void unexpand_tildes(const wcstring &input, std::vector<completion_t> *completions)
+{
+    // If input begins with tilde, then try to replace the corresponding string in each completion with the tilde
+    // If it does not, there's nothing to do
+    if (input.empty() || input.at(0) != L'~')
+        return;
+    
+    // We only operate on completions that replace their contents
+    // If we don't have any, we're done.
+    // In particular, empty vectors are common.
+    bool has_candidate_completion = false;
+    for (size_t i=0; i < completions->size(); i++)
+    {
+        if (completions->at(i).flags & COMPLETE_REPLACES_TOKEN)
+        {
+            has_candidate_completion = true;
+            break;
+        }
+    }
+    if (! has_candidate_completion)
+        return;
+    
+    size_t tail_idx;
+    wcstring username_with_tilde = L"~";
+    username_with_tilde.append(get_home_directory_name(input, &tail_idx));
+    
+    // Expand username_with_tilde
+    wcstring home = username_with_tilde;
+    expand_tilde(home);
+    
+    // Now for each completion that starts with home, replace it with the username_with_tilde
+    for (size_t i=0; i < completions->size(); i++)
+    {
+        completion_t &comp = completions->at(i);
+        if ((comp.flags & COMPLETE_REPLACES_TOKEN) && string_prefixes_string(home, comp.completion))
+        {
+            comp.completion.replace(0, home.size(), username_with_tilde);
+            
+            // And mark that our tilde is literal, so it doesn't try to escape it
+            comp.flags |= COMPLETE_DONT_ESCAPE_TILDES;
+        }
+    }
+}
+
 /**
    Remove any internal separators. Also optionally convert wildcard characters to
    regular equivalents. This is done to support EXPAND_SKIP_WILDCARDS.
@@ -1555,19 +1607,31 @@ static void remove_internal_separator(wcstring &str, bool conv)
 
 
 int expand_string(const wcstring &input, std::vector<completion_t> &output, expand_flags_t flags)
-{
+{    
     parser_t parser(PARSER_TYPE_ERRORS_ONLY, true /* show errors */);
-    std::vector<completion_t> list1, list2;
-    std::vector<completion_t> *in, *out;
-
+    
     size_t i;
     int res = EXPAND_OK;
 
+    /* Make the empty string handling behavior explicit. It's a little weird, but expand_one depends on this. */
+    if (input.empty())
+    {
+        /* Return OK. But only append a completion if ACCEPT_INCOMPLETE is not set. */
+        if (! (flags & ACCEPT_INCOMPLETE))
+        {
+            output.push_back(completion_t(input));
+        }
+        return EXPAND_OK;
+    }
+
     if ((!(flags & ACCEPT_INCOMPLETE)) && expand_is_clean(input.c_str()))
     {
         output.push_back(completion_t(input));
         return EXPAND_OK;
     }
+    
+    std::vector<completion_t> clist1, clist2;
+    std::vector<completion_t> *in = &clist1, *out = &clist2;
 
     if (EXPAND_SKIP_CMDSUBST & flags)
     {
@@ -1581,18 +1645,15 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
             parser.error(CMDSUBST_ERROR, -1, L"Command substitutions not allowed");
             return EXPAND_ERROR;
         }
-        list1.push_back(completion_t(input));
+        in->push_back(completion_t(input));
     }
     else
     {
-        int cmdsubst_ok = expand_cmdsubst(parser, input, list1);
+        int cmdsubst_ok = expand_cmdsubst(parser, input, *in);
         if (! cmdsubst_ok)
             return EXPAND_ERROR;
     }
-
-    in = &list1;
-    out = &list2;
-
+    
     for (i=0; i < in->size(); i++)
     {
         /*
@@ -1624,9 +1685,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
     }
 
     in->clear();
-
-    in = &list2;
-    out = &list1;
+    std::swap(in, out); // note: this swaps the pointers only (last output is next input)
 
     for (i=0; i < in->size(); i++)
     {
@@ -1638,9 +1697,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
         }
     }
     in->clear();
-
-    in = &list1;
-    out = &list2;
+    std::swap(in, out); // note: this swaps the pointers only (last output is next input)
 
     for (i=0; i < in->size(); i++)
     {
@@ -1678,9 +1735,7 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
     }
 
     in->clear();
-
-    in = &list2;
-    out = &list1;
+    std::swap(in, out); // note: this swaps the pointers only (last output is next input)
 
     for (i=0; i < in->size(); i++)
     {
@@ -1694,7 +1749,6 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
                 wildcard_has(next, 1))
         {
             const wchar_t *start, *rest;
-            std::vector<completion_t> *list = out;
 
             if (next[0] == '/')
             {
@@ -1707,39 +1761,28 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
                 rest = next;
             }
 
+            std::vector<completion_t> expanded;
+            wc_res = wildcard_expand_string(rest, start, flags, expanded);
             if (flags & ACCEPT_INCOMPLETE)
             {
-                list = &output;
+                out->insert(out->end(), expanded.begin(), expanded.end());
             }
-
-            wc_res = wildcard_expand_string(rest, start, flags, *list);
-
-            if (!(flags & ACCEPT_INCOMPLETE))
+            else
             {
-
                 switch (wc_res)
                 {
                     case 0:
                     {
-                        if (!(flags & ACCEPT_INCOMPLETE))
-                        {
-                            if (res == EXPAND_OK)
-                                res = EXPAND_WILDCARD_NO_MATCH;
-                            break;
-                        }
+                        if (res == EXPAND_OK)
+                            res = EXPAND_WILDCARD_NO_MATCH;
+                        break;
                     }
 
                     case 1:
                     {
-                        size_t j;
                         res = EXPAND_WILDCARD_MATCH;
-                        sort_completions(*out);
-
-                        for (j=0; j< out->size(); j++)
-                        {
-                            output.push_back(out->at(j));
-                        }
-                        out->clear();
+                        sort_completions(expanded);
+                        out->insert(out->end(), expanded.begin(), expanded.end());
                         break;
                     }
 
@@ -1750,20 +1793,24 @@ int expand_string(const wcstring &input, std::vector<completion_t> &output, expa
 
                 }
             }
-
         }
         else
         {
-            if (flags & ACCEPT_INCOMPLETE)
-            {
-            }
-            else
+            if (! (flags & ACCEPT_INCOMPLETE))
             {
-                output.push_back(completion_t(next));
+                out->push_back(completion_t(next_str));
             }
         }
-
     }
+    
+    // Hack to un-expand tildes (see #647)
+    if (! (flags & EXPAND_SKIP_HOME_DIRECTORIES))
+    {
+        unexpand_tildes(input, out);
+    }
+    
+    // Return our output
+    output.insert(output.end(), out->begin(), out->end());
 
     return res;
 }
diff --git a/fish_tests.cpp b/fish_tests.cpp
index 41ab82b3..94679fac 100644
--- a/fish_tests.cpp
+++ b/fish_tests.cpp
@@ -1701,7 +1701,7 @@ int main(int argc, char **argv)
     setlocale(LC_ALL, "");
     srand(time(0));
     configure_thread_assertions_for_testing();
-
+    
     program_name=L"(ignore)";
 
     say(L"Testing low-level functionality");
diff --git a/highlight.cpp b/highlight.cpp
index 452aa2de..488d7517 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -810,7 +810,9 @@ bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_di
     wcstring_list_t parsed_arguments;
     int parsed_last_arg_pos = -1;
     if (! autosuggest_parse_command(str, &parsed_command, &parsed_arguments, &parsed_last_arg_pos))
+    {
         return false;
+    }
 
     bool result = false;
     if (parsed_command == L"cd" && ! parsed_arguments.empty())
diff --git a/reader.cpp b/reader.cpp
index 85b82137..7978fb59 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -1005,7 +1005,9 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
 
         if (do_escape)
         {
-            escaped = escape(val, ESCAPE_ALL | ESCAPE_NO_QUOTED);
+            /* Respect COMPLETE_DONT_ESCAPE_TILDES */
+            bool no_tilde = !! (flags & COMPLETE_DONT_ESCAPE_TILDES);
+            escaped = escape(val, ESCAPE_ALL | ESCAPE_NO_QUOTED | (no_tilde ? ESCAPE_NO_TILDE : 0));
             sb.append(escaped);
             move_cursor = wcslen(escaped);
             free(escaped);
@@ -1034,6 +1036,7 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
         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 modify the command line, then delete the trailing quote so that we can insert within the quotes instead of after them. See https://github.com/fish-shell/fish-shell/issues/552 */
diff --git a/wildcard.cpp b/wildcard.cpp
index 3177cf35..b95425d0 100644
--- a/wildcard.cpp
+++ b/wildcard.cpp
@@ -703,8 +703,7 @@ static int wildcard_expand_internal(const wchar_t *wc,
                                     expand_flags_t flags,
                                     std::vector<completion_t> &out,
                                     std::set<wcstring> &completion_set,
-                                    std::set<file_id_t> &visited_files
-                                   )
+                                    std::set<file_id_t> &visited_files)
 {
 
     /* Points to the end of the current wildcard segment */