search: don't modify search.start coord *before* finding next match

The match logic uses the last start coordinate to determine which end
points in the selection to update. This sometimes fails when the start
coordinate has been changed by e.g. a key binding - the new start
coordinate is incorrectly matched against the old-but-modified start
coordinate, causing foot to e.g. *not* upate the selection start
coordinate.

Example:

  $ echo 'test\n\test\ntest'

Then do a scrollback search for 'test. The first match is found
correctly (the last 'test'), but searching for the previous match
(ctrl+r) does not select the middle 'test'.

Fix by passing the search direction to search_find_next(), and have
_it_ calculate the coordinate to start search. There are three possibilities:

* forward
* backward
* "backward", but at the same position

The first two are used when searching for next/prev match with ctrl+s
and ctrl+r. The last one is used when the search criteria is
updated. In this case, we don't want to move to the previous match,
*unless* the current match no longer matches.
This commit is contained in:
Daniel Eklöf 2022-04-21 18:54:27 +02:00
parent dd03e10c6c
commit 9907d7bbe9
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
3 changed files with 73 additions and 55 deletions

View file

@ -129,6 +129,8 @@
`footclient` instances (https://codeberg.org/dnkl/foot/issues/931).
* Search prev/next not updating the selection correctly when the
previous and new match overlaps.
* Various minor fixes to scrollback search, and how it finds the
next/prev match.
### Security

123
search.c
View file

@ -301,7 +301,7 @@ find_next(struct terminal *term, enum search_direction direction,
#define ROW_INC(_r) ((_r) = ((_r) + 1) & (grid->num_rows - 1))
struct grid *grid = term->grid;
const bool backward = direction == SEARCH_BACKWARD;
const bool backward = direction != SEARCH_FORWARD;
LOG_DBG("%s: start: %dx%d, end: %dx%d", backward ? "backward" : "forward",
abs_start.row, abs_start.col, abs_end.row, abs_end.col);
@ -401,13 +401,9 @@ find_next(struct terminal *term, enum search_direction direction,
}
static void
search_find_next(struct terminal *term)
search_find_next(struct terminal *term, enum search_direction direction)
{
struct grid *grid = term->grid;
enum search_direction direction = term->search.direction;
const bool backward = term->search.direction == SEARCH_BACKWARD;
term->search.direction = SEARCH_BACKWARD;
if (term->search.len == 0) {
term->search.match = (struct coord){-1, -1};
@ -423,13 +419,49 @@ search_find_next(struct terminal *term)
(len > 0 && start.row >= 0 && start.col >= 0));
if (len == 0) {
if (backward) {
start.row = grid_row_absolute_in_view(grid, term->rows - 1);
start.col = term->cols - 1;
} else {
/* No previous match, start from the top, or bottom, of the scrollback */
switch (direction) {
case SEARCH_FORWARD:
start.row = grid_row_absolute_in_view(grid, 0);
start.col = 0;
break;
case SEARCH_BACKWARD:
case SEARCH_BACKWARD_SAME_POSITION:
start.row = grid_row_absolute_in_view(grid, term->rows - 1);
start.col = term->cols - 1;
break;
}
} else {
/* Continue from last match */
xassert(start.row >= 0);
xassert(start.col >= 0);
switch (direction) {
case SEARCH_BACKWARD_SAME_POSITION:
break;
case SEARCH_BACKWARD:
if (--start.col < 0) {
start.col = term->cols - 1;
start.row += grid->num_rows - 1;
start.row &= grid->num_rows - 1;
}
break;
case SEARCH_FORWARD:
if (++start.col >= term->cols) {
start.col = 0;
start.row++;
start.row &= grid->num_rows - 1;
}
break;
}
xassert(start.row >= 0);
xassert(start.row < grid->num_rows);
xassert(start.col >= 0);
xassert(start.col < term->cols);
}
LOG_DBG(
@ -439,20 +471,26 @@ search_find_next(struct terminal *term)
grid->offset, grid->view);
struct coord end = start;
if (backward) {
/* Search backwards, until we reach the cell *after* current start */
if (++end.col >= term->cols) {
end.col = 0;
end.row++;
}
} else {
switch (direction) {
case SEARCH_FORWARD:
/* Search forward, until we reach the cell *before* current start */
if (--end.col < 0) {
end.col = term->cols - 1;
end.row += grid->num_rows - 1;
end.row &= grid->num_rows - 1;
}
break;
case SEARCH_BACKWARD:
case SEARCH_BACKWARD_SAME_POSITION:
/* Search backwards, until we reach the cell *after* current start */
if (++end.col >= term->cols) {
end.col = 0;
end.row++;
end.row &= grid->num_rows - 1;
}
break;
}
end.row &= grid->num_rows - 1;
struct range match;
bool found = find_next(term, direction, start, end, &match);
@ -748,14 +786,15 @@ from_clipboard_done(void *user)
struct terminal *term = user;
LOG_DBG("search: buffer: %ls", (const wchar_t *)term->search.buf);
search_find_next(term);
search_find_next(term, SEARCH_BACKWARD_SAME_POSITION);
render_refresh_search(term);
}
static bool
execute_binding(struct seat *seat, struct terminal *term,
const struct key_binding *binding, uint32_t serial,
bool *update_search_result, bool *redraw)
bool *update_search_result, enum search_direction *direction,
bool *redraw)
{
*update_search_result = *redraw = false;
const enum bind_action_search action = binding->action;
@ -791,20 +830,7 @@ execute_binding(struct seat *seat, struct terminal *term,
term->search.last.len = 0;
}
else if (term->search.match_len > 0) {
int new_col = term->search.match.col - 1;
int new_row = term->search.match.row;
if (new_col < 0) {
new_col = term->cols - 1;
new_row--;
}
if (new_row >= 0) {
term->search.match.col = new_col;
term->search.match.row = new_row;
}
}
*direction = SEARCH_BACKWARD;
*update_search_result = *redraw = true;
return true;
@ -817,21 +843,7 @@ execute_binding(struct seat *seat, struct terminal *term,
term->search.last.len = 0;
}
else if (term->search.match_len > 0) {
int new_col = term->search.match.col + 1;
int new_row = term->search.match.row;
if (new_col >= term->cols) {
new_col = 0;
new_row++;
}
if (new_row < grid->num_rows) {
term->search.match.col = new_col;
term->search.match.row = new_row;
term->search.direction = SEARCH_FORWARD;
}
}
*direction = SEARCH_FORWARD;
*update_search_result = *redraw = true;
return true;
@ -990,6 +1002,7 @@ search_input(struct seat *seat, struct terminal *term,
? xkb_compose_state_get_status(seat->kbd.xkb_compose_state)
: XKB_COMPOSE_NOTHING;
enum search_direction search_direction = SEARCH_BACKWARD_SAME_POSITION;
bool update_search_result = false;
bool redraw = false;
@ -1002,7 +1015,8 @@ search_input(struct seat *seat, struct terminal *term,
bind->mods == (bind_mods & ~bind_consumed)) {
if (execute_binding(seat, term, bind, serial,
&update_search_result, &redraw))
&update_search_result, &search_direction,
&redraw))
{
goto update_search;
}
@ -1016,7 +1030,8 @@ search_input(struct seat *seat, struct terminal *term,
for (size_t i = 0; i < raw_count; i++) {
if (bind->k.sym == raw_syms[i]) {
if (execute_binding(seat, term, bind, serial,
&update_search_result, &redraw))
&update_search_result, &search_direction,
&redraw))
{
goto update_search;
}
@ -1028,7 +1043,8 @@ search_input(struct seat *seat, struct terminal *term,
tll_foreach(bind->k.key_codes, code) {
if (code->item == key) {
if (execute_binding(seat, term, bind, serial,
&update_search_result, &redraw))
&update_search_result, &search_direction,
&redraw))
{
goto update_search;
}
@ -1052,6 +1068,7 @@ search_input(struct seat *seat, struct terminal *term,
}
update_search_result = redraw = count > 0;
search_direction = SEARCH_BACKWARD_SAME_POSITION;
if (count == 0)
return;
@ -1061,7 +1078,7 @@ search_input(struct seat *seat, struct terminal *term,
update_search:
LOG_DBG("search: buffer: %ls", (const wchar_t *)term->search.buf);
if (update_search_result)
search_find_next(term);
search_find_next(term, search_direction);
if (redraw)
render_refresh_search(term);
}

View file

@ -263,7 +263,7 @@ enum selection_kind {
};
enum selection_direction {SELECTION_UNDIR, SELECTION_LEFT, SELECTION_RIGHT};
enum selection_scroll_direction {SELECTION_SCROLL_NOT, SELECTION_SCROLL_UP, SELECTION_SCROLL_DOWN};
enum search_direction { SEARCH_BACKWARD, SEARCH_FORWARD};
enum search_direction { SEARCH_BACKWARD_SAME_POSITION, SEARCH_BACKWARD, SEARCH_FORWARD };
struct ptmx_buffer {
void *data;
@ -503,7 +503,6 @@ struct terminal {
size_t len;
size_t sz;
size_t cursor;
enum search_direction direction;
int original_view;
bool view_followed_offset;