Initial support for navigating completions that appear under the

commandline using arrow keys

builtin_commandline.cpp

@@ -149,7 +149,7 @@ static void write_part(const wchar_t *begin,
     {
         wchar_t *buff = wcsndup(begin, end-begin);
 //    fwprintf( stderr, L"Subshell: %ls, end char %lc\n", buff, *end );
-        wcstring out;
+            wcstring out;
         tokenizer_t tok(buff, TOK_ACCEPT_UNFINISHED);
         for (; tok_has_next(&tok); tok_next(&tok))
         {

common.h

@@ -87,6 +87,15 @@ enum
 };
 typedef unsigned int escape_flags_t;
 
+/* Directions */
+enum cardinal_direction_t
+{
+    direction_north,
+    direction_east,
+    direction_south,
+    direction_west
+};
+
 /**
  Helper macro for errors
  */

pager.cpp

@@ -122,6 +122,12 @@ static std::vector<char> pager_buffer;
 */
 static wcstring out_buff;
 
+/* Returns numer / denom, rounding up */
+static size_t divide_round_up(size_t numer, size_t denom)
+{
+    return numer / denom + (numer % denom ? 1 : 0);
+}
+
 /**
    This function calculates the minimum width for each completion
    entry in the specified array_list. This width depends on the
@@ -639,7 +645,7 @@ int pager_t::completion_try_print(int cols, const wcstring &prefix, const comp_i
     */
     int print=0;
 
-    int rows = (int)((lst.size()-1)/cols+1);
+    int rows = (int)divide_round_up(lst.size(), cols);
 
     int pref_tot_width=0;
     int min_tot_width = 0;
@@ -857,6 +863,7 @@ int pager_t::completion_try_print(int cols, const wcstring &prefix, const comp_i
 
 page_rendering_t pager_t::render() const
 {
+    
     /**
        Try to print the completions. Start by trying to print the
        list in PAGER_MAX_COLS columns, if the completions won't
@@ -864,34 +871,54 @@ page_rendering_t pager_t::render() const
        column never fails.
     */
     page_rendering_t rendering;
-    for (int i = PAGER_MAX_COLS; i>0; i--)
+    rendering.term_width = this->term_width;
+    rendering.term_height = this->term_height;
+    rendering.selected_completion_idx = this->selected_completion_idx;
+    
+    if (! this->empty())
     {
-        /* Initially empty rendering */
+        int cols;
-        rendering.screen_data.resize(0);
+        bool done = false;
-        
+        for (cols = PAGER_MAX_COLS; cols > 0 && ! done; cols--)
-        switch (completion_try_print(i, prefix, completion_infos, &rendering))
         {
-            case PAGER_RETRY:
+            /* Initially empty rendering */
-                break;
+            rendering.screen_data.resize(0);
+            
+            switch (completion_try_print(cols, prefix, completion_infos, &rendering))
+            {
+                case PAGER_RETRY:
+                    break;
 
-            case PAGER_DONE:
+                case PAGER_DONE:
-                i=0;
+                    done = true;
-                break;
+                    break;
 
-            case PAGER_RESIZE:
+                case PAGER_RESIZE:
-                /*
+                    /*
-                  This means we got a resize event, so we start
+                      This means we got a resize event, so we start
-                  over from the beginning. Since it the screen got
+                      over from the beginning. Since it the screen got
-                  bigger, we might be able to fit all completions
+                      bigger, we might be able to fit all completions
-                  on-screen.
+                      on-screen.
-                */
+                    */
-                i=PAGER_MAX_COLS+1;
+                    cols=PAGER_MAX_COLS+1;
-                break;
+                    break;
+            }
         }
+        assert(cols >= 0);
+        rendering.cols = (size_t)cols;
+        rendering.rows = divide_round_up(completion_infos.size(), rendering.cols);
     }
     return rendering;
 }
 
+void pager_t::update_rendering(page_rendering_t *rendering) const
+{
+    if (rendering->term_width != this->term_width || rendering->term_height != this->term_height || rendering->selected_completion_idx != this->selected_completion_idx)
+    {
+        *rendering = this->render();
+    }
+}
+
 pager_t::pager_t() : term_width(0), term_height(0), selected_completion_idx(-1)
 {
 }
@@ -906,6 +933,62 @@ void pager_t::set_selected_completion(size_t idx)
     this->selected_completion_idx = idx;
 }
 
+bool pager_t::select_next_completion_in_direction(cardinal_direction_t direction, const page_rendering_t &rendering)
+{
+    /* Handle the case of nothing selected yet */
+    if (selected_completion_idx == (size_t)(-1))
+    {
+        return false;
+    }
+ 
+    /* We have a completion index; we wish to compute its row and column. Completions are rendered column first, i.e. we go south before we go west. */
+    size_t current_row = selected_completion_idx % rendering.rows;
+    size_t current_col = selected_completion_idx / rendering.rows;
+    
+    switch (direction)
+    {
+        case direction_north:
+        {
+            /* Go up a whole row */
+            if (current_row > 0)
+                current_row--;
+            break;
+        }
+            
+        case direction_south:
+        {
+            /* Go down, unless we are in the last row. Note that this means that we may set selected_completion_idx to an out-of-bounds value if the last row is incomplete; this is a feature (it allows "last column memory"). */
+            if (current_row + 1 < rendering.rows)
+                current_row++;
+            break;
+        }
+        
+        case direction_east:
+        {
+            if (current_col + 1 < rendering.cols)
+                current_col++;
+            break;
+        }
+        
+        case direction_west:
+        {
+            if (current_col > 0)
+                current_col--;
+            break;
+        }
+    }
+    
+    size_t new_selected_completion_idx = current_col * rendering.rows + current_row;
+    if (new_selected_completion_idx != selected_completion_idx)
+    {
+        selected_completion_idx = new_selected_completion_idx;
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+}
 
 void pager_t::clear()
 {
@@ -913,3 +996,7 @@ void pager_t::clear()
     completion_infos.clear();
     prefix.clear();
 }
+
+page_rendering_t::page_rendering_t() : term_width(-1), term_height(-1), rows(0), cols(0), selected_completion_idx(-1)
+{
+}

pager.h

@@ -6,9 +6,18 @@
 #include "screen.h"
 
 /* Represents rendering from the pager */
-struct page_rendering_t
+class page_rendering_t
 {
+    public:
+    int term_width;
+    int term_height;
+    size_t rows;
+    size_t cols;
+    size_t selected_completion_idx;
     screen_data_t screen_data;
+    
+    /* Returns a rendering with invalid data, useful to indicate "no rendering" */
+    page_rendering_t();
 };
 
 typedef std::vector<completion_t> completion_list_t;
@@ -79,9 +88,15 @@ class pager_t
     /* Sets the index of the selected completion */
     void set_selected_completion(size_t completion_idx);
     
+    /* Changes the selected completion in the given direction according to the layout of the given rendering. Returns true if the values changed. */
+    bool select_next_completion_in_direction(cardinal_direction_t direction, const page_rendering_t &rendering);
+    
     /* Produces a rendering of the completions, at the given term size */
     page_rendering_t render() const;
     
+    /* Updates the rendering if it's stale */
+    void update_rendering(page_rendering_t *rendering) const;
+    
     /* Indicates if there are no completions, and therefore nothing to render */
     bool empty() const;
     

reader.cpp

@@ -201,6 +201,9 @@ public:
     /** Current pager */
     pager_t current_pager;
     
+    /** Current page rendering */
+    page_rendering_t current_page_rendering;
+    
     /** Whether we are navigating the pager */
     bool is_navigating_pager;
 
@@ -538,6 +541,10 @@ static void reader_repaint()
 
     std::vector<int> indents = data->indents;
     indents.resize(len);
+    
+    // Re-render our completions page if necessary
+    data->current_pager.set_term_size(common_get_width(), common_get_height());
+    data->current_pager.update_rendering(&data->current_page_rendering);
 
     s_write(&data->screen,
             data->left_prompt_buff,
@@ -547,7 +554,7 @@ static void reader_repaint()
             &colors[0],
             &indents[0],
             data->buff_pos,
-            data->current_pager);
+            data->current_page_rendering);
 
     data->repaint_needed = false;
 }
@@ -1852,33 +1859,31 @@ static bool handle_completions(const std::vector<completion_t> &comp)
                 prefix.append(data->command_line, prefix_start + len - PREFIX_MAX_LEN, PREFIX_MAX_LEN);
             }
 
+            wchar_t quote;
+            parse_util_get_parameter_info(data->command_line, data->buff_pos, &quote, NULL, NULL);
+            bool is_quoted = (quote != L'\0');
+            
+            if (1)
             {
-                int is_quoted;
+                data->current_pager.set_prefix(prefix);
-
+                data->current_pager.set_completions(surviving_completions);
-                wchar_t quote;
-                parse_util_get_parameter_info(data->command_line, data->buff_pos, &quote, NULL, NULL);
-                is_quoted = (quote != L'\0');
                 
-                if (1)
+                /* Invalidate our rendering */
-                {
+                data->current_page_rendering = page_rendering_t();
-                    pager_t pager;
+            }
-                    pager.set_term_size(common_get_width(), common_get_height());
+            else
-                    pager.set_prefix(prefix);
+            {
-                    pager.set_completions(surviving_completions);
+                /* Clear the autosuggestion from the old commandline before abandoning it (see #561) */
-                    data->current_pager = pager;
+                if (! data->autosuggestion.empty())
-                }
+                    reader_repaint_without_autosuggestion();
-                else
+
-                {
+                write_loop(1, "\n", 1);
-                    /* Clear the autosuggestion from the old commandline before abandoning it (see #561) */
+
-                    if (! data->autosuggestion.empty())
+                run_pager(prefix, is_quoted, surviving_completions);
-                        reader_repaint_without_autosuggestion();
+                
-
+                s_reset(&data->screen, screen_reset_abandon_line);
-                    write_loop(1, "\n", 1);
+
-
-                    run_pager(prefix, is_quoted, surviving_completions);
-                }
             }
-            s_reset(&data->screen, screen_reset_abandon_line);
             reader_repaint();
             success = false;
         }
@@ -2970,6 +2975,9 @@ const wchar_t *reader_readline(void)
 
     /* The cycle index in our completion list */
     size_t completion_cycle_idx = (size_t)(-1);
+    
+    /* Indicates if we are currently navigating pager contents */
+    bool is_navigating_pager_contents = false;
 
     /* The command line before completion */
     wcstring cycle_command_line;
@@ -3152,6 +3160,8 @@ const wchar_t *reader_readline(void)
                     
                     if (next_comp != NULL)
                     {
+                        is_navigating_pager_contents = true;
+                    
                         size_t cursor_pos = cycle_cursor_pos;
                         const wcstring new_cmd_line = completion_apply_to_command_line(next_comp->completion, next_comp->flags, cycle_command_line, &cursor_pos, false);
                         reader_set_buffer(new_cmd_line, cursor_pos);
@@ -3628,38 +3638,49 @@ const wchar_t *reader_readline(void)
             case R_UP_LINE:
             case R_DOWN_LINE:
             {
-                int line_old = parse_util_get_line_from_offset(data->command_line, data->buff_pos);
+                if (is_navigating_pager_contents)
-                int line_new;
-
-                if (c == R_UP_LINE)
-                    line_new = line_old-1;
-                else
-                    line_new = line_old+1;
-
-                int line_count = parse_util_lineno(data->command_line.c_str(), data->command_length())-1;
-
-                if (line_new >= 0 && line_new <= line_count)
                 {
-                    size_t base_pos_new;
+                    if (data->current_pager.select_next_completion_in_direction(c == R_UP_LINE ? direction_north : direction_south, data->current_page_rendering))
-                    size_t base_pos_old;
+                    {
+                        reader_repaint();
+                    }
+                }
+                else
+                {
+                    /* Not navigating the pager contents */
+                    int line_old = parse_util_get_line_from_offset(data->command_line, data->buff_pos);
+                    int line_new;
 
-                    int indent_old;
+                    if (c == R_UP_LINE)
-                    int indent_new;
+                        line_new = line_old-1;
-                    size_t line_offset_old;
+                    else
-                    size_t total_offset_new;
+                        line_new = line_old+1;
 
-                    base_pos_new = parse_util_get_offset_from_line(data->command_line, line_new);
+                    int line_count = parse_util_lineno(data->command_line.c_str(), data->command_length())-1;
 
-                    base_pos_old = parse_util_get_offset_from_line(data->command_line,  line_old);
+                    if (line_new >= 0 && line_new <= line_count)
+                    {
+                        size_t base_pos_new;
+                        size_t base_pos_old;
 
-                    assert(base_pos_new != (size_t)(-1) && base_pos_old != (size_t)(-1));
+                        int indent_old;
-                    indent_old = data->indents.at(base_pos_old);
+                        int indent_new;
-                    indent_new = data->indents.at(base_pos_new);
+                        size_t line_offset_old;
+                        size_t total_offset_new;
 
-                    line_offset_old = data->buff_pos - parse_util_get_offset_from_line(data->command_line, line_old);
+                        base_pos_new = parse_util_get_offset_from_line(data->command_line, line_new);
-                    total_offset_new = parse_util_get_offset(data->command_line, line_new, line_offset_old - 4*(indent_new-indent_old));
+
-                    data->buff_pos = total_offset_new;
+                        base_pos_old = parse_util_get_offset_from_line(data->command_line,  line_old);
-                    reader_repaint();
+
+                        assert(base_pos_new != (size_t)(-1) && base_pos_old != (size_t)(-1));
+                        indent_old = data->indents.at(base_pos_old);
+                        indent_new = data->indents.at(base_pos_new);
+
+                        line_offset_old = data->buff_pos - parse_util_get_offset_from_line(data->command_line, line_old);
+                        total_offset_new = parse_util_get_offset(data->command_line, line_new, line_offset_old - 4*(indent_new-indent_old));
+                        data->buff_pos = total_offset_new;
+                        reader_repaint();
+                    }
                 }
 
                 break;

screen.cpp

@@ -1237,7 +1237,7 @@ void s_write(screen_t *s,
              const highlight_spec_t *colors,
              const int *indent,
              size_t cursor_pos,
-             const pager_t &pager)
+             const page_rendering_t &pager)
 {
     screen_data_t::cursor_t cursor_arr;
 
@@ -1325,12 +1325,8 @@ void s_write(screen_t *s,
 
     s->desired.cursor = cursor_arr;
     
-    /* append pager_data */
+    /* Append pager_data (none if empty) */
-    if (! pager.empty())
+    s->desired.append_lines(pager.screen_data);
-    {
-        const page_rendering_t rendering = pager.render();
-        s->desired.append_lines(rendering.screen_data);
-    }
     
     s_update(s, layout.left_prompt.c_str(), layout.right_prompt.c_str());
     s_save_status(s);

screen.h

@@ -16,7 +16,7 @@
 #include <sys/stat.h>
 #include "highlight.h"
 
-class pager_t;
+class page_rendering_t;
 
 /**
    A class representing a single line of a screen.
@@ -117,6 +117,11 @@ public:
     {
         this->line_datas.insert(this->line_datas.end(), d.line_datas.begin(), d.line_datas.end());
     }
+    
+    bool empty() const
+    {
+        return line_datas.empty();
+    }
 };
 
 /**
@@ -205,7 +210,7 @@ void s_write(screen_t *s,
              const highlight_spec_t *colors,
              const int *indent,
              size_t cursor_pos,
-             const pager_t &pager_data);
+             const page_rendering_t &pager_data);
 
 /**
     This function resets the screen buffers internal knowledge about