diff --git a/CHANGELOG.md b/CHANGELOG.md index a7ad4371..89c19a0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,10 @@ box, that does not result in a grid update (for example, when the search criteria did not result in any matches) ([#1040][1040]). * foot freezing in scrollback search mode, using 100% CPU ([#1036][1036]). +* Crash when extending a selection to the next word boundary in + scrollback search mode ([#1036][1036]). +* Scrollback search mode not always highlighting all matches + correctly. [1040]: https://codeberg.org/dnkl/foot/issues/1040 [1036]: https://codeberg.org/dnkl/foot/issues/1036 diff --git a/grid.c b/grid.c index 5a0b5c7e..4a3995a0 100644 --- a/grid.c +++ b/grid.c @@ -15,6 +15,34 @@ #define TIME_REFLOW 0 +/* + * “sb” (scrollback relative) coordinates + * + * The scrollback relative row number 0 is the *first*, and *oldest* + * row in the scrollback history (and thus the *first* row to be + * scrolled out). Thus, a higher number means further *down* in the + * scrollback, with the *highest* number being at the bottom of the + * screen, where new input appears. + */ +int +grid_row_abs_to_sb(const struct grid *grid, int screen_rows, int abs_row) +{ + const int scrollback_start = grid->offset + screen_rows; + int rebased_row = abs_row - scrollback_start + grid->num_rows; + + rebased_row &= grid->num_rows - 1; + return rebased_row; +} + +int grid_row_sb_to_abs(const struct grid *grid, int screen_rows, int sb_rel_row) +{ + const int scrollback_start = grid->offset + screen_rows; + int abs_row = sb_rel_row + scrollback_start; + + abs_row &= grid->num_rows - 1; + return abs_row; +} + static void ensure_row_has_extra_data(struct row *row) { diff --git a/grid.h b/grid.h index 7819db4d..22bd76bb 100644 --- a/grid.h +++ b/grid.h @@ -21,6 +21,10 @@ void grid_resize_and_reflow( size_t tracking_points_count, struct coord *const _tracking_points[static tracking_points_count]); +/* Convert row numbers between scrollback-relative and absolute coordinates */ +int grid_row_abs_to_sb(const struct grid *grid, int screen_rows, int abs_row); +int grid_row_sb_to_abs(const struct grid *grid, int screen_rows, int sb_rel_row); + static inline int grid_row_absolute(const struct grid *grid, int row_no) { @@ -33,6 +37,7 @@ grid_row_absolute_in_view(const struct grid *grid, int row_no) return (grid->view + row_no) & (grid->num_rows - 1); } + static inline struct row * _grid_row_maybe_alloc(struct grid *grid, int row_no, bool alloc_if_null) { diff --git a/search.c b/search.c index 73f831eb..09b59d50 100644 --- a/search.c +++ b/search.c @@ -82,11 +82,7 @@ search_ensure_size(struct terminal *term, size_t wanted_size) static bool has_wrapped_around(const struct terminal *term, int abs_row_no) { - const struct grid *grid = term->grid; - int scrollback_start = grid->offset + term->rows; - int rebased_row = abs_row_no - scrollback_start + grid->num_rows; - rebased_row &= grid->num_rows - 1; - + int rebased_row = grid_row_abs_to_sb(term->grid, term->rows, abs_row_no); return rebased_row == 0; } @@ -182,6 +178,9 @@ search_update_selection(struct terminal *term, const struct range *match) int end_row = match->end.row; int end_col = match->end.col; + xassert(start_row >= 0); + xassert(start_row < grid->num_rows); + bool move_viewport = true; int view_end = (grid->view + term->rows - 1) & (grid->num_rows - 1); @@ -196,24 +195,14 @@ search_update_selection(struct terminal *term, const struct range *match) } if (move_viewport) { - int old_view = grid->view; - int new_view = start_row - term->rows / 2; + int rebased_new_view = grid_row_abs_to_sb(grid, term->rows, start_row); - while (new_view < 0) - new_view += grid->num_rows; + rebased_new_view -= term->rows / 2; + rebased_new_view = + min(max(rebased_new_view, 0), grid->num_rows - term->rows); - new_view = ensure_view_is_allocated(term, new_view); - - /* Don't scroll past scrollback history */ - int end = (grid->offset + term->rows - 1) & (grid->num_rows - 1); - if (end >= grid->offset) { - /* Not wrapped */ - if (new_view >= grid->offset && new_view <= end) - new_view = grid->offset; - } else { - if (new_view >= grid->offset || new_view <= end) - new_view = grid->offset; - } + const int old_view = grid->view; + int new_view = grid_row_sb_to_abs(grid, term->rows, rebased_new_view); #if defined(_DEBUG) /* Verify all to-be-visible rows have been allocated */ @@ -227,23 +216,8 @@ search_update_selection(struct terminal *term, const struct range *match) term_damage_view(term); } -#if 0 - /* Selection endpoint is inclusive */ - if (--end_col < 0) { - end_col = term->cols - 1; - end_row--; - } -#endif - - /* - * Begin a new selection if the start coords changed - * - * Note: check column against selection.coords, since our “old” - * start column isn’t reliable - we modify it when searching - * “next” or “prev”. - */ if (start_row != term->search.match.row || - start_col != term->selection.coords.start.col) + start_col != term->search.match.col) { int selection_row = start_row - grid->view + grid->num_rows; selection_row &= grid->num_rows - 1; @@ -516,7 +490,7 @@ search_matches_new_iter(struct terminal *term) { return (struct search_match_iterator){ .term = term, - .start = {-2, -2}, + .start = {0, 0}, }; } @@ -529,86 +503,63 @@ search_matches_next(struct search_match_iterator *iter) if (term->search.match_len == 0) goto no_match; - struct range match; - bool found; - - const bool return_primary_match = - iter->start.row == -2 && term->selection.coords.end.row >= 0; - - if (return_primary_match) { - /* First, return the primary match */ - match = term->selection.coords; - found = true; - } - - else if (iter->start.row >= term->rows) { + if (iter->start.row >= term->rows) goto no_match; + + xassert(iter->start.row >= 0); + xassert(iter->start.row < term->rows); + xassert(iter->start.col >= 0); + xassert(iter->start.col < term->cols); + + struct coord abs_start = iter->start; + abs_start.row = grid_row_absolute_in_view(grid, abs_start.row); + + struct coord abs_end = { + term->cols - 1, + grid_row_absolute_in_view(grid, term->rows - 1)}; + + struct range match; + bool found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match); + if (!found) + goto no_match; + + LOG_DBG("match at (absolute coordinates) %dx%d-%dx%d", + match.start.row, match.start.col, + match.end.row, match.end.col); + + /* Convert absolute row numbers back to view relative */ + match.start.row = match.start.row - grid->view + grid->num_rows; + match.start.row &= grid->num_rows - 1; + match.end.row = match.end.row - grid->view + grid->num_rows; + match.end.row &= grid->num_rows - 1; + + LOG_DBG("match at (view-local coordinates) %dx%d-%dx%d, view=%d", + match.start.row, match.start.col, + match.end.row, match.end.col, grid->view); + + xassert(match.start.row >= 0); + xassert(match.start.row < term->rows); + xassert(match.end.row >= 0); + xassert(match.end.row < term->rows); + + xassert(match.end.row > match.start.row || + (match.end.row == match.start.row && + match.end.col >= match.start.col)); + + /* Continue at next column, next time */ + iter->start.row = match.start.row; + iter->start.col = match.start.col + 1; + + if (iter->start.col >= term->cols) { + iter->start.col = 0; + iter->start.row++; /* Overflow is caught in next iteration */ } - else { - xassert(iter->start.row >= 0); - xassert(iter->start.row < term->rows); - xassert(iter->start.col >= 0); - xassert(iter->start.col < term->cols); - - struct coord abs_start = iter->start; - abs_start.row = grid_row_absolute_in_view(grid, abs_start.row); - - struct coord abs_end = { - term->cols - 1, - grid_row_absolute_in_view(grid, term->rows - 1)}; - - found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match); - } - - if (found) { - LOG_DBG("match at %dx%d-%dx%d", - match.start.row, match.start.col, - match.end.row, match.end.col); - - /* Convert absolute row numbers back to view relative */ - match.start.row = match.start.row - grid->view + grid->num_rows; - match.start.row &= grid->num_rows - 1; - match.end.row = match.end.row - grid->view + grid->num_rows; - match.end.row &= grid->num_rows - 1; - - xassert(match.start.row >= 0); - xassert(match.start.row < term->rows); - xassert(match.end.row >= 0); - xassert(match.end.row < term->rows); - - xassert(match.end.row > match.start.row || - (match.end.row == match.start.row && - match.end.col >= match.start.col)); - - if (return_primary_match) { - iter->start.row = 0; - iter->start.col = 0; - } else { - /* Continue at next column, next time */ - iter->start.row = match.start.row; - iter->start.col = match.start.col + 1; - - if (iter->start.col >= term->cols) { - iter->start.col = 0; - iter->start.row++; /* Overflow is caught in next iteration */ - } - - xassert(iter->start.row >= 0); - xassert(iter->start.row <= term->rows); - xassert(iter->start.col >= 0); - xassert(iter->start.col < term->cols); - - if (match.start.row == term->search.match.row && - match.start.col == term->search.match.col) - { - /* Primary match is handled explicitly */ - LOG_DBG("primary match: skipping"); - return search_matches_next(iter); - } - } - return match; - } + xassert(iter->start.row >= 0); + xassert(iter->start.row <= term->rows); + xassert(iter->start.col >= 0); + xassert(iter->start.col < term->cols); + return match; no_match: iter->start.row = -1; @@ -663,15 +614,18 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only) if (term->search.match_len == 0) return; - xassert(term->selection.coords.end.row != -1); + xassert(term->selection.coords.end.row >= 0); struct grid *grid = term->grid; const bool move_cursor = term->search.cursor == term->search.len; - const struct coord old_end = term->selection.coords.end; + struct coord old_end = selection_get_end(term); struct coord new_end = old_end; struct row *row = NULL; + xassert(new_end.row >= 0); + xassert(new_end.row < grid->num_rows); + /* Advances a coordinate by one column, to the right. Returns * false if we’ve reached the scrollback wrap-around */ #define advance_pos(coord) __extension__ \ @@ -691,12 +645,16 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only) if (!advance_pos(new_end)) return; + xassert(new_end.row >= 0); + xassert(new_end.row < grid->num_rows); xassert(grid->rows[new_end.row] != NULL); /* Find next word boundary */ - new_end.row -= grid->view; + new_end.row -= grid->view + grid->num_rows; + new_end.row &= grid->num_rows - 1; selection_find_word_boundary_right(term, &new_end, spaces_only); new_end.row += grid->view; + new_end.row &= grid->num_rows - 1; struct coord pos = old_end; row = grid->rows[pos.row]; diff --git a/selection.c b/selection.c index 8a18c6bb..091d3ed0 100644 --- a/selection.c +++ b/selection.c @@ -38,6 +38,29 @@ static const char *const mime_type_map[] = { [DATA_OFFER_MIME_TEXT_UTF8_STRING] = "UTF8_STRING", }; +static inline struct coord +bounded(const struct grid *grid, struct coord coord) +{ + coord.row &= grid->num_rows - 1; + return coord; +} + +struct coord +selection_get_start(const struct terminal *term) +{ + if (term->selection.coords.start.row < 0) + return term->selection.coords.start; + return bounded(term->grid, term->selection.coords.start); +} + +struct coord +selection_get_end(const struct terminal *term) +{ + if (term->selection.coords.end.row < 0) + return term->selection.coords.end; + return bounded(term->grid, term->selection.coords.end); +} + bool selection_on_rows(const struct terminal *term, int row_start, int row_end) { @@ -2461,3 +2484,4 @@ const struct zwp_primary_selection_device_v1_listener primary_selection_device_l .data_offer = &primary_data_offer, .selection = &primary_selection, }; + diff --git a/selection.h b/selection.h index 295d8d1c..0a6ece91 100644 --- a/selection.h +++ b/selection.h @@ -79,3 +79,6 @@ void selection_find_word_boundary_left( struct terminal *term, struct coord *pos, bool spaces_only); void selection_find_word_boundary_right( struct terminal *term, struct coord *pos, bool spaces_only); + +struct coord selection_get_start(const struct terminal *term); +struct coord selection_get_end(const struct terminal *term); diff --git a/sixel.c b/sixel.c index 5316c0ad..0c94117f 100644 --- a/sixel.c +++ b/sixel.c @@ -7,8 +7,9 @@ #define LOG_ENABLE_DBG 0 #include "log.h" #include "debug.h" -#include "render.h" +#include "grid.h" #include "hsl.h" +#include "render.h" #include "util.h" #include "xmalloc.h" #include "xsnprintf.h" @@ -138,25 +139,6 @@ sixel_erase(struct terminal *term, struct sixel *sixel) sixel_destroy(sixel); } -/* - * Calculates the scrollback relative row number, given an absolute row number. - * - * The scrollback relative row number 0 is the *first*, and *oldest* - * row in the scrollback history (and thus the *first* row to be - * scrolled out). Thus, a higher number means further *down* in the - * scrollback, with the *highest* number being at the bottom of the - * screen, where new input appears. - */ -static int -rebase_row(const struct terminal *term, int abs_row) -{ - int scrollback_start = term->grid->offset + term->rows; - int rebased_row = abs_row - scrollback_start + term->grid->num_rows; - - rebased_row &= term->grid->num_rows - 1; - return rebased_row; -} - /* * Verify the sixels are sorted correctly. * @@ -175,7 +157,8 @@ verify_list_order(const struct terminal *term) size_t idx = 0; tll_foreach(term->grid->sixel_images, it) { - int row = rebase_row(term, it->item.pos.row + it->item.rows - 1); + int row = grid_row_abs_to_sb( + term->grid, term->rows, it->item.pos.row + it->item.rows - 1); int col = it->item.pos.col; int col_count = it->item.cols; @@ -232,7 +215,8 @@ verify_scrollback_consistency(const struct terminal *term) int last_row = -1; for (int i = 0; i < six->rows; i++) { - int row_no = rebase_row(term, six->pos.row + i); + int row_no = grid_row_abs_to_sb( + term->grid, term->rows, six->pos.row + i); if (last_row != -1) xassert(last_row < row_no); @@ -295,10 +279,14 @@ verify_sixels(const struct terminal *term) static void sixel_insert(struct terminal *term, struct sixel sixel) { - int end_row = rebase_row(term, sixel.pos.row + sixel.rows - 1); + int end_row = grid_row_abs_to_sb( + term->grid, term->rows, sixel.pos.row + sixel.rows - 1); tll_foreach(term->grid->sixel_images, it) { - if (rebase_row(term, it->item.pos.row + it->item.rows - 1) < end_row) { + int rebased = grid_row_abs_to_sb( + term->grid, term->rows, it->item.pos.row + it->item.rows - 1); + + if (rebased < end_row) { tll_insert_before(term->grid->sixel_images, it, sixel); goto out; } @@ -325,7 +313,7 @@ sixel_scroll_up(struct terminal *term, int rows) tll_rforeach(term->grid->sixel_images, it) { struct sixel *six = &it->item; - int six_start = rebase_row(term, six->pos.row); + int six_start = grid_row_abs_to_sb(term->grid, term->rows, six->pos.row); if (six_start < rows) { sixel_erase(term, six); @@ -358,7 +346,8 @@ sixel_scroll_down(struct terminal *term, int rows) tll_foreach(term->grid->sixel_images, it) { struct sixel *six = &it->item; - int six_end = rebase_row(term, six->pos.row + six->rows - 1); + int six_end = grid_row_abs_to_sb( + term->grid, term->rows, six->pos.row + six->rows - 1); if (six_end >= term->grid->num_rows - rows) { sixel_erase(term, six); tll_remove(term->grid->sixel_images, it); @@ -668,7 +657,8 @@ _sixel_overwrite_by_rectangle( /* We should never generate scrollback wrapping sixels */ xassert(end < term->grid->num_rows); - const int scrollback_rel_start = rebase_row(term, start); + const int scrollback_rel_start = grid_row_abs_to_sb( + term->grid, term->rows, start); bool UNUSED would_have_breaked = false; @@ -677,7 +667,8 @@ _sixel_overwrite_by_rectangle( const int six_start = six->pos.row; const int six_end = (six_start + six->rows - 1); - const int six_scrollback_rel_end = rebase_row(term, six_end); + const int six_scrollback_rel_end = + grid_row_abs_to_sb(term->grid, term->rows, six_end); /* We should never generate scrollback wrapping sixels */ xassert(six_end < term->grid->num_rows); @@ -776,7 +767,7 @@ sixel_overwrite_by_row(struct terminal *term, int _row, int col, int width) width = term->grid->num_cols - col; const int row = (term->grid->offset + _row) & (term->grid->num_rows - 1); - const int scrollback_rel_row = rebase_row(term, row); + const int scrollback_rel_row = grid_row_abs_to_sb(term->grid, term->rows, row); tll_foreach(term->grid->sixel_images, it) { struct sixel *six = &it->item; @@ -786,7 +777,8 @@ sixel_overwrite_by_row(struct terminal *term, int _row, int col, int width) /* We should never generate scrollback wrapping sixels */ xassert(six_end >= six_start); - const int six_scrollback_rel_end = rebase_row(term, six_end); + const int six_scrollback_rel_end = + grid_row_abs_to_sb(term->grid, term->rows, six_end); if (six_scrollback_rel_end < scrollback_rel_row) { /* All remaining sixels are *before* "our" row */ @@ -888,7 +880,8 @@ sixel_reflow(struct terminal *term) int last_row = -1; for (int j = 0; j < six->rows; j++) { - int row_no = rebase_row(term, six->pos.row + j); + int row_no = grid_row_abs_to_sb( + term->grid, term->rows, six->pos.row + j); if (last_row != -1 && last_row >= row_no) { sixel_destroy(six); sixel_destroyed = true; diff --git a/terminal.c b/terminal.c index f1b05f57..7d230cdf 100644 --- a/terminal.c +++ b/terminal.c @@ -2154,18 +2154,18 @@ term_erase(struct terminal *term, int start_row, int start_col, void term_erase_scrollback(struct terminal *term) { - const int num_rows = term->grid->num_rows; + const struct grid *grid = term->grid; + const int num_rows = grid->num_rows; const int mask = num_rows - 1; - const int start = (term->grid->offset + term->rows) & mask; - const int end = (term->grid->offset - 1) & mask; + const int start = (grid->offset + term->rows) & mask; + const int end = (grid->offset - 1) & mask; - const int scrollback_start = term->grid->offset + term->rows; - const int rel_start = (start - scrollback_start + num_rows) & mask; - const int rel_end = (end - scrollback_start + num_rows) & mask; + const int rel_start = grid_row_abs_to_sb(grid, term->rows, start); + const int rel_end = grid_row_abs_to_sb(grid, term->rows, end); - const int sel_start = term->selection.coords.start.row; - const int sel_end = term->selection.coords.end.row; + const int sel_start = selection_get_start(term).row; + const int sel_end = selection_get_end(term).row; if (sel_end >= 0) { /* @@ -2183,8 +2183,8 @@ term_erase_scrollback(struct terminal *term) * closer to the screen bottom. */ - const int rel_sel_start = (sel_start - scrollback_start + num_rows) & mask; - const int rel_sel_end = (sel_end - scrollback_start + num_rows) & mask; + const int rel_sel_start = grid_row_abs_to_sb(grid, term->rows, sel_start); + const int rel_sel_end = grid_row_abs_to_sb(grid, term->rows, sel_end); if ((rel_sel_start <= rel_start && rel_sel_end >= rel_start) || (rel_sel_start <= rel_end && rel_sel_end >= rel_end) || @@ -2196,8 +2196,9 @@ term_erase_scrollback(struct terminal *term) tll_foreach(term->grid->sixel_images, it) { struct sixel *six = &it->item; - const int six_start = (six->pos.row - scrollback_start + num_rows) & mask; - const int six_end = (six->pos.row + six->rows - 1 - scrollback_start + num_rows) & mask; + const int six_start = grid_row_abs_to_sb(grid, term->rows, six->pos.row); + const int six_end = grid_row_abs_to_sb( + grid, term->rows, six->pos.row + six->rows - 1); if ((six_start <= rel_start && six_end >= rel_start) || (six_start <= rel_end && six_end >= rel_end) ||