From 2b8d2deb0ccc5016a03c3023e0a2709e88c162d1 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 28 Nov 2020 16:15:01 -0800 Subject: [PATCH] Introduces "smartcase" completions "smartcase" performs case-insensitive matching if the input string is all lowercase, and case-sensitive matching otherwise. When completing e.g. files, we will now show both case sensitive and insensitive completions if the input string does not contain uppercase characters. This is a delicate fix in an interactive component with low test coverage. It's likely something will regress here. Fixes #3978 --- CHANGELOG.rst | 3 +-- src/complete.cpp | 5 ++++- src/fish_tests.cpp | 35 ++++++++++++++++++++++++++--------- src/wcstringutil.cpp | 19 +++++++++++++++---- src/wcstringutil.h | 5 +++-- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 18d6373cc..e0b9c9fbd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -140,8 +140,7 @@ Interactive improvements - ``help`` works properly on MSYS2 (:issue:`7113`). - Resuming a piped job by its number, like ``fg %1`` has been fixed (:issue:`7406`). - Commands run from key bindings now use the same tty modes as normal commands (:issue:`7483`). -- Autosuggestions from history are now case-sensitive (:issue:`3978`). - +- Autosuggestions from history are now case-sensitive, and tab completions are "smartcase": they offer case-insensitive matches if the input string is lowercase (:issue:`3978`). New or improved bindings ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/complete.cpp b/src/complete.cpp index e79f870fe..3250d4b64 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -290,9 +290,12 @@ void completions_sort_and_prioritize(completion_list_t *comps, completion_reques // Lastly, if this is for an autosuggestion, prefer to avoid completions that duplicate // arguments, and penalize files that end in tilde - they're frequently autosave files from e.g. - // emacs. + // emacs. Also prefer samecase to smartcase. if (flags & completion_request_t::autosuggestion) { stable_sort(comps->begin(), comps->end(), [](const completion_t &a, const completion_t &b) { + if (a.match.case_fold != b.match.case_fold) { + return a.match.case_fold < b.match.case_fold; + } return compare_completions_by_duplicate_arguments(a, b) || compare_completions_by_tilde(a, b); }); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 638f3e8f8..794c03771 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2132,9 +2132,13 @@ static void test_fuzzy_match() { do_test(test_fuzzy(L"", L"", type_t::exact, case_fold_t::samecase)); do_test(test_fuzzy(L"alpha", L"alpha", type_t::exact, case_fold_t::samecase)); do_test(test_fuzzy(L"alp", L"alpha", type_t::prefix, case_fold_t::samecase)); + do_test(test_fuzzy(L"alpha", L"AlPhA", type_t::exact, case_fold_t::smartcase)); + do_test(test_fuzzy(L"alpha", L"AlPhA!", type_t::prefix, case_fold_t::smartcase)); + do_test(test_fuzzy(L"ALPHA", L"alpha!", type_t::prefix, case_fold_t::icase)); do_test(test_fuzzy(L"ALPHA!", L"alPhA!", type_t::exact, case_fold_t::icase)); do_test(test_fuzzy(L"alPh", L"ALPHA!", type_t::prefix, case_fold_t::icase)); do_test(test_fuzzy(L"LPH", L"ALPHA!", type_t::substr, case_fold_t::samecase)); + do_test(test_fuzzy(L"lph", L"AlPhA!", type_t::substr, case_fold_t::smartcase)); do_test(test_fuzzy(L"lPh", L"ALPHA!", type_t::substr, case_fold_t::icase)); do_test(test_fuzzy(L"AA", L"ALPHA!", type_t::subseq, case_fold_t::samecase)); do_test(!string_fuzzy_match_string(L"lh", L"ALPHA!").has_value()); // no subseq icase @@ -2897,7 +2901,8 @@ static void test_complete() { struct test_complete_vars_t : environment_t { wcstring_list_t get_names(int flags) const override { UNUSED(flags); - return {L"Foo1", L"Foo2", L"Foo3", L"Bar1", L"Bar2", L"Bar3"}; + return {L"Foo1", L"Foo2", L"Foo3", L"Bar1", L"Bar2", + L"Bar3", L"alpha", L"ALPHA!", L"gamma1", L"GAMMA2"}; } maybe_t get(const wcstring &key, @@ -2921,13 +2926,24 @@ static void test_complete() { completions = do_complete(L"$", {}); completions_sort_and_prioritize(&completions); - do_test(completions.size() == 6); - do_test(completions.at(0).completion == L"Bar1"); - do_test(completions.at(1).completion == L"Bar2"); - do_test(completions.at(2).completion == L"Bar3"); - do_test(completions.at(3).completion == L"Foo1"); - do_test(completions.at(4).completion == L"Foo2"); - do_test(completions.at(5).completion == L"Foo3"); + do_test(completions.size() == 10); + do_test(completions.at(0).completion == L"alpha"); + do_test(completions.at(1).completion == L"ALPHA!"); + do_test(completions.at(2).completion == L"Bar1"); + do_test(completions.at(3).completion == L"Bar2"); + do_test(completions.at(4).completion == L"Bar3"); + do_test(completions.at(5).completion == L"Foo1"); + do_test(completions.at(6).completion == L"Foo2"); + do_test(completions.at(7).completion == L"Foo3"); + do_test(completions.at(8).completion == L"gamma1"); + do_test(completions.at(9).completion == L"GAMMA2"); + + // Smartcase test. Lowercase inputs match both lowercase and uppercase. + completions = do_complete(L"$a", {}); + completions_sort_and_prioritize(&completions); + do_test(completions.size() == 2); + do_test(completions.at(0).completion == L"$ALPHA!"); + do_test(completions.at(1).completion == L"lpha"); completions = do_complete(L"$F", {}); completions_sort_and_prioritize(&completions); @@ -2942,9 +2958,10 @@ static void test_complete() { completions = do_complete(L"$1", completion_request_t::fuzzy_match); completions_sort_and_prioritize(&completions); - do_test(completions.size() == 2); + do_test(completions.size() == 3); do_test(completions.at(0).completion == L"$Bar1"); do_test(completions.at(1).completion == L"$Foo1"); + do_test(completions.at(2).completion == L"$gamma1"); if (system("mkdir -p 'test/complete_test'")) err(L"mkdir failed"); if (system("touch 'test/complete_test/has space'")) err(L"touch failed"); diff --git a/src/wcstringutil.cpp b/src/wcstringutil.cpp index 8a344b09d..a492454f8 100644 --- a/src/wcstringutil.cpp +++ b/src/wcstringutil.cpp @@ -158,6 +158,15 @@ static bool subsequence_in_string(const wcstring &needle, const wcstring &haysta maybe_t string_fuzzy_match_t::try_create(const wcstring &string, const wcstring &match_against, bool anchor_start) { + // Helper to lazily compute if case insensitive matches should use icase or smartcase. + // Use icase if the input contains any uppercase characters, smartcase otherwise. + auto get_case_fold = [&] { + for (wchar_t c : string) { + if (towlower(c) != c) return case_fold_t::icase; + } + return case_fold_t::smartcase; + }; + // A string cannot fuzzy match against a shorter string. if (string.size() > match_against.size()) { return none(); @@ -175,12 +184,12 @@ maybe_t string_fuzzy_match_t::try_create(const wcstring &s // exact icase if (wcscasecmp(string.c_str(), match_against.c_str()) == 0) { - return string_fuzzy_match_t{contain_type_t::exact, case_fold_t::icase}; + return string_fuzzy_match_t{contain_type_t::exact, get_case_fold()}; } // prefix icase if (string_prefixes_string_case_insensitive(string, match_against)) { - return string_fuzzy_match_t{contain_type_t::prefix, case_fold_t::icase}; + return string_fuzzy_match_t{contain_type_t::prefix, get_case_fold()}; } // If anchor_start is set, this is as far as we go. @@ -196,7 +205,7 @@ maybe_t string_fuzzy_match_t::try_create(const wcstring &s // substr icase if ((location = ifind(match_against, string, true /* fuzzy */)) != wcstring::npos) { - return string_fuzzy_match_t{contain_type_t::substr, case_fold_t::icase}; + return string_fuzzy_match_t{contain_type_t::substr, get_case_fold()}; } // subseq samecase @@ -212,10 +221,12 @@ uint32_t string_fuzzy_match_t::rank() const { // Combine our type and our case fold into a single number, such that better matches are // smaller. Treat 'exact' types the same as 'prefix' types; this is because we do not // prefer exact matches to prefix matches when presenting completions to the user. + // Treat smartcase the same as samecase; see #3978. auto effective_type = (type == contain_type_t::exact ? contain_type_t::prefix : type); + auto effective_case = (case_fold == case_fold_t::smartcase ? case_fold_t::samecase : case_fold); // Type dominates fold. - return static_cast(effective_type) * 8 + static_cast(case_fold); + return static_cast(effective_type) * 8 + static_cast(effective_case); } template diff --git a/src/wcstringutil.h b/src/wcstringutil.h index 489ce804f..2ed24a2bb 100644 --- a/src/wcstringutil.h +++ b/src/wcstringutil.h @@ -48,8 +48,9 @@ struct string_fuzzy_match_t { // The case-folding required for the match. enum class case_fold_t : uint8_t { - samecase, // exact match: foobar matches foobar - icase, // case insensitive: FoBaR matches foobar + samecase, // exact match: foobar matches foobar + smartcase, // case insensitive match with lowercase input. foobar matches FoBar. + icase, // case insensitive: FoBaR matches foobAr }; case_fold_t case_fold;