From 639cd66ba15aa7ad4c52887b08aa760b22d51b68 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 27 Nov 2020 15:00:16 -0800 Subject: [PATCH] Conditionally make autosuggestions case sensitive When fish presents an autosuggestion, there is some logic around whether to retain it or discard it as the user types "into" it. Prior to this change, we would retain the autosuggestion if the user's input text is a case-insensitive prefix of the autosuggestion. This is reasonable for certain case-insensitive autosuggestions like files, but it is confusing especially for history items, e.g. `git branch -d ...` and `git branch -D ...` should not be considered a match. With this change, when we compute the autosuggestion we record whether it is "icase", and that controls whether the autosuggestion permits a case-insensitive extension. This addresses part of #3978. --- CHANGELOG.rst | 1 + src/reader.cpp | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d1c2d9f55..18d6373cc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -140,6 +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`). New or improved bindings diff --git a/src/reader.cpp b/src/reader.cpp index 05919e3f2..d86c55e3e 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -424,10 +424,14 @@ class reader_history_search_t { /// The result of an autosuggestion computation. struct autosuggestion_t { // The text to use, as an extension of the command line. - wcstring text; + wcstring text{}; // The string which was searched for. - wcstring search_string; + wcstring search_string{}; + + // Whether the autosuggestion should be case insensitive. + // This is true for file-generated autosuggestions, but not for history. + bool icase{false}; // Clear our contents. void clear() { @@ -437,6 +441,10 @@ struct autosuggestion_t { // \return whether we have empty text. bool empty() const { return text.empty(); } + + autosuggestion_t() = default; + autosuggestion_t(wcstring text, wcstring search_string, bool icase) + : text(std::move(text)), search_string(std::move(search_string)), icase(icase) {} }; struct highlight_result_t { @@ -1607,7 +1615,9 @@ static std::function get_autosuggestion_performer( if (autosuggest_validate_from_history(item, working_directory, ctx)) { // The command autosuggestion was handled specially, so we're done. - return {searcher.current_string(), search_string}; + // History items are case-sensitive, see #3978. + return autosuggestion_t{searcher.current_string(), search_string, + false /* icase */}; } } } @@ -1634,7 +1644,8 @@ static std::function get_autosuggestion_performer( size_t cursor = cursor_pos; wcstring suggestion = completion_apply_to_command_line( comp.completion, comp.flags, search_string, &cursor, true /* append only */); - return {std::move(suggestion), search_string}; + // Normal completions are case-insensitive. + return autosuggestion_t{std::move(suggestion), search_string, true /* icase */}; } return nothing; @@ -1680,7 +1691,9 @@ void reader_data_t::update_autosuggestion() { // text avoid recomputing the autosuggestion. const editable_line_t &el = command_line; if (!autosuggestion.empty() && - string_prefixes_string_case_insensitive(el.text(), autosuggestion.text)) { + (autosuggestion.icase + ? string_prefixes_string_case_insensitive(el.text(), autosuggestion.text) + : string_prefixes_string(el.text(), autosuggestion.text))) { return; }