From 453aac14af4ea43e31d4730b6bb7d836ccb69d54 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 28 Jul 2022 09:44:48 +0200 Subject: [PATCH] Advance pager history search with Control-R/Control-S Note that every change to the search field still starts a new search, from the end of history. We could change this in future but it's unclear to me what the expected behavior is. I don't find the traditional readline behavior very intuitive. --- doc_src/cmds/bind.rst | 4 ++-- src/history.cpp | 2 ++ src/history.h | 16 +++++++++---- src/reader.cpp | 56 ++++++++++++++++++++++++++++++++++--------- 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/doc_src/cmds/bind.rst b/doc_src/cmds/bind.rst index a44c69203..eddb0346a 100644 --- a/doc_src/cmds/bind.rst +++ b/doc_src/cmds/bind.rst @@ -197,7 +197,7 @@ The following special input functions are available: from the current autosuggestion. ``history-pager`` - invoke the searchable pager on history (incremental search). + invoke the searchable pager on history (incremental search); or if the history pager is already active, search further backwards in time. ``history-search-backward`` search the history for the previous match @@ -252,7 +252,7 @@ The following special input functions are available: only execute the next function if the previous succeeded (note: only some functions report success) ``pager-toggle-search`` - toggles the search field if the completions pager is visible. + toggles the search field if the completions pager is visible; or if used after ``history-pager``, search forwards in time. ``prevd-or-backward-word`` if the commandline is empty, then move backward in the directory history, otherwise move one word to the left diff --git a/src/history.cpp b/src/history.cpp index 8ac5bb8b4..032736d5c 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -677,6 +677,8 @@ const history_item_t &history_search_t::current_item() const { const wcstring &history_search_t::current_string() const { return this->current_item().str(); } +size_t history_search_t::current_index() const { return this->current_index_; } + void history_impl_t::clear_file_state() { // Erase everything we know about our file. file_contents.reset(); diff --git a/src/history.h b/src/history.h index 71057e0a2..e4b66e9de 100644 --- a/src/history.h +++ b/src/history.h @@ -280,6 +280,9 @@ class history_search_t { /// Returns the current search result item contents. asserts if there is no current item. const wcstring ¤t_string() const; + /// Returns the index of the current history item. + size_t current_index() const; + /// return whether we are case insensitive. bool ignores_case() const { return flags_ & history_search_ignore_case; } @@ -287,8 +290,13 @@ class history_search_t { /// alive. history_search_t(history_t *hist, const wcstring &str, enum history_search_type_t type = history_search_type_t::contains, - history_search_flags_t flags = 0) - : history_(hist), orig_term_(str), canon_term_(str), search_type_(type), flags_(flags) { + history_search_flags_t flags = 0, size_t starting_index = 0) + : history_(hist), + orig_term_(str), + canon_term_(str), + search_type_(type), + flags_(flags), + current_index_(starting_index) { if (ignores_case()) { std::transform(canon_term_.begin(), canon_term_.end(), canon_term_.begin(), towlower); } @@ -297,8 +305,8 @@ class history_search_t { /// Construct from a shared_ptr. TODO: this should be the only constructor. history_search_t(const std::shared_ptr &hist, const wcstring &str, enum history_search_type_t type = history_search_type_t::contains, - history_search_flags_t flags = 0) - : history_search_t(hist.get(), str, type, flags) {} + history_search_flags_t flags = 0, size_t starting_index = 0) + : history_search_t(hist.get(), str, type, flags, starting_index) {} /** Default constructor. */ history_search_t() = default; diff --git a/src/reader.cpp b/src/reader.cpp index 5ec479a6c..91e8b916a 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -569,6 +569,7 @@ struct highlight_result_t { struct history_pager_result_t { completion_list_t matched_commands; + size_t final_index; }; /// readline_loop_state_t encapsulates the state used in a readline loop. @@ -692,6 +693,9 @@ class reader_data_t : public std::enable_shared_from_this { reader_history_search_t history_search{}; /// Whether the in-pager history search is active. bool history_pager_active{false}; + /// The range in history covered by the history pager's current page. + size_t history_pager_history_index_start{static_cast(-1)}; + size_t history_pager_history_index_end{static_cast(-1)}; /// The cursor selection mode. cursor_selection_mode_t cursor_selection_mode{cursor_selection_mode_t::exclusive}; @@ -752,7 +756,8 @@ class reader_data_t : public std::enable_shared_from_this { /// Do what we need to do whenever our command line changes. void command_line_changed(const editable_line_t *el); void maybe_refilter_pager(const editable_line_t *el); - void fill_history_pager(); + void fill_history_pager(bool new_search, history_search_direction_t direction = + history_search_direction_t::backward); /// Do what we need to do whenever our pager selection changes. void pager_selection_changed(); @@ -1216,7 +1221,7 @@ void reader_data_t::command_line_changed(const editable_line_t *el) { s_generation.store(1 + read_generation_count(), std::memory_order_relaxed); } else if (el == &this->pager.search_field_line) { if (history_pager_active) { - fill_history_pager(); + fill_history_pager(true, history_search_direction_t::backward); return; } this->pager.refilter_completions(); @@ -1233,31 +1238,56 @@ void reader_data_t::maybe_refilter_pager(const editable_line_t *el) { } static history_pager_result_t history_pager_search(const std::shared_ptr &history, + history_search_direction_t direction, + size_t history_index, const wcstring &search_string) { // Use a small page size because we don't always offer practical ways to select a item from // a large page (since we don't support subsequence filtering here). constexpr size_t page_size = 12; completion_list_t completions; history_search_t search{history, search_string, history_search_type_t::contains, - smartcase_flags(search_string)}; - while (completions.size() < page_size && - search.go_to_next_match(history_search_direction_t::backward)) { + smartcase_flags(search_string), history_index}; + while (completions.size() < page_size && search.go_to_next_match(direction)) { const history_item_t &item = search.current_item(); completions.push_back(completion_t{ item.str(), L"", string_fuzzy_match_t::exact_match(), COMPLETE_REPLACES_COMMANDLINE | COMPLETE_DONT_ESCAPE | COMPLETE_DONT_SORT}); } - return {completions}; + if (direction == history_search_direction_t::forward) + std::reverse(completions.begin(), completions.end()); + return {completions, search.current_index()}; } -void reader_data_t::fill_history_pager() { - auto shared_this = this->shared_from_this(); +void reader_data_t::fill_history_pager(bool new_search, history_search_direction_t direction) { + assert(!new_search || direction == history_search_direction_t::backward); + size_t index; + if (new_search) { + index = 0; + } else if (direction == history_search_direction_t::forward) { + index = history_pager_history_index_start; + } else { + assert(direction == history_search_direction_t::backward); + index = history_pager_history_index_end; + } const wcstring &search_term = pager.search_field_line.text(); + auto shared_this = this->shared_from_this(); debounce_history_pager().perform( - [=]() { return history_pager_search(shared_this->history, search_term); }, - [shared_this, search_term](const history_pager_result_t &result) { + [=]() { return history_pager_search(shared_this->history, direction, index, search_term); }, + [=](const history_pager_result_t &result) { if (search_term != shared_this->pager.search_field_line.text()) return; // Stale request. + if (result.matched_commands.empty() && !new_search) { + // No more matches, keep the existing ones and flash. + shared_this->flash(); + return; + } + if (direction == history_search_direction_t::forward) { + shared_this->history_pager_history_index_start = result.final_index; + shared_this->history_pager_history_index_end = index; + } else { + shared_this->history_pager_history_index_start = index; + shared_this->history_pager_history_index_end = result.final_index; + } shared_this->pager.set_completions(result.matched_commands); shared_this->select_completion_in_direction(selection_motion_t::next, true); shared_this->super_highlight_me_plenty(); @@ -3389,6 +3419,10 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat break; } case rl::pager_toggle_search: { + if (history_pager_active) { + fill_history_pager(false, history_search_direction_t::forward); + break; + } if (!pager.empty()) { // Toggle search, and begin navigating if we are now searching. bool sfs = pager.is_search_field_shown(); @@ -3707,7 +3741,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } case rl::history_pager: { if (history_pager_active) { - // TODO Deepen search. + fill_history_pager(false, history_search_direction_t::backward); break; }