Prefer special autosuggestions to match case

Fixes #2672

4 files changed, 73 insertions, 61 deletions

diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp
index 90d9dadc..f875438d 100644
--- a/src/fish_tests.cpp
+++ b/src/fish_tests.cpp
@@ -2302,7 +2302,7 @@ static void test_completion_insertions()
 static void perform_one_autosuggestion_special_test(const wcstring &command, const wcstring &wd, const wcstring &expected, long line)
 {
     wcstring suggestion;
-    bool success = autosuggest_suggest_special(command, wd, suggestion);
+    bool success = autosuggest_suggest_special(command, wd, &suggestion);
     if (! success)
     {
         printf("line %ld: autosuggest_suggest_special() failed for command %ls\n", line, command.c_str());
@@ -2330,8 +2330,11 @@ static void test_autosuggest_suggest_special()
     if (system("mkdir -p /tmp/autosuggest_test/start/unique2/unique3/multi4")) err(L"mkdir failed");
     if (system("mkdir -p /tmp/autosuggest_test/start/unique2/unique3/multi42")) err(L"mkdir failed");
     if (system("mkdir -p /tmp/autosuggest_test/start/unique2/.hiddenDir/moreStuff")) err(L"mkdir failed");
-    
+
     const wcstring wd = L"/tmp/autosuggest_test/";
+    
+    env_set(L"AUTOSUGGEST_TEST_LOC", wd.c_str(), ENV_LOCAL);
+
     perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/0", wd, L"cd /tmp/autosuggest_test/0foobar/", __LINE__);
     perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/0", wd, L"cd \"/tmp/autosuggest_test/0foobar/\"", __LINE__);
     perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/0", wd, L"cd '/tmp/autosuggest_test/0foobar/'", __LINE__);
@@ -2373,6 +2376,8 @@ static void test_autosuggest_suggest_special()
     perform_one_autosuggestion_special_test(L"cd 5", wd, L"cd 5foo\\\"bar/", __LINE__);
     perform_one_autosuggestion_special_test(L"cd \"5", wd, L"cd \"5foo\\\"bar/\"", __LINE__);
     perform_one_autosuggestion_special_test(L"cd '5", wd, L"cd '5foo\"bar/'", __LINE__);
+    
+    //perform_one_autosuggestion_special_test(L"cd $AUTOSUGGEST_TEST_LOC/0", wd, L"cd $AUTOSUGGEST_TEST_LOC/0foobar/", __LINE__);
 
     perform_one_autosuggestion_special_test(L"cd ~/test_autosuggest_suggest_specia", wd, L"cd ~/test_autosuggest_suggest_special/", __LINE__);
     
diff --git a/src/highlight.cpp b/src/highlight.cpp
index 9ed94e08..3c70f68c 100644
--- a/src/highlight.cpp
+++ b/src/highlight.cpp
@@ -286,9 +286,8 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
             }
             else
             {
-                DIR *dir = NULL;
-
                 /* We do not end with a slash; it does not have to be a directory */
+                DIR *dir = NULL;
                 const wcstring dir_name = wdirname(abs_path);
                 const wcstring filename_fragment = wbasename(abs_path);
                 if (dir_name == L"/" && filename_fragment == L"/")
@@ -298,63 +297,77 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
                 }
                 else if ((dir = wopendir(dir_name)))
                 {
+                    // Check if we're case insensitive
+                    const bool do_case_insensitive = fs_is_case_insensitive(dir_name, dirfd(dir), case_sensitivity_cache);
+                    
+                    wcstring matched_file;
+                    bool match_is_case_insensitive = false;
+                    
                     // We opened the dir_name; look for a string where the base name prefixes it
+                    // Don't ask for the is_dir value unless we care, because it can cause extra filesystem access
                     wcstring ent;
-
-                    // Check if we're case insensitive
-                    bool case_insensitive = fs_is_case_insensitive(dir_name, dirfd(dir), case_sensitivity_cache);
-
-                    // Don't ask for the is_dir value unless we care, because it can cause extra filesystem acces */
                     bool is_dir = false;
                     while (wreaddir_resolving(dir, dir_name, ent, require_dir ? &is_dir : NULL))
                     {
-
-                        /* Determine which function to call to check for prefixes */
-                        bool (*prefix_func)(const wcstring &, const wcstring &);
-                        if (case_insensitive)
+                        // Maybe skip directories
+                        if (require_dir && ! is_dir)
                         {
-                            prefix_func = string_prefixes_string_case_insensitive;
+                            continue;
                         }
-                        else
+                        
+                        if (string_prefixes_string(filename_fragment, ent))
                         {
-                            prefix_func = string_prefixes_string;
+                            // We matched, case-sensitive. This is as good as it gets.
+                            matched_file = ent;
+                            match_is_case_insensitive = false;
+                            break;
                         }
-
-                        if (prefix_func(filename_fragment, ent) && (! require_dir || is_dir))
+                        else if (do_case_insensitive && string_prefixes_string_case_insensitive(filename_fragment, ent))
                         {
-                            result = true;
-                            if (out_suggested_cdpath)
+                            // Case insensitive match.
+                            // If we want to return a suggestion, we keep going in hopes of getting a case-sensitive match, which is better (#2672)
+                            // If we don't care about the suggestion, we're done
+                            matched_file = ent;
+                            match_is_case_insensitive = true;
+                            if (out_suggested_cdpath == NULL)
                             {
-                                /* We want to return the path in the same "form" as it was given, preserving all magic, etc. Take the given path, get its basename. Append that to the output if the basename actually prefixes the path (which it won't if the given path contains no slashes), and isn't a slash (so we don't duplicate slashes). Then append the directory entry. */
-                                
-                                wcstring suggestion;
-                                const wcstring path_base = wdirname(potential_path_fragment);
-
-                                if (prefix_func(path_base, potential_path_fragment))
-                                {
-                                    suggestion.append(path_base);
-                                    if (! string_suffixes_string(L"/", *out_suggested_cdpath))
-                                    {
-                                        suggestion.push_back(L'/');
-                                    }
-                                }
-                                append_path_component(suggestion, ent);
-                                
-                                /* A trailing '/' makes autosuggestion a bit nicer and is needed for the singles traversal */
-                                suggestion.push_back(L'/');
-                                
-                                /* Now descend the deepest unique hierarchy we have. */
-                                wcstring start_point = dir_name;
-                                append_path_component(start_point, ent);
-                                append_path_component(suggestion, descend_unique_hierarchy(start_point));
-                                
-                                /* Return our computed suggestion */
-                                out_suggested_cdpath->swap(suggestion);
+                                // Early out
+                                break;
                             }
-                            break;
                         }
                     }
                     closedir(dir);
+                    
+                    /* Can't have a case insensitive match unless we're doing that */
+                    assert(do_case_insensitive || ! match_is_case_insensitive);
+                    
+                    /* We succeeded if we found a match */
+                    result = ! matched_file.empty();
+                    
+                    if (out_suggested_cdpath != NULL)
+                    {
+                        /* We want to return the path in the same "form" as it was given, preserving all magic, etc. Take the given path, get its basename. Append that to the output if the basename actually prefixes the path (which it won't if the given path contains no slashes), and isn't a slash (so we don't duplicate slashes). Then append the directory entry. */
+                        
+                        wcstring suggestion;
+                        const wcstring path_base = wdirname(potential_path_fragment);
+                        if (string_prefixes_string(path_base, potential_path_fragment) ||
+                            (do_case_insensitive && string_prefixes_string_case_insensitive(path_base, potential_path_fragment)))
+                        {
+                            suggestion.append(path_base);
+                        }
+                        append_path_component(suggestion, ent);
+                        
+                        /* A trailing '/' makes autosuggestion a bit nicer and is needed for the singles traversal */
+                        suggestion.push_back(L'/');
+                        
+                        /* Now descend the deepest unique hierarchy we have. */
+                        wcstring start_point = dir_name;
+                        append_path_component(start_point, ent);
+                        append_path_component(suggestion, descend_unique_hierarchy(start_point));
+                        
+                        /* Return our computed suggestion */
+                        out_suggested_cdpath->swap(suggestion);
+                    }
                 }
             }
         }
@@ -392,12 +405,6 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d
 
     /* Call is_potential_path with all of these directories */
     bool result = is_potential_path(path, directories, flags | PATH_REQUIRE_DIR, out_path);
-#if 0
-    if (out_path)
-    {
-        printf("%ls -> %ls\n", path.c_str(), out_path->c_str());
-    }
-#endif
     return result;
 }
 
@@ -519,7 +526,7 @@ static bool autosuggest_parse_command(const wcstring &buff, wcstring *out_expand
 }
 
 /* We have to return an escaped string here */
-bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_directory, wcstring &out_suggestion)
+bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_directory, wcstring *out_suggestion)
 {
     if (str.empty())
         return false;
@@ -541,7 +548,7 @@ bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_di
 
         /* We always return true because we recognized the command. This prevents us from falling back to dumber algorithms; for example we won't suggest a non-directory for the cd command. */
         result = true;
-        out_suggestion.clear();
+        out_suggestion->clear();
 
         /* Unescape the parameter */
         wcstring unescaped_dir;
@@ -559,11 +566,11 @@ bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_di
             wcstring escaped_suggested_path = parse_util_escape_string_with_quote(suggested_path, quote);
 
             /* Return it */
-            out_suggestion = str;
-            out_suggestion.erase(last_arg_node.source_start);
-            if (quote != L'\0') out_suggestion.push_back(quote);
-            out_suggestion.append(escaped_suggested_path);
-            if (quote != L'\0') out_suggestion.push_back(quote);
+            out_suggestion->assign(str);
+            out_suggestion->erase(last_arg_node.source_start);
+            if (quote != L'\0') out_suggestion->push_back(quote);
+            out_suggestion->append(escaped_suggested_path);
+            if (quote != L'\0') out_suggestion->push_back(quote);
         }
     }
     else
diff --git a/src/highlight.h b/src/highlight.h
index e0cd612e..03e192a2 100644
--- a/src/highlight.h
+++ b/src/highlight.h
@@ -115,7 +115,7 @@ bool autosuggest_validate_from_history(const history_item_t &item, file_detectio
 
 /** Given the command line contents 'str', return via reference a suggestion by specially recognizing the command. The suggestion is escaped. Returns true if we recognized the command (even if we couldn't think of a suggestion for it).
 */
-bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_directory, wcstring &outString);
+bool autosuggest_suggest_special(const wcstring &str, const wcstring &working_directory, wcstring *out_suggestion);
 
 /* Tests whether the specified string cpath is the prefix of anything we could cd to. directories is a list of possible parent directories (typically either the working directory, or the cdpath). This does I/O!
 
diff --git a/src/reader.cpp b/src/reader.cpp
index 5e1d335e..a1027c39 100644
--- a/src/reader.cpp
+++ b/src/reader.cpp
@@ -1483,7 +1483,7 @@ struct autosuggestion_context_t
 
         /* Try handling a special command like cd */
         wcstring special_suggestion;
-        if (autosuggest_suggest_special(search_string, working_directory, special_suggestion))
+        if (autosuggest_suggest_special(search_string, working_directory, &special_suggestion))
         {
             this->autosuggestion = special_suggestion;
             return 1;