selection: handle viewport wrap around correctly

When the viewport wraps around, the selection points may be
"incorrect".

CHANGELOG.md

@@ -36,8 +36,8 @@
   when the mouse button is released - not as soon as `shift` is
   released.
 * Selected cells did not appear selected if modified.
-* Very rare crash when beginning a selection at the same time the
+* Selection handling when viewport wrapped around.
-  terminal content was scrolled.
+
 
 ### Security
 

commands.c

@@ -3,9 +3,10 @@
 #define LOG_MODULE "commands"
 #define LOG_ENABLE_DBG 0
 #include "log.h"
-#include "terminal.h"
-#include "render.h"
 #include "grid.h"
+#include "render.h"
+#include "selection.h"
+#include "terminal.h"
 #include "util.h"
 
 void
@@ -73,6 +74,7 @@ cmd_scrollback_up(struct terminal *term, int rows)
     else
         diff = (term->grid->num_rows - new_view) + term->grid->view;
 
+    selection_view_up(term, new_view);
     term->grid->view = new_view;
 
     if (diff >= 0 && diff < term->rows) {
@@ -146,6 +148,7 @@ cmd_scrollback_down(struct terminal *term, int rows)
     else
         diff = (term->grid->num_rows - term->grid->view) + new_view;
 
+    selection_view_down(term, new_view);
     term->grid->view = new_view;
 
     if (diff >= 0 && diff < term->rows) {

selection.c

@@ -69,6 +69,34 @@ selection_on_rows(const struct terminal *term, int row_start, int row_end)
     return false;
 }
 
+void
+selection_view_up(struct terminal *term, int new_view)
+{
+    if (likely(term->selection.start.row < 0))
+        return;
+
+    if (likely(new_view < term->grid->view))
+        return;
+
+    term->selection.start.row += term->grid->num_rows;
+    if (term->selection.end.row >= 0)
+        term->selection.end.row += term->grid->num_rows;
+}
+
+void
+selection_view_down(struct terminal *term, int new_view)
+{
+    if (likely(term->selection.start.row < 0))
+        return;
+
+    if (likely(new_view > term->grid->view))
+        return;
+
+    term->selection.start.row &= term->grid->num_rows - 1;
+    if (term->selection.end.row >= 0)
+        term->selection.end.row &= term->grid->num_rows - 1;
+}
+
 static void
 foreach_selected_normal(
     struct terminal *term, struct coord _start, struct coord _end,

selection.h

@@ -19,6 +19,9 @@ void selection_extend(struct terminal *term, int col, int row, uint32_t serial);
 
 bool selection_on_rows(const struct terminal *term, int start, int end);
 
+void selection_view_up(struct terminal *term, int new_view);
+void selection_view_down(struct terminal *term, int new_view);
+
 void selection_mark_word(struct terminal *term, int col, int row,
                          bool spaces_only, uint32_t serial);
 void selection_mark_row(struct terminal *term, int row, uint32_t serial);

terminal.c

@@ -1748,28 +1748,16 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows
     assert(rows <= region.end - region.start);
 
     /* Cancel selections that cannot be scrolled */
-    if (unlikely(term->selection.start.row != -1)) {
+    if (unlikely(term->selection.end.row >= 0)) {
-        if (likely(term->selection.end.row != -1)) {
+        /*
-            /*
+         * Selection is (partly) inside either the top or bottom
-             * Selection is (partly) inside either the top or bottom
+         * scrolling regions, or on (at least one) of the lines
-             * scrolling regions, or on (at least one) of the lines
+         * scrolled in (i.e. re-used lines).
-             * scrolled in (i.e. re-used lines).
+         */
-             */
+        if (selection_on_top_region(term, region) ||
-            if (selection_on_top_region(term, region) ||
+            selection_on_bottom_region(term, region) ||
-                selection_on_bottom_region(term, region) ||
+            selection_on_rows(term, region.end - rows, region.end - 1))
-                selection_on_rows(term, region.end - rows, region.end - 1))
+        {
-            {
-                selection_cancel(term);
-            }
-        } else {
-            /*
-             * User started a selection, but didn't move the
-             * cursor.
-             *
-             * Not 100% sure this is needed for forward scrolling, but
-             * let's keep it here, for consistency with reverse
-             * scrolling.
-             */
             selection_cancel(term);
         }
     }
@@ -1778,8 +1766,10 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows
     term->grid->offset += rows;
     term->grid->offset &= term->grid->num_rows - 1;
 
-    if (view_follows)
+    if (view_follows) {
+        selection_view_down(term, term->grid->offset);
         term->grid->view = term->grid->offset;
+    }
 
     /* Top non-scrolling region. */
     for (int i = region.start - 1; i >= 0; i--)
@@ -1820,27 +1810,16 @@ term_scroll_reverse_partial(struct terminal *term,
     assert(rows <= region.end - region.start);
 
     /* Cancel selections that cannot be scrolled */
-    if (unlikely(term->selection.start.row != -1)) {
+    if (unlikely(term->selection.end.row >= 0)) {
-        if (likely(term->selection.end.row != -1)) {
+        /*
-            /*
+         * Selection is (partly) inside either the top or bottom
-             * Selection is (partly) inside either the top or bottom
+         * scrolling regions, or on (at least one) of the lines
-             * scrolling regions, or on (at least one) of the lines
+         * scrolled in (i.e. re-used lines).
-             * scrolled in (i.e. re-used lines).
+         */
-             */
+        if (selection_on_top_region(term, region) ||
-            if (selection_on_top_region(term, region) ||
+            selection_on_bottom_region(term, region) ||
-                selection_on_bottom_region(term, region) ||
+            selection_on_rows(term, region.start, region.start + rows - 1))
-                selection_on_rows(term, region.start, region.start + rows - 1))
+        {
-            {
-                selection_cancel(term);
-            }
-        } else {
-            /*
-             * User started a selection, but didn't move the
-             * cursor.
-             *
-             * Since we're scrolling in reverse, the result may be a
-             * *huge* selection that covers empty (NULL) lines.
-             */
             selection_cancel(term);
         }
     }
@@ -1854,8 +1833,10 @@ term_scroll_reverse_partial(struct terminal *term,
     assert(term->grid->offset >= 0);
     assert(term->grid->offset < term->grid->num_rows);
 
-    if (view_follows)
+    if (view_follows) {
+        selection_view_up(term, term->grid->offset);
         term->grid->view = term->grid->offset;
+    }
 
     /* Bottom non-scrolling region */
     for (int i = region.end + rows; i < term->rows + rows; i++)