From 3be85ef4adc63058a2d19a644d365b784c50c652 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 19:02:16 +0000 Subject: [PATCH] Review fixes: exception-safe AdoptOriginal; Qt draws cells straddling the left edge AdoptOriginal cleared the table before allocating its piece list, so a bad_alloc left a live buffer with the file's size and no pieces; OpenFromFile no longer caught it either. The piece list is now built before anything is touched (the rest cannot throw) and OpenFromFile again reports "File too large to load" with the buffer unchanged. Qt skipped every character starting left of the horizontal scroll offset, so a tab or wide character straddling it left blank columns while selection and cursor still used the real ones. Such a character is now drawn at its own column and clipped by the viewport. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- Buffer.cc | 15 ++++++++++----- PieceTable.cc | 8 ++++++-- PieceTable.h | 1 + QtFrontend.cc | 15 +++++++++++++-- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Buffer.cc b/Buffer.cc index 9ba4aa3..d72bb60 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -617,11 +617,16 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) return false; } (void) kte::syscall::Close(fd); - // Build the new content aside and swap it in only once complete: if the - // copy runs out of memory, the buffer keeps its old text (clearing first - // left an empty buffer that a later save wrote over the file). - // The bytes become the piece table's original storage, uncopied. - content_.AdoptOriginal(std::move(data)); // not used under PieceTable + // The bytes become the piece table's original storage, uncopied. If that + // runs out of memory the buffer keeps its old text (clearing first left + // an empty buffer that a later save wrote over the file). + try { + content_.AdoptOriginal(std::move(data)); + } catch (const std::bad_alloc &) { + err = "File too large to load: " + norm; + kte::ErrorHandler::Instance().Error("Buffer", err, norm); + return false; + } filename_ = norm; is_file_backed_ = true; is_virtual_ = false; diff --git a/PieceTable.cc b/PieceTable.cc index b550bad..bf220a9 100644 --- a/PieceTable.cc +++ b/PieceTable.cc @@ -206,13 +206,17 @@ PieceTable::Clear() void PieceTable::AdoptOriginal(std::string &&bytes) { + // Allocate before touching the current content: if this throws, the + // table is unchanged (everything below is non-throwing). + std::vector pieces; + if (!bytes.empty()) + pieces.push_back(Piece{Source::Original, 0, bytes.size()}); Clear(); std::string().swap(add_); // release a previous content's capacity std::string().swap(materialized_); original_ = std::move(bytes); total_size_ = original_.size(); - if (total_size_ > 0) - pieces_.push_back(Piece{Source::Original, 0, total_size_}); + pieces_.swap(pieces); version_++; } diff --git a/PieceTable.h b/PieceTable.h index 3033f48..7b9eb0e 100644 --- a/PieceTable.h +++ b/PieceTable.h @@ -86,6 +86,7 @@ public: // Replace the content with `bytes`, taken over without copying (a loaded // file). Undo spans into the old storage are invalidated, as by Clear(). + // If it throws (std::bad_alloc), the content is unchanged. void AdoptOriginal(std::string &&bytes); // Accessors diff --git a/QtFrontend.cc b/QtFrontend.cc index 5915272..e9ff502 100644 --- a/QtFrontend.cc +++ b/QtFrontend.cc @@ -204,10 +204,21 @@ protected: p.drawText(QPointF(col_x(col), baseline), QString::fromUtf8(d, static_cast(n))); }; auto draw_seg = [&](std::size_t s, std::size_t e, const QColor &color) { - // Only the visible columns: characters left of coloffs or - // right of the viewport are skipped. + // Only the visible columns: characters that end at or + // before coloffs, or start right of the viewport, are + // skipped. A tab or wide character straddling coloffs + // starts left of it; it is drawn at its own column and + // the clip cuts its left part. + const std::size_t first = s; s = static_cast(std::lower_bound(col_of.begin() + s, col_of.begin() + e, coloffs) - col_of.begin()); + if (s > first && col_of[s] > coloffs) { + // The character before s ends past coloffs: back up to + // its first byte. + --s; + while (s > first && (static_cast(line[s]) & 0xC0) == 0x80) + --s; + } e = static_cast(std::lower_bound(col_of.begin() + s, col_of.begin() + e, coloffs + vis_cols) - col_of.begin()); if (e <= s)