From 86663522d5915c0fdb2b8b132bacc247fcfd1fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 17 Aug 2022 17:36:10 +0200 Subject: [PATCH] selection: never highlight selected, empty cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a regression, where empty cells "between" non-empty cells (i.e. non-trailing empty cells) sometimes were incorrectly highlighted. The idea has always been to highlight exactly those cells that will get extracted when they’re copied. This means we’ve not highlighted trailing empty cells, but we _have_ highlighted other empty cells, since they are converted to spaces when copied (whereas trailing empty cells are skipped). fa2d9f86996467ba33cc381f810ea966a4323381 changed how a selection is updated. That is, which cells gets marked as selected, and which ones gets unmarked. Since we no longer walk all the cells, but instead work with pixman regions representing selection diffs, we can no longer determine (with certainty) which empty cells should be selected and which shouldn’t. Before this patch (but after fa2d9f86996467ba33cc381f810ea966a4323381), we sometimes incorrectly highlighted empty cells that should not have been highlighted. This happened when we’ve first (correctly) highlighted a region of empty cells, but then shrink the selection such that all those empty cells should be de-selected. This patch changes the selection behavior to *never* highlight empty cells. This fixes the regression, but also means slightly different behavior, compared to pre-fa2d9f86996467ba33cc381f810ea966a4323381. The other alternative is to always highlight all empty cells. But, since I personally like the fact that we’re skipping trailing empty cells, I prefer the approach taken by this patch. --- CHANGELOG.md | 4 ++++ selection.c | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0bf2624..8e95c9df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,8 @@ ### Changed * Window is now dimmed while in Unicode input mode. +* Selected empty cells are **never** highlighted as being + selected. They used to be, when followed by non-empty cells. ### Deprecated @@ -61,6 +63,8 @@ * Crash on buggy compositors (GNOME) that sometimes send pointer-enter events with a NULL surface. Foot now ignores these events, and the subsequent motion and leave events. +* Regression: “random” selected empty cells being highlighted as + selected when they should not. ### Security diff --git a/selection.c b/selection.c index 8c3a8357..2fd62f8a 100644 --- a/selection.c +++ b/selection.c @@ -737,8 +737,27 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes, row->dirty = true; for (int c = box->x1, empty_count = 0; c < box->x2; c++) { - if (selected && row->cells[c].wc == 0) { - empty_count++; + if (row->cells[c].wc == 0) { + /* + * We used to highlight empty cells *if* they were + * followed by non-empty cell(s), since this + * corresponds to what gets extracted when the + * selection is copied (that is, empty cells + * “between” non-empty cells are converted to + * spaces). + * + * However, they way we handle selection updates + * (diffing the “old” selection area against the + * “new” one, using pixman regions), means we + * can’t correctly update the state of empty + * cells. The result is “random” empty cells being + * rendered as selected when they shouldn’t. + * + * “Fix” by *never* highlighting selected empty + * cells (they still get converted to spaces when + * copied, if followed by non-empty cells). + */ + /* empty_count++; */ continue; }