Remove the reader_selected_completion_changed callback. Fix a hang when

the pager gets empty, as reported in 291

pager.cpp

@@ -388,7 +388,6 @@ void pager_t::refilter_completions()
             this->completion_infos.push_back(info);
         }
     }
-    note_selection_changed();
 }
 
 void pager_t::set_completions(const completion_list_t &raw_completions)
@@ -732,7 +731,6 @@ bool pager_t::select_next_completion_in_direction(selection_direction_t directio
                 {
                     selected_completion_idx = 0;
                 }
-                note_selection_changed();
                 return true;
 
             /* These do nothing */
@@ -897,7 +895,6 @@ bool pager_t::select_next_completion_in_direction(selection_direction_t directio
             }
         }
 
-        this->note_selection_changed();
         return true;
     }
     else
@@ -909,7 +906,7 @@ bool pager_t::select_next_completion_in_direction(selection_direction_t directio
 size_t pager_t::visual_selected_completion_index(size_t rows, size_t cols) const
 {
     /* No completions -> no selection */
-    if (completion_infos.empty())
+    if (completion_infos.empty() || rows == 0 || cols == 0)
     {
         return PAGER_SELECTION_NONE;
     }
@@ -997,10 +994,6 @@ size_t pager_t::cursor_position() const
     return result;
 }
 
-void pager_t::note_selection_changed()
-{
-    reader_selected_completion_changed(this);
-}
 
 /* Constructor */
 page_rendering_t::page_rendering_t() : term_width(-1), term_height(-1), rows(0), cols(0), row_start(0), row_end(0), selected_completion_idx(-1), remaining_to_disclose(0), search_field_shown(false)

reader.cpp

@@ -272,6 +272,9 @@ class reader_data_t
     /** Do what we need to do whenever our command line changes */
     void command_line_changed(const editable_line_t *el);
 
+    /** Do what we need to do whenever our pager selection */
+    void pager_selection_changed();
+
     /** Expand abbreviations at the current cursor position, minus backtrack_amt. */
     bool expand_abbreviation_as_necessary(size_t cursor_backtrack);
 
@@ -686,7 +689,35 @@ void reader_data_t::command_line_changed(const editable_line_t *el)
     else if (el == &this->pager.search_field_line)
     {
         this->pager.refilter_completions();
+        this->pager_selection_changed();
+    }
+}
+
+void reader_data_t::pager_selection_changed()
+{
+    ASSERT_IS_MAIN_THREAD();
+
+    const completion_t *completion = this->pager.selected_completion(this->current_page_rendering);
+
+    /* Update the cursor and command line */
+    size_t cursor_pos = this->cycle_cursor_pos;
+    wcstring new_cmd_line;
+
+    if (completion == NULL)
+    {
+        new_cmd_line = this->cycle_command_line;
+    }
+    else
+    {
+        new_cmd_line = completion_apply_to_command_line(completion->completion, completion->flags, this->cycle_command_line, &cursor_pos, false);
     }
+    reader_set_buffer_maintaining_pager(new_cmd_line, cursor_pos);
+
+    /* Since we just inserted a completion, don't immediately do a new autosuggestion */
+    this->suppress_autosuggestion = true;
+
+    /* Trigger repaint (see #765) */
+    reader_repaint_needed();
 }
 
 /* Expand abbreviations at the given cursor position. Does NOT inspect 'data'. */
@@ -1644,7 +1675,11 @@ static void select_completion_in_direction(enum selection_direction_t dir)
 {
     assert(data != NULL);
     /* Note: this will probably trigger reader_selected_completion_changed, which will cause us to update stuff */
-    data->pager.select_next_completion_in_direction(dir, data->current_page_rendering);
+    bool selection_changed = data->pager.select_next_completion_in_direction(dir, data->current_page_rendering);
+    if (selection_changed)
+    {
+        data->pager_selection_changed();
+    }
 }
 
 /**
@@ -1964,6 +1999,8 @@ static bool handle_completions(const std::vector<completion_t> &comp, bool conti
                 /* Invalidate our rendering */
                 data->current_page_rendering = page_rendering_t();
 
+                /* Modify the command line to reflect the new pager */
+                data->pager_selection_changed();
             }
             else
             {

reader.h

@@ -301,8 +301,4 @@ bool reader_expand_abbreviation_in_command(const wcstring &cmdline, size_t curso
 /* Apply a completion string. Exposed for testing only. */
 wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flags_t flags, const wcstring &command_line, size_t *inout_cursor_pos, bool append_only);
 
-/* Called by pager */
-class pager_t;
-void reader_selected_completion_changed(pager_t *pager);
-
 #endif