history pager: delete selected history entry with Shift-Delete

After accidentally running a command that includes a pasted password, I want
to delete command from history. Today we need to recall or type (part of)
that command and type "history delete".  Let's maybe add a shortcut to do
this from the history pager.

The current shortcut is Shift+Delete. I don't think that's very discoverable,
maybe we should use Delete instead (but only if the cursor is at the end of
the commandline, otherwise delete a char).

Closes #9454

doc_src/cmds/bind.rst

@@ -198,6 +198,9 @@ The following special input functions are available:
 ``history-pager``
     invoke the searchable pager on history (incremental search); or if the history pager is already active, search further backwards in time.
 
+``history-pager-delete``
+    permanently delete the history item selected in the history pager
+
 ``history-search-backward``
     search the history for the previous match
 

share/functions/__fish_shared_key_bindings.fish

@@ -38,6 +38,7 @@ function __fish_shared_key_bindings -d "Bindings shared between emacs and vi mod
     bind --preset $argv \cs pager-toggle-search
     # shift-tab does a tab complete followed by a search.
     bind --preset $argv --key btab complete-and-search
+    bind --preset $argv -k sdc history-pager-delete or backward-delete-char # shifted delete
 
     bind --preset $argv \e\n "commandline -f expand-abbr; commandline -i \n"
     bind --preset $argv \e\r "commandline -f expand-abbr; commandline -i \n"

share/functions/fish_default_key_bindings.fish

@@ -51,7 +51,6 @@ function fish_default_key_bindings -d "emacs-like key binds"
 
     bind --preset $argv -k home beginning-of-line
     bind --preset $argv -k end end-of-line
-    bind --preset $argv -k sdc backward-delete-char # shifted delete
 
     bind --preset $argv \ca beginning-of-line
     bind --preset $argv \ce end-of-line

share/functions/fish_vi_key_bindings.fish

@@ -125,8 +125,6 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish'
     bind -s --preset -M default \ch backward-char
     bind -s --preset -M insert \x7f backward-delete-char
     bind -s --preset -M default \x7f backward-char
-    bind -s --preset -M insert -k sdc backward-delete-char # shifted delete
-    bind -s --preset -M default -k sdc backward-delete-char # shifted delete
 
     bind -s --preset dd kill-whole-line
     bind -s --preset D kill-line

src/input.cpp

@@ -129,6 +129,7 @@ static constexpr const input_function_metadata_t input_function_metadata[] = {
     {L"forward-single-char", readline_cmd_t::forward_single_char},
     {L"forward-word", readline_cmd_t::forward_word},
     {L"history-pager", readline_cmd_t::history_pager},
+    {L"history-pager-delete", readline_cmd_t::history_pager_delete},
     {L"history-prefix-search-backward", readline_cmd_t::history_prefix_search_backward},
     {L"history-prefix-search-forward", readline_cmd_t::history_prefix_search_forward},
     {L"history-search-backward", readline_cmd_t::history_search_backward},

src/input_common.h

@@ -29,6 +29,7 @@ enum class readline_cmd_t {
     history_prefix_search_backward,
     history_prefix_search_forward,
     history_pager,
+    history_pager_delete,
     delete_char,
     backward_delete_char,
     kill_line,

src/pager.cpp

@@ -883,6 +883,15 @@ const completion_t *pager_t::selected_completion(const page_rendering_t &renderi
     return result;
 }
 
+size_t pager_t::selected_completion_index() const { return selected_completion_idx; }
+
+void pager_t::set_selected_completion_index(size_t new_index) {
+    // Current users are off by one at most.
+    assert(new_index == PAGER_SELECTION_NONE || new_index <= completion_infos.size());
+    if (new_index == completion_infos.size()) --new_index;
+    selected_completion_idx = new_index;
+}
+
 /// Get the selected row and column. Completions are rendered column first, i.e. we go south before
 /// we go west. So if we have N rows, and our selected index is N + 2, then our row is 2 (mod by N)
 /// and our column is 1 (divide by N).

src/pager.h

@@ -165,6 +165,9 @@ class pager_t {
     // Returns the currently selected completion for the given rendering.
     const completion_t *selected_completion(const page_rendering_t &rendering) const;
 
+    size_t selected_completion_index() const;
+    void set_selected_completion_index(size_t new_index);
+
     // Indicates the row and column for the given rendering. Returns -1 if no selection.
     size_t get_selected_row(const page_rendering_t &rendering) const;
     size_t get_selected_column(const page_rendering_t &rendering) const;

src/reader.cpp

@@ -728,6 +728,8 @@ 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 direction of the last successful history pager search.
+    history_search_direction_t history_pager_direction{};
     /// 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)};
@@ -791,8 +793,14 @@ 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(bool new_search, history_search_direction_t direction =
+    enum class history_pager_invocation_t {
-                                                 history_search_direction_t::backward);
+        anew,
+        advance,
+        refresh,
+    };
+    void fill_history_pager(
+        history_pager_invocation_t why,
+        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();
@@ -1272,7 +1280,8 @@ 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(true, history_search_direction_t::backward);
+            fill_history_pager(history_pager_invocation_t::anew,
+                               history_search_direction_t::backward);
             return;
         }
         this->pager.refilter_completions();
@@ -1325,16 +1334,29 @@ static history_pager_result_t history_pager_search(const std::shared_ptr<history
     return {completions, last_index, search.go_to_next_match(direction)};
 }
 
-void reader_data_t::fill_history_pager(bool new_search, history_search_direction_t direction) {
+void reader_data_t::fill_history_pager(history_pager_invocation_t why,
-    assert(!new_search || direction == history_search_direction_t::backward);
+                                       history_search_direction_t direction) {
-    size_t index;
+    size_t index = -1;
-    if (new_search) {
+    maybe_t<size_t> old_pager_index;
-        index = 0;
+    switch (why) {
-    } else if (direction == history_search_direction_t::forward) {
+        case history_pager_invocation_t::anew:
-        index = history_pager_history_index_start;
+            assert(direction == history_search_direction_t::backward);
-    } else {
+            index = 0;
-        assert(direction == history_search_direction_t::backward);
+            break;
-        index = history_pager_history_index_end;
+        case history_pager_invocation_t::advance:
+            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;
+            }
+            break;
+        case history_pager_invocation_t::refresh:
+            // Redo the previous search previous direction.
+            direction = history_pager_direction;
+            index = history_pager_history_index_start;
+            old_pager_index = pager.selected_completion_index();
+            break;
     }
     const wcstring &search_term = pager.search_field_line.text();
     auto shared_this = this->shared_from_this();
@@ -1345,11 +1367,12 @@ void reader_data_t::fill_history_pager(bool new_search, history_search_direction
         [=](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) {
+            if (result.matched_commands.empty() && why == history_pager_invocation_t::advance) {
                 // No more matches, keep the existing ones and flash.
                 shared_this->flash();
                 return;
             }
+            history_pager_direction = direction;
             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;
@@ -1360,7 +1383,12 @@ void reader_data_t::fill_history_pager(bool new_search, history_search_direction
             shared_this->pager.extra_progress_text =
                 result.have_more_results ? _(L"Search again for more results") : L"";
             shared_this->pager.set_completions(result.matched_commands);
-            shared_this->select_completion_in_direction(selection_motion_t::next, true);
+            if (why == history_pager_invocation_t::refresh) {
+                pager.set_selected_completion_index(*old_pager_index);
+                pager_selection_changed();
+            } else {
+                shared_this->select_completion_in_direction(selection_motion_t::next, true);
+            }
             shared_this->super_highlight_me_plenty();
             shared_this->layout_and_repaint(L"history-pager");
         };
@@ -3578,7 +3606,8 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
         }
         case rl::pager_toggle_search: {
             if (history_pager_active) {
-                fill_history_pager(false, history_search_direction_t::forward);
+                fill_history_pager(history_pager_invocation_t::advance,
+                                   history_search_direction_t::forward);
                 break;
             }
             if (!pager.empty()) {
@@ -3792,7 +3821,8 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
         }
         case rl::history_pager: {
             if (history_pager_active) {
-                fill_history_pager(false, history_search_direction_t::backward);
+                fill_history_pager(history_pager_invocation_t::advance,
+                                   history_search_direction_t::backward);
                 break;
             }
 
@@ -3810,6 +3840,20 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
             insert_string(&pager.search_field_line, command_line.text());
             break;
         }
+        case rl::history_pager_delete: {
+            if (!history_pager_active) {
+                inputter.function_set_status(false);
+                break;
+            }
+            inputter.function_set_status(true);
+            if (auto completion = pager.selected_completion(current_page_rendering)) {
+                history->remove(completion->completion);
+                history->save();
+                fill_history_pager(history_pager_invocation_t::refresh,
+                                   history_search_direction_t::backward);
+            }
+            break;
+        }
         case rl::backward_char: {
             editable_line_t *el = active_edit_line();
             if (is_navigating_pager_contents()) {