diff --git a/Buffer.cc b/Buffer.cc index 9d8f6b5..efdef54 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -628,6 +628,7 @@ Buffer::insert_text(int row, int col, std::string_view text) if (!text.empty()) { content_.Insert(off, text.data(), text.size()); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) swap_rec_->OnInsert(row, col, text); } @@ -709,6 +710,7 @@ Buffer::delete_text(int row, int col, std::size_t len) return; content_.Delete(start, actual); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) swap_rec_->OnDelete(row, col, actual); } @@ -727,6 +729,7 @@ Buffer::split_line(int row, const int col) const char nl = '\n'; content_.Insert(off, &nl, 1); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) swap_rec_->OnInsert(row, c, std::string_view("\n", 1)); } @@ -746,6 +749,7 @@ Buffer::join_lines(int row) // end_of_line now equals line end (clamped before newline). The newline should be exactly at this position. content_.Delete(end_of_line, 1); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) swap_rec_->OnDelete(row, col, 1); } @@ -762,6 +766,7 @@ Buffer::insert_row(int row, const std::string_view text) const char nl = '\n'; content_.Insert(off + text.size(), &nl, 1); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) { // Avoid allocation: emit the row text insertion (if any) and the newline insertion. if (!text.empty()) @@ -789,6 +794,7 @@ Buffer::delete_row(int row) return; content_.Delete(start, actual); rows_cache_dirty_ = true; + edited_at_(row); if (swap_rec_) swap_rec_->OnDelete(row, 0, actual); } diff --git a/Buffer.h b/Buffer.h index c6f177e..ba33ce2 100644 --- a/Buffer.h +++ b/Buffer.h @@ -390,21 +390,17 @@ public: void SetDirty(bool d) { + // The version tracks content, not the dirty flag: every text mutation + // bumps it (see edited_at_), including undo/redo back to the saved state. dirty_ = d; - // Bump even when clearing: undo/redo back to the saved state changes the - // text and then calls SetDirty(false). Without a bump the highlighter - // keeps serving spans for the pre-undo text. - MarkContentChanged(); } - // Invalidate version-keyed caches (syntax highlighting) after a content change. + // Whole-buffer content change (load, reload, replace): bump the version + // and drop all cached highlighting. void MarkContentChanged() { - ++version_; - if (highlighter_) { - highlighter_->InvalidateFrom(0); - } + edited_at_(0); } @@ -699,6 +695,15 @@ private: // Syntax/highlighting state std::uint64_t version_ = 0; // increment on edits + + // Record a content change starting at `row`: bump the version and let the + // highlighter drop cached rows from `row` down (rows above are unaffected). + void edited_at_(int row) + { + ++version_; + if (highlighter_) + highlighter_->OnEdit(row < 0 ? 0 : row, version_); + } bool syntax_enabled_ = true; bool syntax_user_override_ = false; // true once user explicitly set syntax state via a command std::string filetype_; diff --git a/syntax/HighlighterEngine.cc b/syntax/HighlighterEngine.cc index 6cfab06..206f894 100644 --- a/syntax/HighlighterEngine.cc +++ b/syntax/HighlighterEngine.cc @@ -2,6 +2,8 @@ #include "../Buffer.h" #include "LanguageHighlighter.h" +#include + namespace kte { HighlighterEngine::HighlighterEngine() = default; @@ -14,109 +16,79 @@ HighlighterEngine::SetHighlighter(std::unique_ptr hl) { std::lock_guard lock(mtx_); hl_ = std::move(hl); + clear_caches_locked(); +} + + +void +HighlighterEngine::clear_caches_locked() const +{ cache_.clear(); - state_cache_.clear(); - state_last_contig_.clear(); + states_.clear(); +} + + +void +HighlighterEngine::invalidate_from_locked(int row) const +{ + if (row < 0) + row = 0; + cache_.erase(cache_.lower_bound(row), cache_.end()); + if (states_.size() > static_cast(row)) + states_.resize(static_cast(row)); } LineHighlight HighlighterEngine::GetLine(const Buffer &buf, int row, std::uint64_t buf_version) const { - std::unique_lock lock(mtx_); - auto it = cache_.find(row); - if (it != cache_.end() && it->second.version == buf_version) { - return it->second; // return by value (copy) + std::lock_guard lock(mtx_); + if (!have_version_ || buf_version != version_) { + // Content changed without OnEdit: nothing cached can be trusted. + clear_caches_locked(); + version_ = buf_version; + have_version_ = true; } + auto it = cache_.find(row); + if (it != cache_.end()) + return it->second; // return by value (copy) - // We'll compute into a local result to avoid exposing references to cache LineHighlight result; result.version = buf_version; - result.spans.clear(); - if (!hl_) { - // Cache empty result and return it + if (!hl_ || row < 0) { cache_[row] = result; return result; } - // Copy shared_ptr-like raw pointer for use outside critical sections - LanguageHighlighter *hl_ptr = hl_.get(); - bool is_stateful = dynamic_cast(hl_ptr) != nullptr; - - if (!is_stateful) { - // Stateless fast path: we can release the lock while computing to reduce contention - lock.unlock(); - hl_ptr->HighlightLine(buf, row, result.spans); - // Update cache and return - std::lock_guard gl(mtx_); + auto *stateful = dynamic_cast(hl_.get()); + if (!stateful) { + hl_->HighlightLine(buf, row, result.spans); cache_[row] = result; return result; } - // Stateful path: we need to walk from a known previous state. Keep lock while consulting caches, - // but release during heavy computation. - auto *stateful = static_cast(hl_ptr); - - StatefulHighlighter::LineState prev_state; - int start_row = -1; - - // Fast path: state_last_contig_ tracks the highest row we've cached state - // for, per version. If that row is already below our target, it's - // necessarily the best anchor the O(n) scan below would have found, so we - // can skip the scan entirely. Re-validated against state_cache_ before - // use since InvalidateFrom()/SetHighlighter() may have raced/cleared it. - auto contig_it = state_last_contig_.find(buf_version); - if (contig_it != state_last_contig_.end() && contig_it->second < row) { - auto sc_it = state_cache_.find(contig_it->second); - if (sc_it != state_cache_.end() && sc_it->second.version == buf_version) { - start_row = contig_it->second; - prev_state = sc_it->second.state; - } + // Stateful: continue from the last row with a known end state (or the + // row just above the target, if that is known), caching every row's + // spans and state along the way so neighbouring rows are cache hits. + const int known = static_cast(states_.size()) - 1; + const int start = std::min(known, row - 1); + StatefulHighlighter::LineState state = (start >= 0) ? states_[static_cast(start)] + : StatefulHighlighter::LineState{}; + const int nrows = static_cast(buf.Nrows()); + for (int r = start + 1; r <= row && r < nrows; ++r) { + LineHighlight lh; + lh.version = buf_version; + state = stateful->HighlightLineStateful(buf, r, state, lh.spans); + if (static_cast(r) == states_.size()) + states_.push_back(state); + else + states_[static_cast(r)] = state; + if (r == row) + result = lh; + else + cache_.insert_or_assign(r, std::move(lh)); } - - if (start_row < 0 && !state_cache_.empty()) { - // linear search over map (unordered), track best candidate - int best = -1; - for (const auto &kv: state_cache_) { - int r = kv.first; - // Only use cached state if it's for the current version and row still exists - if (r <= row - 1 && kv.second.version == buf_version) { - // Validate that the cached row index is still valid in the buffer - if (r >= 0 && static_cast(r) < buf.Nrows()) { - if (r > best) - best = r; - } - } - } - if (best >= 0) { - start_row = best; - prev_state = state_cache_.at(best).state; - } - } - - // We'll compute states and the target line's spans without holding the lock for most of the work. - // Create a local copy of prev_state and iterate rows; we will update caches under lock. - lock.unlock(); - StatefulHighlighter::LineState cur_state = prev_state; - for (int r = start_row + 1; r <= row; ++r) { - std::vector tmp; - std::vector &out = (r == row) ? result.spans : tmp; - auto next_state = stateful->HighlightLineStateful(buf, r, cur_state, out); - // Update state cache for r - std::lock_guard gl(mtx_); - StateEntry se; - se.version = buf_version; - se.state = next_state; - state_cache_[r] = se; - cur_state = next_state; - int &contig = state_last_contig_[buf_version]; - if (r > contig) - contig = r; - } - - // Store in cache and return by value - lock.lock(); cache_[row] = result; return result; } @@ -126,31 +98,18 @@ void HighlighterEngine::InvalidateFrom(int row) { std::lock_guard lock(mtx_); - if (cache_.empty()) - return; - // Simple implementation: erase all rows >= row - for (auto it = cache_.begin(); it != cache_.end();) { - if (it->first >= row) - it = cache_.erase(it); - else - ++it; - } - if (!state_cache_.empty()) { - for (auto it = state_cache_.begin(); it != state_cache_.end();) { - if (it->first >= row) - it = state_cache_.erase(it); - else - ++it; - } - } - // A version's tracked contiguous-state row is no longer valid once any - // row at or above it has been evicted from state_cache_ above. - for (auto it = state_last_contig_.begin(); it != state_last_contig_.end();) { - if (it->second >= row) - it = state_last_contig_.erase(it); - else - ++it; - } + invalidate_from_locked(row); +} + + +void +HighlighterEngine::OnEdit(int row, std::uint64_t buf_version) +{ + std::lock_guard lock(mtx_); + if (have_version_) + invalidate_from_locked(row); + version_ = buf_version; + have_version_ = true; } diff --git a/syntax/HighlighterEngine.h b/syntax/HighlighterEngine.h index 8e581a9..5c5e164 100644 --- a/syntax/HighlighterEngine.h +++ b/syntax/HighlighterEngine.h @@ -2,8 +2,8 @@ #pragma once #include +#include #include -#include #include #include @@ -29,6 +29,12 @@ public: // Invalidate cached lines from row (inclusive) void InvalidateFrom(int row); + // The buffer changed at `row` and is now at `buf_version`. Highlighting + // of a row depends only on the rows above it, so rows before `row` stay + // cached. A version change that arrives without OnEdit (a path that + // changed content without reporting it) still clears the whole cache. + void OnEdit(int row, std::uint64_t buf_version); + bool HasHighlighter() const { @@ -46,20 +52,21 @@ public: private: std::unique_ptr hl_; - // Simple cache by row index (mutable to allow caching in const GetLine) - mutable std::unordered_map cache_; + // Cached spans by row, valid for version_ (ordered so InvalidateFrom can + // drop a tail cheaply). + mutable std::map cache_; - // For stateful highlighters, remember per-line state (state after finishing that row) - struct StateEntry { - std::uint64_t version{0}; - // Using the interface type; forward-declare via header - StatefulHighlighter::LineState state; - }; + // For stateful highlighters: states_[r] is the state after row r, for + // every row 0..states_.size()-1 (always a contiguous prefix). + mutable std::vector states_; - mutable std::unordered_map state_cache_; + // Buffer version the caches describe. + mutable std::uint64_t version_{0}; + mutable bool have_version_{false}; - // Track best known contiguous state row for a given version to avoid O(n) scans - mutable std::unordered_map state_last_contig_; + void clear_caches_locked() const; + + void invalidate_from_locked(int row) const; // Guards the caches above. mutable std::mutex mtx_; diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc index 4de17cf..c9a25a8 100644 --- a/tests/test_audit_regressions.cc +++ b/tests/test_audit_regressions.cc @@ -9,6 +9,9 @@ #include "ErrorRecovery.h" #include "PieceTable.h" #include "UndoTree.h" +#include "syntax/HighlighterEngine.h" +#include "syntax/HighlighterRegistry.h" +#include "syntax/LanguageHighlighter.h" #include #include @@ -202,15 +205,77 @@ TEST(Audit_D6_Reflow_OnBlankLine_KeepsParagraphsSeparate) } -// D9: clearing the dirty flag (undo to the saved state) changes the version -// so version-keyed highlight caches are refreshed. -TEST(Audit_D9_SetDirtyFalse_BumpsVersion) +static bool +row_has_comment(Buffer &b, int row) { - Buffer b; - b.SetDirty(true); - const auto v = b.Version(); + const auto lh = b.Highlighter()->GetLine(b, row, b.Version()); + for (const auto &sp: lh.spans) + if (sp.kind == kte::TokenKind::Comment) + return true; + return false; +} + + +// D9: undo back to the saved state must not leave stale highlighting (the +// rest of the file coloured as a comment after "/*" is undone). +TEST(Audit_D9_UndoToSaved_RefreshesHighlighting) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "int a;\nint b;\n"); + b.EnsureHighlighter(); + b.Highlighter()->SetHighlighter(kte::HighlighterRegistry::CreateFor("cpp")); + if (auto *u = b.Undo()) + u->mark_saved(); b.SetDirty(false); - ASSERT_TRUE(b.Version() != v); + ASSERT_TRUE(!row_has_comment(b, 1)); + + b.SetCursor(0, 0); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "/*")); + ASSERT_TRUE(row_has_comment(b, 1)); + + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Line(0), std::string("int a;")); + ASSERT_TRUE(!b.Dirty()); + ASSERT_TRUE(!row_has_comment(b, 1)); +} + + +// P3: an edit keeps cached highlighting for the rows above it; only rows +// from the edit down are recomputed. +TEST(Audit_P3_EditKeepsHighlightCacheAbove) +{ + struct Counting : kte::StatefulHighlighter { + int *calls; + explicit Counting(int *c) : calls(c) {} + + void HighlightLine(const Buffer &, int, std::vector &) const override {} + + LineState HighlightLineStateful(const Buffer &, int, const LineState &prev, + std::vector &) const override + { + ++*calls; + return prev; + } + }; + + TestHarness h; + Buffer &b = h.Buf(); + std::string text; + for (int i = 0; i < 1000; ++i) + text += "line\n"; + b.insert_text(0, 0, text); + b.EnsureHighlighter(); + int calls = 0; + b.Highlighter()->SetHighlighter(std::make_unique(&calls)); + + (void) b.Highlighter()->GetLine(b, 999, b.Version()); + ASSERT_EQ(calls, 1000); + + calls = 0; + b.insert_text(990, 0, "x"); + (void) b.Highlighter()->GetLine(b, 999, b.Version()); + ASSERT_EQ(calls, 10); // rows 990..999, not the whole file }