From 08588cd0fc25128a8ec013d65ca15d8e66d28e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 19 May 2020 18:49:42 +0200 Subject: [PATCH] selection: handle viewport wrap around correctly When the viewport wraps around, the selection points may be "incorrect". --- CHANGELOG.md | 4 +-- commands.c | 7 ++++-- selection.c | 28 +++++++++++++++++++++ selection.h | 3 +++ terminal.c | 71 +++++++++++++++++++--------------------------------- 5 files changed, 64 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f0ca7f..b7256188 100644 --- a/CHANGELOG.md +++ b/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 - terminal content was scrolled. +* Selection handling when viewport wrapped around. + ### Security diff --git a/commands.c b/commands.c index ffe03f3b..2727c1f2 100644 --- a/commands.c +++ b/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) { diff --git a/selection.c b/selection.c index 69acbe4e..53664188 100644 --- a/selection.c +++ b/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, diff --git a/selection.h b/selection.h index 3a520820..758d5d29 100644 --- a/selection.h +++ b/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); diff --git a/terminal.c b/terminal.c index 5dd29aa0..5e17e7c9 100644 --- a/terminal.c +++ b/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 (likely(term->selection.end.row != -1)) { - /* - * Selection is (partly) inside either the top or bottom - * scrolling regions, or on (at least one) of the lines - * scrolled in (i.e. re-used lines). - */ - if (selection_on_top_region(term, region) || - selection_on_bottom_region(term, region) || - 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. - */ + if (unlikely(term->selection.end.row >= 0)) { + /* + * Selection is (partly) inside either the top or bottom + * scrolling regions, or on (at least one) of the lines + * scrolled in (i.e. re-used lines). + */ + if (selection_on_top_region(term, region) || + selection_on_bottom_region(term, region) || + selection_on_rows(term, region.end - rows, region.end - 1)) + { 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 (likely(term->selection.end.row != -1)) { - /* - * Selection is (partly) inside either the top or bottom - * scrolling regions, or on (at least one) of the lines - * scrolled in (i.e. re-used lines). - */ - if (selection_on_top_region(term, region) || - selection_on_bottom_region(term, region) || - 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. - */ + if (unlikely(term->selection.end.row >= 0)) { + /* + * Selection is (partly) inside either the top or bottom + * scrolling regions, or on (at least one) of the lines + * scrolled in (i.e. re-used lines). + */ + if (selection_on_top_region(term, region) || + selection_on_bottom_region(term, region) || + selection_on_rows(term, region.start, region.start + rows - 1)) + { 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++)