Merge branch 'osc8-overwrite-2'

Closes #801
Closes #804

CHANGELOG.md

@@ -53,6 +53,8 @@
 * Regression: `pipe-*` key bindings not being parsed correctly,
   resulting in invalid error messages
   (https://codeberg.org/dnkl/foot/issues/809).
+* OSC-8 data not being cleared when cell is overwritten
+  (https://codeberg.org/dnkl/foot/issues/804).
 
 
 ### Security

grid.c

@@ -236,7 +236,7 @@ grid_resize_without_reflow(
                 .id = it->item.id,
                 .uri = xstrdup(it->item.uri),
             };
-            grid_row_add_uri_range(new_row, range);
+            grid_row_uri_range_add(new_row, range);
         }
     }
 
@@ -303,7 +303,7 @@ reflow_uri_range_start(struct row_uri_range *range, struct row *new_row,
         .uri = range->uri,
     };
     range->uri = NULL;
-    grid_row_add_uri_range(new_row, new_range);
+    grid_row_uri_range_add(new_row, new_range);
 }
 
 static void
@@ -366,7 +366,7 @@ _line_wrap(struct grid *old_grid, struct row **new_grid, struct row *row,
                 .id = range->id,
                 .uri = xstrdup(range->uri),
             };
-            grid_row_add_uri_range(new_row, new_range);
+            grid_row_uri_range_add(new_row, new_range);
         }
     }
 
@@ -845,14 +845,148 @@ ensure_row_has_extra_data(struct row *row)
 }
 
 void
-grid_row_add_uri_range(struct row *row, struct row_uri_range range)
+grid_row_uri_range_add(struct row *row, struct row_uri_range range)
 {
     ensure_row_has_extra_data(row);
     tll_rforeach(row->extra->uri_ranges, it) {
         if (it->item.end < range.start) {
             tll_insert_after(row->extra->uri_ranges, it, range);
-            return;
+            goto out;
         }
     }
+
     tll_push_front(row->extra->uri_ranges, range);
+
+out:
+    ;
+#if defined(_DEBUG)
+        tll_foreach(row->extra->uri_ranges, it1) {
+            tll_foreach(row->extra->uri_ranges, it2) {
+                if (&it1->item == &it2->item)
+                    continue;
+
+                xassert(it1->item.start != it2->item.start);
+                xassert(it1->item.start != it2->item.end);
+                xassert(it1->item.end != it2->item.start);
+                xassert(it1->item.end != it2->item.end);
+            }
+        }
+#endif
+}
+
+void
+grid_row_uri_range_erase(struct row *row, int start, int end)
+{
+    xassert(row->extra != NULL);
+    xassert(start <= end);
+
+    /* Split up, or remove, URI ranges affected by the erase */
+    tll_foreach(row->extra->uri_ranges, it) {
+        struct row_uri_range *old = &it->item;
+
+        if (old->end < start)
+            continue;
+
+        if (old->start > end)
+            return;
+
+        if (start <= old->start && end >= old->end) {
+            /* Erase range covers URI completely - remove it */
+            grid_row_uri_range_destroy(old);
+            tll_remove(row->extra->uri_ranges, it);
+        }
+
+        else if (start > old->start && end < old->end) {
+            /* Erase range erases a part in the middle of the URI */
+            struct row_uri_range old_tail = {
+                .start = end + 1,
+                .end = old->end,
+                .id = old->id,
+                .uri = old->uri != NULL ? xstrdup(old->uri) : NULL,
+            };
+            tll_insert_after(row->extra->uri_ranges, it, old_tail);
+            old->end = start - 1;
+            return;  /* There can be no more URIs affected by the erase range */
+        }
+
+        else if (start <= old->start && end >= old->start) {
+            /* Erase range erases the head of the URI */
+            xassert(start <= old->start);
+            old->start = end + 1;
+            return;  /* There can be no more overlapping URIs */
+        }
+
+        else if (start <= old->end && end >= old->end) {
+            /* Erase range erases the tail of the URI */
+            xassert(end >= old->end);
+            old->end = start - 1;
+        }
+    }
+}
+
+UNITTEST
+{
+    struct row_data row_data = {.uri_ranges = tll_init()};
+    struct row row = {.extra = &row_data};
+
+#define row_has_no_overlapping_uris(row)                                \
+    do {                                                                \
+        tll_foreach((row)->extra->uri_ranges, it1) {                    \
+            tll_foreach((row)->extra->uri_ranges, it2) {                \
+                if (&it1->item == &it2->item)                           \
+                    continue;                                           \
+                xassert(it1->item.start != it2->item.start);            \
+                xassert(it1->item.start != it2->item.end);              \
+                xassert(it1->item.end != it2->item.start);              \
+                xassert(it1->item.end != it2->item.end);                \
+            }                                                           \
+        }                                                               \
+    } while (0)
+
+    grid_row_uri_range_add(&row, (struct row_uri_range){1, 10});
+    xassert(tll_length(row_data.uri_ranges) == 1);
+    xassert(tll_front(row_data.uri_ranges).start == 1);
+    xassert(tll_front(row_data.uri_ranges).end == 10);
+    row_has_no_overlapping_uris(&row);
+
+    grid_row_uri_range_add(&row, (struct row_uri_range){11, 20});
+    xassert(tll_length(row_data.uri_ranges) == 2);
+    xassert(tll_back(row_data.uri_ranges).start == 11);
+    xassert(tll_back(row_data.uri_ranges).end == 20);
+    row_has_no_overlapping_uris(&row);
+
+    /* Erase both URis */
+    grid_row_uri_range_erase(&row, 1, 20);
+    xassert(tll_length(row_data.uri_ranges) == 0);
+    row_has_no_overlapping_uris(&row);
+
+    /* Two URIs, then erase second half of the first, first half of
+       the second */
+    grid_row_uri_range_add(&row, (struct row_uri_range){1, 10});
+    grid_row_uri_range_add(&row, (struct row_uri_range){11, 20});
+    grid_row_uri_range_erase(&row, 5, 15);
+    xassert(tll_length(row_data.uri_ranges) == 2);
+    xassert(tll_front(row_data.uri_ranges).start == 1);
+    xassert(tll_front(row_data.uri_ranges).end == 4);
+    xassert(tll_back(row_data.uri_ranges).start == 16);
+    xassert(tll_back(row_data.uri_ranges).end == 20);
+    row_has_no_overlapping_uris(&row);
+
+    tll_pop_back(row_data.uri_ranges);
+    tll_pop_back(row_data.uri_ranges);
+    xassert(tll_length(row_data.uri_ranges) == 0);
+
+    /* One URI, erase middle part of it */
+    grid_row_uri_range_add(&row, (struct row_uri_range){1, 10});
+    grid_row_uri_range_erase(&row, 5, 6);
+    xassert(tll_length(row_data.uri_ranges) == 2);
+    xassert(tll_front(row_data.uri_ranges).start == 1);
+    xassert(tll_front(row_data.uri_ranges).end == 4);
+    xassert(tll_back(row_data.uri_ranges).start == 7);
+    xassert(tll_back(row_data.uri_ranges).end == 10);
+    row_has_no_overlapping_uris(&row);
+
+#undef row_has_no_overlapping_uris
+
+    tll_free(row_data.uri_ranges);
 }

grid.h

@@ -74,7 +74,8 @@ grid_row_in_view(struct grid *grid, int row_no)
     return row;
 }
 
-void grid_row_add_uri_range(struct row *row, struct row_uri_range range);
+void grid_row_uri_range_add(struct row *row, struct row_uri_range range);
+void grid_row_uri_range_erase(struct row *row, int start, int end);
 
 static inline void
 grid_row_uri_range_destroy(struct row_uri_range *range)

terminal.c

@@ -1781,63 +1781,8 @@ erase_cell_range(struct terminal *term, struct row *row, int start, int end)
     } else
         memset(&row->cells[start], 0, (end - start + 1) * sizeof(row->cells[0]));
 
-    if (likely(row->extra == NULL))
+    if (unlikely(row->extra != NULL))
-        return;
+        grid_row_uri_range_erase(row, start, end);
-
-    /* Split up, or remove, URI ranges affected by the erase */
-    tll_foreach(row->extra->uri_ranges, it) {
-        if (it->item.start > end) {
-            /* This range, and all subsequent ranges, start *after*
-             * the erase range */
-            break;
-        }
-
-        if (it->item.start < start && it->item.end >= start) {
-            /*
-             * URI crosses the erase *start* point.
-             *
-             * Create a new range for the URI part *before* the erased
-             * cells.
-             *
-             * Also modify this URI range’s start point so that we can
-             * remove it below.
-             */
-            struct row_uri_range range_before = {
-                .start = it->item.start,
-                .end = start - 1,
-                .id = it->item.id,
-                .uri = xstrdup(it->item.uri),
-            };
-            tll_insert_before(row->extra->uri_ranges, it, range_before);
-            it->item.start = start;
-        }
-
-        if (it->item.start <= end && it->item.end > end) {
-            /*
-             * URI crosses the erase *end* point.
-             *
-             * Create a new range for the URI part *after* the erased
-             * cells.
-             *
-             * Also modify the URI range’s end point so that we can
-             * remove it below.
-             */
-            struct row_uri_range range_after = {
-                .start = end + 1,
-                .end = it->item.end,
-                .id = it->item.id,
-                .uri = xstrdup(it->item.uri),
-            };
-            tll_insert_before(row->extra->uri_ranges, it, range_after);
-            it->item.end = end;
-        }
-
-        if (it->item.start >= start && it->item.end <= end) {
-            /* URI range completey covered by the erase - remove it */
-            free(it->item.uri);
-            tll_remove(row->extra->uri_ranges, it);
-        }
-    }
 }
 
 static inline void
@@ -3218,6 +3163,8 @@ term_print(struct terminal *term, wchar_t wc, int width)
 {
     xassert(width > 0);
 
+    struct grid *grid = term->grid;
+
     if (unlikely(term->charsets.set[term->charsets.selected] == CHARSET_GRAPHIC) &&
         wc >= 0x60 && wc <= 0x7e)
     {
@@ -3236,43 +3183,52 @@ term_print(struct terminal *term, wchar_t wc, int width)
     print_linewrap(term);
     print_insert(term, width);
 
+    int col = grid->cursor.point.col;
+
     if (unlikely(width > 1) && likely(term->auto_margin) &&
-        term->grid->cursor.point.col + width > term->cols)
+        col + width > term->cols)
     {
         /* Multi-column character that doesn't fit on current line -
          * pad with spacers */
-        for (size_t i = term->grid->cursor.point.col; i < term->cols; i++)
+        for (size_t i = col; i < term->cols; i++)
             print_spacer(term, i, 0);
 
         /* And force a line-wrap */
-        term->grid->cursor.lcf = 1;
+        grid->cursor.lcf = 1;
         print_linewrap(term);
+        col = 0;
     }
 
     sixel_overwrite_at_cursor(term, width);
 
     /* *Must* get current cell *after* linewrap+insert */
-    struct row *row = term->grid->cur_row;
+    struct row *row = grid->cur_row;
-    struct cell *cell = &row->cells[term->grid->cursor.point.col];
-
-    cell->wc = term->vt.last_printed = wc;
-    cell->attrs = term->vt.attrs;
-
     row->dirty = true;
     row->linebreak = true;
 
+    struct cell *cell = &row->cells[col];
+    cell->wc = term->vt.last_printed = wc;
+    cell->attrs = term->vt.attrs;
+
+    const int uri_start = col;
+
     /* Advance cursor the 'additional' columns while dirty:ing the cells */
-    for (int i = 1; i < width && term->grid->cursor.point.col < term->cols - 1; i++) {
+    for (int i = 1; i < width && col < term->cols - 1; i++) {
-        term->grid->cursor.point.col++;
+        col++;
-        print_spacer(term, term->grid->cursor.point.col, width - i);
+        print_spacer(term, col, width - i);
     }
 
     /* Advance cursor */
-    if (unlikely(++term->grid->cursor.point.col >= term->cols)) {
+    if (unlikely(++col >= term->cols)) {
-        term->grid->cursor.lcf = true;
+        grid->cursor.lcf = true;
-        term->grid->cursor.point.col--;
+        col--;
     } else
-        xassert(!term->grid->cursor.lcf);
+        xassert(!grid->cursor.lcf);
+
+    grid->cursor.point.col = col;
+
+    if (unlikely(row->extra != NULL))
+        grid_row_uri_range_erase(row, uri_start, uri_start + width - 1);
 }
 
 static void
@@ -3284,28 +3240,38 @@ ascii_printer_generic(struct terminal *term, wchar_t wc)
 static void
 ascii_printer_fast(struct terminal *term, wchar_t wc)
 {
+    struct grid *grid = term->grid;
+
     xassert(term->charsets.set[term->charsets.selected] == CHARSET_ASCII);
     xassert(!term->insert_mode);
-    xassert(tll_length(term->grid->sixel_images) == 0);
+    xassert(tll_length(grid->sixel_images) == 0);
 
     print_linewrap(term);
 
     /* *Must* get current cell *after* linewrap+insert */
-    struct row *row = term->grid->cur_row;
+    int col = grid->cursor.point.col;
-    struct cell *cell = &row->cells[term->grid->cursor.point.col];
+    const int uri_start = col;
-
-    cell->wc = term->vt.last_printed = wc;
-    cell->attrs = term->vt.attrs;
 
+    struct row *row = grid->cur_row;
     row->dirty = true;
     row->linebreak = true;
 
+    struct cell *cell = &row->cells[col];
+    cell->wc = term->vt.last_printed = wc;
+    cell->attrs = term->vt.attrs;
+
+
     /* Advance cursor */
-    if (unlikely(++term->grid->cursor.point.col >= term->cols)) {
+    if (unlikely(++col >= term->cols)) {
-        term->grid->cursor.lcf = true;
+        grid->cursor.lcf = true;
-        term->grid->cursor.point.col--;
+        col--;
     } else
-        xassert(!term->grid->cursor.lcf);
+        xassert(!grid->cursor.lcf);
+
+    grid->cursor.point.col = col;
+
+    if (unlikely(row->extra != NULL))
+        grid_row_uri_range_erase(row, uri_start, uri_start);
 }
 
 static void
@@ -3590,21 +3556,7 @@ term_osc8_close(struct terminal *term)
             .id = term->vt.osc8.id,
             .uri = xstrdup(term->vt.osc8.uri),
         };
-        grid_row_add_uri_range(row, range);
+        grid_row_uri_range_add(row, range);
-
-#if defined(_DEBUG)
-        tll_foreach(row->extra->uri_ranges, it1) {
-            tll_foreach(row->extra->uri_ranges, it2) {
-                if (&it1->item == &it2->item)
-                    continue;
-
-                xassert(it1->item.start != it2->item.start);
-                xassert(it1->item.start != it2->item.end);
-                xassert(it1->item.end != it2->item.start);
-                xassert(it1->item.end != it2->item.end);
-            }
-        }
-#endif
         start_col = 0;
 
         if (r == end.row)