terminal: fix crash when selected an empty row

Previously we prevented empty rows intentionally for the purpose of
accounting the scroll back buffer's capacity. However, when adding
reflow() the same constraint was not kept, meaning we would end up
with empty rows in the scroll back.

Since empty rows are more memory efficient we should just handle them
properly. Scroll back accounting now treats empty rows as having a
single cell in them. Selection related code paths now handle empty
rows properly.

Author: coletrammer

lib/src/terminal/row_group.cpp

@@ -225,7 +225,7 @@ auto RowGroup::transfer_from(RowGroup& from, usize from_index, usize to_index, u
     auto total_cells = 0_usize;
     for (auto [from_row, to_row] : di::zip(di::View(from_begin, from_end), di::View(to_begin, to_end))) {
         auto cols_to_take = desired_cols.value_or(from_row.cells.size());
-        total_cells += cols_to_take;
+        total_cells += cols_to_take > 0 ? cols_to_take : 1; // Consider empty rows to have 1 cell.
         to_row.cells.resize(cols_to_take);
 
         // Truncate any wide cells if they won't fully fit into the requested column size.
@@ -295,16 +295,16 @@ auto RowGroup::strip_trailing_empty_cells(usize row_index) -> usize {
         return row.cells.size();
     }
 
-    // Ensure all rows have at least size 1, as otherwise our cell based scroll back
-    // limit breaks down (you could have an unbounded number of blank lines). Multi
-    // cells are never empty, so no special handling is needed.
     while (row.cells.size() > 1) {
         if (row.cells.back().value().is_empty()) {
             row.cells.pop_back();
             continue;
         }
         break;
     }
-    return row.cells.size();
+
+    // Ensure all rows have are considered at least size 1, as otherwise our cell based scroll back
+    // limit breaks down (you could have an unbounded number of blank lines).
+    return row.cells.empty() ? 1 : row.cells.size();
 }
 }

lib/src/terminal/screen.cpp

@@ -1460,7 +1460,11 @@ auto Screen::clamp_selection_point(AbsolutePosition const& point) const
     adjusted_point.row = di::clamp(point.row, absolute_row_start(), absolute_row_end() - 1);
     auto [row_index, row_group] = find_row(adjusted_point.row);
     auto const& row = row_group.rows()[row_index];
-    adjusted_point.col = di::clamp(point.col, 0_u32, u32(row.cells.size() - 1));
+    if (row.cells.empty()) {
+        adjusted_point.col = 0;
+    } else {
+        adjusted_point.col = di::clamp(point.col, 0_u32, u32(row.cells.size() - 1));
+    }
 
     // Adjust the selection point to be at the start of any potential multicell.
     while (adjusted_point.col > 0 && row.cells[adjusted_point.col].is_nonprimary_in_multi_cell()) {
@@ -1474,6 +1478,10 @@ void Screen::begin_selection(AbsolutePosition const& point, BeginSelectionMode m
 
     auto [adjusted_point, row_group, row_index] = clamp_selection_point(point);
     auto const& row = row_group.rows()[row_index];
+    if (row.cells.empty()) {
+        m_selection = { adjusted_point, adjusted_point };
+        return;
+    }
     switch (mode) {
         case BeginSelectionMode::Single: {
             m_selection = { adjusted_point, adjusted_point };
@@ -1528,7 +1536,6 @@ void Screen::begin_selection(AbsolutePosition const& point, BeginSelectionMode m
 
 void Screen::update_selection(AbsolutePosition const& point) {
     auto [adjusted_point, row_group, row_index] = clamp_selection_point(point);
-    // auto const& row = row_group.rows()[row_index];
     if (!m_selection) {
         begin_selection(adjusted_point, BeginSelectionMode::Single);
     }
@@ -1585,7 +1592,7 @@ void Screen::invalidate_region(Selection const& region) {
         // Fast path: the entire row is contained the selection.
         auto [row, group] = find_row(r);
         auto const& row_object = group.rows()[row];
-        if (r > start.row && r < end.row) {
+        if ((r > start.row && r < end.row) || row_object.cells.empty()) {
             row_object.stale = false;
             continue;
         }
@@ -1644,6 +1651,12 @@ auto Screen::selected_text(Selection selection) const -> di::String {
             }
             continue;
         }
+        if (row_object.cells.empty()) {
+            if (r != end.row) {
+                text.push_back('\n');
+            }
+            continue;
+        }
 
         // Slow path: iterate over the whole row and add the relevant cells.
         auto iter_start_col = r == start.row ? start.col : 0_usize;

lib/test/src/test_screen.cpp

@@ -381,6 +381,25 @@ static void selection() {
     ASSERT_EQ(screen.selected_text(), u8"ab猫\n猫fghh猫"_sv);
 }
 
+static void selection_empty() {
+    auto screen = Screen({ 2, 5 }, Screen::ScrollBackEnabled::Yes);
+
+    put_text(screen, u8"a\n"_sv
+                     u8"\n"_sv
+                     u8"b\n"_sv
+                     u8"c"_sv);
+
+    screen.begin_selection({ screen.absolute_row_screen_start(), 0 }, Screen::BeginSelectionMode::Word);
+    screen.update_selection({ screen.absolute_row_start() + 1, 4 }); // Out of bounds.
+
+    auto text = screen.selected_text();
+    ASSERT_EQ(text, "\nb"_sv);
+
+    screen.begin_selection({ screen.absolute_row_start() + 1, 4 }, Screen::BeginSelectionMode::Line); // Out of bounds.
+    text = screen.selected_text();
+    ASSERT_EQ(text, ""_sv);
+}
+
 static void put_text_random() {
     auto screen = Screen({ 2, 200 }, Screen::ScrollBackEnabled::Yes);
     auto decoder = ttx::Utf8StreamDecoder {};
@@ -1084,6 +1103,7 @@ TEST(screen, put_text_wide)
 TEST(screen, put_text_damage_tracking)
 TEST(screen, put_text_random)
 TEST(screen, selection)
+TEST(screen, selection_empty)
 TEST(screen, cursor_movement)
 TEST(screen, origin_mode_cursor_movement)
 TEST(screen, clear_row)