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

@@ -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

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();

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 &current_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<history_t> &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;

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_data_t> {
     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<size_t>(-1)};
+    size_t history_pager_history_index_end{static_cast<size_t>(-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<reader_data_t> {
     /// 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_t> &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;
             }