Fix stale TUI styling after edits
This commit is contained in:
parent
7cf79aeec0
commit
9718958c4c
|
|
@ -17,6 +17,11 @@
|
|||
-- when `pmacs.parse._dispatch` is called below; drained by the tick
|
||||
-- step.
|
||||
local pending_parse_jobs = {}
|
||||
local parse_job_buffer_keys = {}
|
||||
local inflight_parse_by_buffer = {}
|
||||
local parse_buffer_by_key = {}
|
||||
local parse_lang_by_buffer = {}
|
||||
local reparse_requested_by_buffer = {}
|
||||
|
||||
local raw_dispatch = pmacs.parse._dispatch
|
||||
|
||||
|
|
@ -24,8 +29,18 @@ local raw_dispatch = pmacs.parse._dispatch
|
|||
-- pending set. Calls into `_dispatch` from outside this file (e.g.
|
||||
-- a hand-rolled user script) get the same tracking for free.
|
||||
function pmacs.parse._dispatch(buf, lang)
|
||||
local key = tostring(buf)
|
||||
parse_buffer_by_key[key] = buf
|
||||
parse_lang_by_buffer[key] = lang
|
||||
local inflight = inflight_parse_by_buffer[key]
|
||||
if inflight then
|
||||
reparse_requested_by_buffer[key] = true
|
||||
return inflight
|
||||
end
|
||||
local job_id = raw_dispatch(buf, lang)
|
||||
pending_parse_jobs[job_id] = true
|
||||
parse_job_buffer_keys[job_id] = key
|
||||
inflight_parse_by_buffer[key] = job_id
|
||||
return job_id
|
||||
end
|
||||
|
||||
|
|
@ -68,6 +83,40 @@ pmacs.hook.add("buffer.after-load", function()
|
|||
end
|
||||
end)
|
||||
|
||||
local function reparse_active_buffer_after_edit()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return end
|
||||
if not pmacs.parse._has_view(buf) then return end
|
||||
local pending = pmacs.parse._pending_edits(buf)
|
||||
if not pending or pending == 0 then return end
|
||||
local path = buf:name()
|
||||
if not path then return end
|
||||
local lang = pmacs.parse.language_for_path(path)
|
||||
if not lang then return end
|
||||
pmacs.parse._dispatch(buf, lang)
|
||||
end
|
||||
|
||||
pmacs.hook.add("buffer.after-edit", function()
|
||||
-- `ParseView:on_edit` records incremental edits synchronously, but
|
||||
-- highlight overlays only see new spans after a fresh parse settles.
|
||||
local ok, err = pcall(reparse_active_buffer_after_edit)
|
||||
if not ok and pmacs.error then
|
||||
pmacs.error("syntax.after-edit: " .. tostring(err))
|
||||
end
|
||||
end)
|
||||
|
||||
local function dispatch_follow_up_if_dirty(key)
|
||||
local buf = parse_buffer_by_key[key]
|
||||
local lang = parse_lang_by_buffer[key]
|
||||
if not buf or not lang then return end
|
||||
local pending = pmacs.parse._pending_edits(buf)
|
||||
local requested = reparse_requested_by_buffer[key]
|
||||
reparse_requested_by_buffer[key] = nil
|
||||
if requested or (pending and pending > 0) then
|
||||
pmacs.parse._dispatch(buf, lang)
|
||||
end
|
||||
end
|
||||
|
||||
-- After-tick step: any parse job that has settled gets its bundle
|
||||
-- installed into the buffer's view (and its pending entry drained).
|
||||
-- Extension hook on top of the async runtime's tick rather than a
|
||||
|
|
@ -78,8 +127,14 @@ pmacs._async.tick = function(...)
|
|||
local ret = prior_tick(...)
|
||||
for job_id in pairs(pending_parse_jobs) do
|
||||
if pmacs._async._is_complete(job_id) then
|
||||
local key = parse_job_buffer_keys[job_id]
|
||||
pmacs.parse._install_settled(job_id)
|
||||
pending_parse_jobs[job_id] = nil
|
||||
parse_job_buffer_keys[job_id] = nil
|
||||
if key and inflight_parse_by_buffer[key] == job_id then
|
||||
inflight_parse_by_buffer[key] = nil
|
||||
dispatch_follow_up_if_dirty(key)
|
||||
end
|
||||
end
|
||||
end
|
||||
return ret
|
||||
|
|
|
|||
49
src/diag.rs
49
src/diag.rs
|
|
@ -421,6 +421,9 @@ impl View for DiagnosticView {
|
|||
// doesn't today, but the discipline is cheap).
|
||||
let diags: Vec<Diagnostic> = {
|
||||
let guard = self.store.lock().expect("diag store mutex poisoned");
|
||||
if guard.is_stale(&self.uri) {
|
||||
return;
|
||||
}
|
||||
guard.for_uri(&self.uri).to_vec()
|
||||
};
|
||||
if diags.is_empty() {
|
||||
|
|
@ -796,4 +799,50 @@ mod tests {
|
|||
let view = DiagnosticView::new("file:///a", store);
|
||||
assert_eq!(view.kind(), "diagnostic");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diagnostic_view_suppresses_stale_store_entries() {
|
||||
use crate::cell::{Cell, CellSize, UnderlineStyle};
|
||||
|
||||
let store = make_shared_store();
|
||||
{
|
||||
let mut guard = store.lock().expect("diag store");
|
||||
guard.set(
|
||||
"file:///a",
|
||||
vec![diag(0, DiagnosticSeverity::Error, "stale")],
|
||||
);
|
||||
guard.mark_stale("file:///a");
|
||||
}
|
||||
|
||||
let mut buf = Buffer::new(crate::buffer::BufferId::next(), "test.c");
|
||||
buf.apply_edit(crate::buffer::EditOp::Insert {
|
||||
pos: 0,
|
||||
bytes: b"hello\n",
|
||||
})
|
||||
.expect("seed buffer");
|
||||
|
||||
let mut view = DiagnosticView::new("file:///a", store);
|
||||
let mut backing = vec![Cell::default(); 10];
|
||||
let mut grid = CellGrid {
|
||||
cells: &mut backing,
|
||||
stride: 10,
|
||||
size: CellSize::new(1, 10),
|
||||
};
|
||||
view.render(
|
||||
&buf,
|
||||
Viewport {
|
||||
buffer_start: 0,
|
||||
buffer_end: buf.len(),
|
||||
cell_origin: CellCoord::new(0, 0),
|
||||
cell_size: CellSize::new(1, 10),
|
||||
},
|
||||
&mut grid,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
grid.get(CellCoord::new(0, 0)).style.underline,
|
||||
UnderlineStyle::None,
|
||||
"stale diagnostics must not underline shifted TUI bytes"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -531,6 +531,9 @@ impl View for LspStyleView {
|
|||
};
|
||||
let store = mgr.semantic_token_store();
|
||||
let guard = store.lock().expect("semantic-token store mutex poisoned");
|
||||
if guard.is_stale(&uri) {
|
||||
return;
|
||||
}
|
||||
let Some((_, resp)) = guard.for_uri(&uri) else {
|
||||
return;
|
||||
};
|
||||
|
|
@ -878,6 +881,94 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lsp_style_view_suppresses_stale_semantic_tokens() {
|
||||
use crate::cell::{Cell, CellSize};
|
||||
use crate::editor::EditorState;
|
||||
use crate::lsp::PositionEncoding;
|
||||
use crate::semantic_tokens::{SemanticToken, SemanticTokenKey, SemanticTokensResponse};
|
||||
|
||||
let state = EditorState::new();
|
||||
let buffer_id = state.core.borrow().active_window().buffer_id;
|
||||
{
|
||||
let mut core = state.core.borrow_mut();
|
||||
core.registry
|
||||
.clone()
|
||||
.borrow_mut()
|
||||
.get_mut(buffer_id)
|
||||
.expect("active buffer")
|
||||
.apply_edit(crate::buffer::EditOp::Insert {
|
||||
pos: 0,
|
||||
bytes: b"foo\n",
|
||||
})
|
||||
.expect("seed");
|
||||
core.set_buffer_path(buffer_id, Some(std::path::PathBuf::from("/tmp/x.c")));
|
||||
}
|
||||
state.syntax_registry.theme().lock().expect("theme").insert(
|
||||
"function",
|
||||
Style {
|
||||
bold: true,
|
||||
..Style::default()
|
||||
},
|
||||
);
|
||||
let sid = state
|
||||
.lsp_manager
|
||||
.borrow_mut()
|
||||
.insert_initialized_test_client(
|
||||
serde_json::json!({
|
||||
"semanticTokensProvider": {
|
||||
"legend": { "tokenTypes": ["function"], "tokenModifiers": [] }
|
||||
}
|
||||
}),
|
||||
PositionEncoding::Utf16,
|
||||
);
|
||||
let active_path = state.core.borrow().active_buffer_path().expect("path set");
|
||||
let uri = crate::lsp::path_to_file_uri(&active_path);
|
||||
{
|
||||
let mgr = state.lsp_manager.borrow();
|
||||
let store = mgr.semantic_token_store();
|
||||
let mut guard = store.lock().expect("store");
|
||||
guard.set(
|
||||
SemanticTokenKey::new(sid.raw().to_string(), uri.clone()),
|
||||
SemanticTokensResponse {
|
||||
tokens: vec![SemanticToken {
|
||||
line: 0,
|
||||
start: 0,
|
||||
length: 3,
|
||||
token_type: 0,
|
||||
token_modifiers: 0,
|
||||
}],
|
||||
result_id: None,
|
||||
raw: Vec::new(),
|
||||
},
|
||||
);
|
||||
guard.mark_stale(uri);
|
||||
}
|
||||
|
||||
let mut view = LspStyleView::new(state.lsp_manager.clone(), state.syntax_registry.theme());
|
||||
let mut backing: Vec<Cell> = vec![Cell::default(); 20];
|
||||
let mut grid = CellGrid {
|
||||
cells: &mut backing,
|
||||
stride: 20,
|
||||
size: CellSize::new(1, 20),
|
||||
};
|
||||
let viewport = Viewport {
|
||||
buffer_start: 0,
|
||||
buffer_end: u64::MAX,
|
||||
cell_origin: CellCoord::new(0, 0),
|
||||
cell_size: CellSize::new(1, 20),
|
||||
};
|
||||
let registry = state.core.borrow().registry.clone();
|
||||
let reg = registry.borrow();
|
||||
let buf = reg.get(buffer_id).expect("buffer");
|
||||
view.render(buf, viewport, &mut grid);
|
||||
|
||||
assert!(
|
||||
!grid.get(CellCoord::new(0, 0)).style.bold,
|
||||
"stale semantic tokens must not paint over current syntax/text"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[allow(clippy::too_many_lines)] // Scripted fixture; the headline
|
||||
// test next door is similarly
|
||||
|
|
|
|||
|
|
@ -3053,6 +3053,10 @@ impl LspManager {
|
|||
.lock()
|
||||
.expect("diag store mutex poisoned")
|
||||
.mark_stale(uri.clone());
|
||||
self.semantic_token_store
|
||||
.lock()
|
||||
.expect("semantic token store mutex poisoned")
|
||||
.mark_stale(uri.clone());
|
||||
let params = json!({
|
||||
"textDocument": {
|
||||
"uri": uri,
|
||||
|
|
|
|||
|
|
@ -770,6 +770,9 @@ fn lsp_scoped_style_spans(state: &EditorState, vp: &DeclaredViewport) -> Vec<Sty
|
|||
let tokens = {
|
||||
let store = mgr.semantic_token_store();
|
||||
let guard = store.lock().expect("semantic-token store mutex poisoned");
|
||||
if guard.is_stale(&uri) {
|
||||
return Vec::new();
|
||||
}
|
||||
match guard.for_uri(&uri) {
|
||||
Some((_, resp)) => resp.tokens.clone(),
|
||||
None => return Vec::new(),
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@
|
|||
//! priority vs. tree-sitter) is a separate rendering milestone; like
|
||||
//! the other LSP features, nothing here paints — Lua reads the store.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use serde_json::Value;
|
||||
|
|
@ -219,6 +219,10 @@ impl SemanticTokensLegend {
|
|||
#[derive(Default)]
|
||||
pub struct SemanticTokenStore {
|
||||
by_key: HashMap<SemanticTokenKey, SemanticTokensResponse>,
|
||||
/// URIs whose stored semantic tokens are known to be stale
|
||||
/// because the document changed after the last full/delta token
|
||||
/// response was absorbed.
|
||||
stale_uris: HashSet<String>,
|
||||
}
|
||||
|
||||
/// Key into [`SemanticTokenStore`].
|
||||
|
|
@ -250,12 +254,31 @@ impl SemanticTokenStore {
|
|||
|
||||
/// Replace the response at `key`.
|
||||
pub fn set(&mut self, key: SemanticTokenKey, response: SemanticTokensResponse) {
|
||||
self.stale_uris.remove(&key.uri);
|
||||
self.by_key.insert(key, response);
|
||||
}
|
||||
|
||||
/// Drop the entry at `key`.
|
||||
/// Drop the entry at `key`. Also clears the stale flag for that
|
||||
/// URI when no other server has token data for it.
|
||||
pub fn clear(&mut self, key: &SemanticTokenKey) {
|
||||
self.by_key.remove(key);
|
||||
if !self.by_key.keys().any(|k| k.uri == key.uri) {
|
||||
self.stale_uris.remove(&key.uri);
|
||||
}
|
||||
}
|
||||
|
||||
/// Mark all semantic-token entries for `uri` stale. Called when
|
||||
/// a `textDocument/didChange` is sent so renderers do not paint
|
||||
/// byte ranges from a pre-edit token set.
|
||||
pub fn mark_stale(&mut self, uri: impl Into<String>) {
|
||||
self.stale_uris.insert(uri.into());
|
||||
}
|
||||
|
||||
/// `true` iff `uri` has semantic-token data that should not be
|
||||
/// rendered against the current buffer text.
|
||||
#[must_use]
|
||||
pub fn is_stale(&self, uri: &str) -> bool {
|
||||
self.stale_uris.contains(uri)
|
||||
}
|
||||
|
||||
/// Look up the entry at `key`.
|
||||
|
|
@ -425,6 +448,40 @@ mod tests {
|
|||
assert!(s.get(&key).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_flag_clears_on_set_and_final_clear() {
|
||||
let mut s = SemanticTokenStore::new();
|
||||
let key = SemanticTokenKey::new("1", "file:///a");
|
||||
let response = SemanticTokensResponse {
|
||||
tokens: vec![SemanticToken {
|
||||
line: 0,
|
||||
start: 0,
|
||||
length: 1,
|
||||
token_type: 0,
|
||||
token_modifiers: 0,
|
||||
}],
|
||||
result_id: None,
|
||||
raw: vec![0, 0, 1, 0, 0],
|
||||
};
|
||||
|
||||
s.set(key.clone(), response.clone());
|
||||
s.mark_stale("file:///a");
|
||||
assert!(s.is_stale("file:///a"));
|
||||
|
||||
s.set(key.clone(), response);
|
||||
assert!(
|
||||
!s.is_stale("file:///a"),
|
||||
"fresh semantic tokens clear stale flag"
|
||||
);
|
||||
|
||||
s.mark_stale("file:///a");
|
||||
s.clear(&key);
|
||||
assert!(
|
||||
!s.is_stale("file:///a"),
|
||||
"clearing final token entry clears stale flag"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_uri_filters_by_uri_and_picks_lowest_server() {
|
||||
let mk = |tok_type: u32| SemanticTokensResponse {
|
||||
|
|
|
|||
|
|
@ -87,8 +87,10 @@ use std::fmt::Write as _;
|
|||
use std::sync::Arc;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
|
||||
use pmacs::async_runtime::{AsyncRuntime, JobOutcome, JobResult};
|
||||
use pmacs::buffer::{Buffer, BufferId, EditOp};
|
||||
use pmacs::protocol::FrontendId;
|
||||
use pmacs::syntax::{self, ParseRequest, ParseView};
|
||||
|
||||
/// Synthetic Rust source: `n` lines of plausible-but-trivial code.
|
||||
|
|
@ -330,6 +332,20 @@ fn current_tree_language(state: &pmacs::editor::EditorState) -> Option<String> {
|
|||
lua.load(chunk).eval::<Option<String>>().ok().flatten()
|
||||
}
|
||||
|
||||
/// Returns the source snapshot attached to the active buffer's most
|
||||
/// recently installed parse tree.
|
||||
fn current_tree_text(state: &pmacs::editor::EditorState) -> Option<String> {
|
||||
let lua = state.lua_host.lua();
|
||||
let chunk = r"
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return nil end
|
||||
local tree = pmacs.parse.tree(buf)
|
||||
if not tree then return nil end
|
||||
return tree:text()
|
||||
";
|
||||
lua.load(chunk).eval::<Option<String>>().ok().flatten()
|
||||
}
|
||||
|
||||
/// Opening a `.rs` file produces a parse tree whose root is a
|
||||
/// `source_file` and whose language label is `rust`. The dispatch
|
||||
/// runs through the `buffer.after-load` hook installed by
|
||||
|
|
@ -625,6 +641,75 @@ fn m4_3_highlight_updates_within_one_frame_after_parse() {
|
|||
);
|
||||
}
|
||||
|
||||
/// Regression for task #25: an edit delivered through the normal
|
||||
/// key-dispatch path must trigger the syntax runtime to dispatch a
|
||||
/// fresh parse. Without the `buffer.after-edit` hook in
|
||||
/// `builtin/runtime/syntax.lua`, `ParseView:on_edit` accumulates a
|
||||
/// pending edit but `pmacs.parse.tree(buf)` remains the old source
|
||||
/// indefinitely, which leaves syntax colors pinned to stale byte
|
||||
/// offsets in both frontends.
|
||||
#[test]
|
||||
fn m4_3_key_edit_reparses_active_buffer_without_manual_dispatch() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let path = dir.path().join("after_edit.rs");
|
||||
std::fs::write(&path, b"fn main() {}\n").expect("write");
|
||||
|
||||
let mut state = open_and_wait_for_parse(path);
|
||||
assert_eq!(current_tree_text(&state).as_deref(), Some("fn main() {}\n"));
|
||||
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE),
|
||||
);
|
||||
|
||||
let pending_after_key: Option<usize> = state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
r"
|
||||
local buf = pmacs.window.buffer()
|
||||
return pmacs.parse._pending_edits(buf)
|
||||
",
|
||||
)
|
||||
.eval()
|
||||
.expect("pending edit count");
|
||||
assert_eq!(
|
||||
pending_after_key,
|
||||
Some(0),
|
||||
"after-edit syntax hook should dispatch and drain pending parse edits"
|
||||
);
|
||||
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE),
|
||||
);
|
||||
|
||||
let pending_after_second_key: Option<usize> = state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(
|
||||
r"
|
||||
local buf = pmacs.window.buffer()
|
||||
return pmacs.parse._pending_edits(buf)
|
||||
",
|
||||
)
|
||||
.eval()
|
||||
.expect("pending edit count after queued edit");
|
||||
assert_eq!(
|
||||
pending_after_second_key,
|
||||
Some(1),
|
||||
"edits made while a parse is in flight should wait for a follow-up parse"
|
||||
);
|
||||
|
||||
pump_async(&mut state, |s| {
|
||||
current_tree_text(s).as_deref() == Some("xyfn main() {}\n")
|
||||
});
|
||||
assert_eq!(
|
||||
current_tree_text(&state).as_deref(),
|
||||
Some("xyfn main() {}\n")
|
||||
);
|
||||
}
|
||||
|
||||
/// A Lua-set theme observably changes the rendered cells: replacing
|
||||
/// the default theme with one that maps `keyword` to a distinctive
|
||||
/// foreground color produces cells in that color where keywords
|
||||
|
|
@ -1295,6 +1380,13 @@ fn m4_5_did_change_notifications_go_out_after_edits() {
|
|||
.did_change_full(sid, uri, v, text)
|
||||
.expect("didChange");
|
||||
}
|
||||
{
|
||||
let store = mgr.borrow().semantic_token_store();
|
||||
assert!(
|
||||
store.lock().expect("semantic token store").is_stale(uri),
|
||||
"didChange must mark semantic tokens stale so stale TUI LSP styles are suppressed"
|
||||
);
|
||||
}
|
||||
// The fake LSP replies with a `pmacs/echo` notification per
|
||||
// didOpen/didChange (5 total).
|
||||
let evs = drain_lsp_until(&sup, &mgr, sid, Duration::from_secs(5), |evs| {
|
||||
|
|
|
|||
Loading…
Reference in New Issue