From 61a7338f8863a09903d14a1646675483db8c7c97 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 05:12:48 +0000 Subject: [PATCH] Terminal loop, virtual buffers, yank bound - kte refuses to start when stdin is not a terminal: piped bytes were executed as editor commands (C-k s plus a path writes a file), and after EOF the loop spun at full CPU with no way to quit. - The terminal frontend handles all pending input before drawing (a paste used to cost a frame and a full redraw per character) and redraws only after input, a resize or deferred opens, else once a second. The renderer caches, per row and buffer version, where the horizontally scrolled part of a line starts and the cursor's render column, instead of rescanning the line every frame. Idle CPU with the cursor at the end of a 20 MB line: one full core -> ~1%. - Buffers with a virtual name (+HELP+) are not journaled and saving one asks for a file name; it used to write ./+HELP+ and journal under that path. - Yank with a repeat count is capped at 256 MiB in total instead of reserving count x size bytes (C-u 1000000 C-y of a large kill). - Remove an overload clang reports as unused (-Werror on macOS). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- Buffer.cc | 7 +++++ Buffer.h | 10 ++++++++ Command.cc | 22 ++++++++-------- Swap.cc | 4 +-- TerminalFrontend.cc | 38 ++++++++++++++++++++++++---- TerminalFrontend.h | 4 +++ TerminalInputHandler.h | 9 +++++++ TerminalRenderer.cc | 45 ++++++++++++++++++++++++++++++++- TerminalRenderer.h | 29 +++++++++++++++++++++ tests/test_audit_regressions.cc | 34 +++++++++++++++++++++++++ 10 files changed, 184 insertions(+), 18 deletions(-) diff --git a/Buffer.cc b/Buffer.cc index ba12d10..879a935 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -322,6 +322,7 @@ Buffer::Buffer(const Buffer &other) rows_cache_dirty_ = other.rows_cache_dirty_; filename_ = other.filename_; is_file_backed_ = other.is_file_backed_; + is_virtual_ = other.is_virtual_; dirty_ = other.dirty_; read_only_ = other.read_only_; mark_set_ = other.mark_set_; @@ -375,6 +376,7 @@ Buffer::operator=(const Buffer &other) rows_cache_dirty_ = other.rows_cache_dirty_; filename_ = other.filename_; is_file_backed_ = other.is_file_backed_; + is_virtual_ = other.is_virtual_; dirty_ = other.dirty_; read_only_ = other.read_only_; mark_set_ = other.mark_set_; @@ -420,6 +422,7 @@ Buffer::Buffer(Buffer &&other) noexcept rows_(std::move(other.rows_)), filename_(std::move(other.filename_)), is_file_backed_(other.is_file_backed_), + is_virtual_(other.is_virtual_), dirty_(other.dirty_), read_only_(other.read_only_), mark_set_(other.mark_set_), @@ -470,6 +473,7 @@ Buffer::operator=(Buffer &&other) noexcept rows_ = std::move(other.rows_); filename_ = std::move(other.filename_); is_file_backed_ = other.is_file_backed_; + is_virtual_ = other.is_virtual_; dirty_ = other.dirty_; read_only_ = other.read_only_; mark_set_ = other.mark_set_; @@ -548,6 +552,7 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) nrows_ = 0; filename_ = norm; is_file_backed_ = false; + is_virtual_ = false; dirty_ = false; // Reset cursor/viewport state @@ -620,6 +625,7 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) nrows_ = 0; // not used under PieceTable filename_ = norm; is_file_backed_ = true; + is_virtual_ = false; dirty_ = false; RefreshOnDiskIdentity(); @@ -702,6 +708,7 @@ Buffer::SaveAs(const std::string &path, std::string &err) filename_ = out_path; is_file_backed_ = true; + is_virtual_ = false; dirty_ = false; RefreshOnDiskIdentity(); return true; diff --git a/Buffer.h b/Buffer.h index ba33ce2..50394a9 100644 --- a/Buffer.h +++ b/Buffer.h @@ -334,6 +334,7 @@ public: { filename_ = name; is_file_backed_ = false; + is_virtual_ = true; } @@ -343,6 +344,14 @@ public: } + // A virtual buffer (e.g. "+HELP+") has a display name that is not a path: + // it is never journaled, and saving it asks for a file name. + [[nodiscard]] bool IsVirtual() const + { + return is_virtual_; + } + + [[nodiscard]] bool Dirty() const { return dirty_; @@ -677,6 +686,7 @@ private: std::string filename_; bool is_file_backed_ = false; + bool is_virtual_ = false; bool dirty_ = false; bool read_only_ = false; bool mark_set_ = false; diff --git a/Command.cc b/Command.cc index 69fbe42..db76b18 100644 --- a/Command.cc +++ b/Command.cc @@ -108,13 +108,6 @@ utf8_next_len(std::string_view line, std::size_t x) } -static inline std::string_view -line_view(const Buffer::Line &l) -{ - return std::string_view(l.Data(), l.Size()); -} - - static inline std::string_view line_view(const std::string &l) { @@ -937,7 +930,8 @@ cmd_save(CommandContext &ctx) // Allow saving directly to a filename if buffer was opened with a // non-existent path (not yet file-backed but has a filename). if (!buf->IsFileBacked()) { - if (!buf->Filename().empty()) { + // A virtual buffer's name (e.g. +HELP+) is not a path: ask for one. + if (!buf->Filename().empty() && !buf->IsVirtual()) { // If first-time save to an existing path, confirm overwrite if (fs_exists(buf->Filename())) { ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", ""); @@ -1130,7 +1124,7 @@ cmd_save_and_quit(CommandContext &ctx) ctx.editor.SetStatus(err); return false; } - } else if (!buf->Filename().empty()) { + } else if (!buf->Filename().empty() && !buf->IsVirtual()) { if (buf->SaveAs(buf->Filename(), err)) { buf->SetDirty(false); if (auto *sm = ctx.editor.Swap()) @@ -3162,7 +3156,7 @@ cmd_newline(CommandContext &ctx) if (auto *u = buf->Undo()) u->mark_saved(); } - } else if (!buf->Filename().empty()) { + } else if (!buf->Filename().empty() && !buf->IsVirtual()) { if (!buf->SaveAs(buf->Filename(), err)) { ctx.editor.SetStatus(err); proceed_to_close = false; @@ -3974,6 +3968,14 @@ cmd_yank(CommandContext &ctx) } ensure_at_least_one_line(*buf); int repeat = ctx.count > 0 ? ctx.count : 1; + // Bound the total: C-u 1000000 C-y of a large kill would allocate + // gigabytes (and bad_alloc or the OOM killer end the session). + constexpr std::size_t kMaxYankBytes = std::size_t{256} << 20; + if (text.size() * static_cast(repeat) > kMaxYankBytes) { + ctx.editor.SetStatus("Yank too large (" + std::to_string(repeat) + " x " + std::to_string(text.size()) + + " bytes)"); + return false; + } std::string ins; if (repeat == 1) { ins = text; diff --git a/Swap.cc b/Swap.cc index 9d5a88a..ae097c8 100644 --- a/Swap.cc +++ b/Swap.cc @@ -346,8 +346,8 @@ SwapManager::Rehome(Buffer *old_addr, Buffer *new_addr) void SwapManager::Attach(Buffer *buf) { - if (!buf) - return; + if (!buf || buf->IsVirtual()) + return; // e.g. +HELP+: nothing to recover, and its name is not a path std::lock_guard lg(mtx_); const bool fresh = journals_.find(buf) == journals_.end(); JournalCtx &ctx = journals_[buf]; diff --git a/TerminalFrontend.cc b/TerminalFrontend.cc index 94ac0c8..1e8439e 100644 --- a/TerminalFrontend.cc +++ b/TerminalFrontend.cc @@ -1,3 +1,4 @@ +#include #include #include #include @@ -12,6 +13,12 @@ TerminalFrontend::Init(int &argc, char **argv, Editor &ed) { (void) argc; (void) argv; + // Keys are read from stdin. Piped input would be executed as editor + // commands (and after EOF the loop spun at full CPU with no way to quit). + if (!isatty(STDIN_FILENO)) { + std::fprintf(stderr, "kte: standard input is not a terminal\n"); + return false; + } // Ensure Control keys reach the app: disable XON/XOFF and dsusp/susp bindings (e.g., ^S/^Q, ^Y on macOS) { struct termios tio{}; @@ -88,29 +95,50 @@ TerminalFrontend::Step(Editor &ed, bool &running) // Handle resize and keep editor dimensions synced int r, c; getmaxyx(stdscr, r, c); + bool changed = first_frame_; if (r != prev_r_ || c != prev_c_) { resizeterm(r, c); clear(); prev_r_ = r; prev_c_ = c; + changed = true; } ed.SetDimensions(static_cast(r), static_cast(c)); // Allow deferred opens (including swap recovery prompts) to run. - ed.ProcessPendingOpens(); + if (ed.ProcessPendingOpens()) + changed = true; + // Handle all pending input before drawing: one key per frame made a + // pasted block take a frame (and a full redraw) per character. The first + // read waits up to the configured timeout; the rest do not wait. MappedInput mi; - if (input_.Poll(mi)) { - if (mi.hasCommand) { + int handled = 0; + while (handled < 4096 && input_.PollKey(mi)) { + changed = true; + if (mi.hasCommand) Execute(ed, mi.id, mi.arg, mi.count); - } + if (ed.QuitRequested()) + break; + if (++handled == 1) + timeout(0); } + if (handled > 0) + timeout(16); if (ed.QuitRequested()) { running = false; } - renderer_.Draw(ed); + // Redraw after input or a resize, and otherwise about once a second + // (status messages, deferred work): redrawing every 16 ms kept a core + // busy on large or long-line files while idle. + const auto now = std::chrono::steady_clock::now(); + if (changed || now - last_draw_ >= std::chrono::milliseconds(1000)) { + renderer_.Draw(ed); + last_draw_ = now; + first_frame_ = false; + } } diff --git a/TerminalFrontend.h b/TerminalFrontend.h index 3897b08..90a7e98 100644 --- a/TerminalFrontend.h +++ b/TerminalFrontend.h @@ -2,6 +2,7 @@ * TerminalFrontend - couples TerminalInputHandler + TerminalRenderer and owns ncurses lifecycle */ #pragma once +#include #include #include @@ -32,6 +33,9 @@ private: TerminalRenderer renderer_{}; int prev_r_ = 0; int prev_c_ = 0; + // Redraw only when something may have changed (see Step). + bool first_frame_ = true; + std::chrono::steady_clock::time_point last_draw_{}; // Saved terminal attributes to restore on shutdown bool have_orig_tio_ = false; struct termios orig_tio_{}; diff --git a/TerminalInputHandler.h b/TerminalInputHandler.h index b7dfc16..9928a15 100644 --- a/TerminalInputHandler.h +++ b/TerminalInputHandler.h @@ -20,6 +20,15 @@ public: bool Poll(MappedInput &out) override; + // Read one key if available. Returns false when no input was read; true + // when a key was consumed (out.hasCommand says whether it produced a + // command: prefixes such as C-k or ESC do not). + bool PollKey(MappedInput &out) + { + out = {}; + return decode_(out); + } + private: bool decode_(MappedInput &out); diff --git a/TerminalRenderer.cc b/TerminalRenderer.cc index f06657f..c80eae8 100644 --- a/TerminalRenderer.cc +++ b/TerminalRenderer.cc @@ -241,6 +241,45 @@ TerminalRenderer::Draw(Editor &ed) return 0; } }; + // Fast-forward to the scroll offset with the draw loop's own skip + // rules (a tab is skipped only if it ends at or before coloffs, + // any other character while it starts before coloffs), cached + // for the row's current content. + if (coloffs > 0 && src_i == 0) { + if (skip_cache_.size() < static_cast(content_rows)) + skip_cache_.resize(static_cast(content_rows)); + SkipCache &sc = skip_cache_[static_cast(r)]; + if (!(sc.valid && sc.buf == buf && sc.version == buf->Version() && sc.row == li && + sc.coloffs == coloffs)) { + std::size_t s = 0, c = 0; + while (s < line.size()) { + std::mbstate_t st = std::mbstate_t(); + wchar_t w = 0; + std::size_t n = std::mbrtowc(&w, &line[s], line.size() - s, &st); + if (n == (size_t) -1 || n == (size_t) -2) { + w = static_cast(line[s]); + n = 1; + } else if (n == 0) { + w = L'\0'; + n = 1; + } + if (w == L'\t') { + const std::size_t next_tab = tabw - (c % tabw); + if (c + next_tab > coloffs) + break; + c += next_tab; + } else { + if (c >= coloffs) + break; + c += static_cast(kte::CellWidth(w)); + } + s += n; + } + sc = SkipCache{buf, buf->Version(), li, coloffs, s, c, true}; + } + src_i = sc.src; + render_col = sc.col; + } while (written < cols) { bool from_src = false; wchar_t wch = L' '; @@ -391,7 +430,10 @@ TerminalRenderer::Draw(Editor &ed) std::size_t cx = buf->Curx(); int cur_y = static_cast(cy) - static_cast(buf->Rowoffs()); std::size_t rx_recomputed = 0; - if (cy < nlines) { + if (cy < nlines && cursor_cache_.valid && cursor_cache_.buf == buf && + cursor_cache_.version == buf->Version() && cursor_cache_.row == cy && cursor_cache_.cx == cx) { + rx_recomputed = cursor_cache_.rx; + } else if (cy < nlines) { const std::string line_for_cursor = buf->GetLineString(cy); std::size_t src_i_cur = 0; std::size_t render_col_cur = 0; @@ -420,6 +462,7 @@ TerminalRenderer::Draw(Editor &ed) src_i_cur += len; } rx_recomputed = render_col_cur; + cursor_cache_ = CursorCache{buf, buf->Version(), cy, cx, rx_recomputed, true}; } int cur_x = static_cast(rx_recomputed) - static_cast(buf->Coloffs()); if (cur_y >= 0 && cur_y < content_rows && cur_x >= 0 && cur_x < cols) { diff --git a/TerminalRenderer.h b/TerminalRenderer.h index 73d164e..c1fb3cf 100644 --- a/TerminalRenderer.h +++ b/TerminalRenderer.h @@ -2,6 +2,9 @@ * TerminalRenderer - ncurses-based renderer for terminal mode */ #pragma once +#include +#include +#include #include "Renderer.h" @@ -12,4 +15,30 @@ public: ~TerminalRenderer() override; void Draw(Editor &ed) override; + +private: + // Where a row's visible part starts (the draw loop's skip up to the + // horizontal scroll offset), cached per screen row. Scanning a very long + // line from column 0 on every frame made each redraw O(line length). + struct SkipCache { + const void *buf = nullptr; + std::uint64_t version = 0; + std::size_t row = 0; + std::size_t coloffs = 0; + std::size_t src = 0; // byte offset where drawing resumes + std::size_t col = 0; // render column at that offset + bool valid = false; + }; + + std::vector skip_cache_; + + // Render column of the cursor, cached the same way. + struct CursorCache { + const void *buf = nullptr; + std::uint64_t version = 0; + std::size_t row = 0; + std::size_t cx = 0; + std::size_t rx = 0; + bool valid = false; + } cursor_cache_; }; diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index bf736ba..50cb211 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -9,6 +9,7 @@ #include "ErrorRecovery.h" #include "PieceTable.h" #include "RegexGuard.h" +#include "Swap.h" #include "UndoTree.h" #include "syntax/HighlighterEngine.h" #include "syntax/HighlighterRegistry.h" @@ -992,3 +993,36 @@ TEST(Audit_Save_KeepsSpecialModeBits) ASSERT_EQ(::stat(f.c_str(), &st), 0); ASSERT_EQ(st.st_mode & 07777, (mode_t) 06755); } + + +// The help buffer's name is not a path: it is not journaled, and saving it +// asks for a file name instead of writing ./+HELP+. +TEST(Audit_HelpBuffer_NotJournaledOrSavedByName) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + ASSERT_TRUE(h.Exec(CommandId::ShowHelp)); + Buffer *help = ed.CurrentBuffer(); + ASSERT_TRUE(help->IsVirtual()); + ASSERT_TRUE(h.Exec(CommandId::ToggleReadOnly)); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "x")); + ed.Swap()->Flush(help); + ASSERT_TRUE(!std::filesystem::exists(kte::SwapManager::ComputeSwapPathForTests(*help))); + ASSERT_TRUE(h.Exec(CommandId::Save)); + ASSERT_TRUE(ed.PromptActive()); + ASSERT_TRUE(ed.CurrentPromptKind() == Editor::PromptKind::SaveAs); + ASSERT_TRUE(!std::filesystem::exists("+HELP+")); + ASSERT_TRUE(h.Exec(CommandId::Refresh)); +} + + +// A huge repeated yank is refused rather than allocating without bound. +TEST(Audit_Yank_HugeRepeatRefused) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + ed.KillRingPush(std::string(1 << 20, 'k')); + ASSERT_TRUE(!h.Exec(CommandId::Yank, std::string(), 1000000)); + ASSERT_EQ(h.Buf().Nrows(), (std::size_t) 1); + ASSERT_EQ(h.Line(0), std::string()); +}