Merge pull request #102 from levineuwirth/lsp-autopull-and-signature-trigger

builtin/runtime/lsp.lua

@@ -261,6 +261,9 @@ local pending_did_change = {}
 -- Forward declaration — defined below (needs helpers that follow);
 -- `flush_did_change` re-pulls inlay hints after each coalesced send.
 local pull_inlay_hints_quiet
+-- Same, for semantic tokens (Arc 1c). They are pull-model too, and
+-- nothing was pulling them.
+local pull_semantic_tokens_quiet
 
 local function flush_did_change(key)
   local pending = pending_did_change[key]
@@ -282,6 +285,8 @@ local function flush_did_change(key)
   -- supersede-keyed per (server, method, uri), so a burst of flushes
   -- cancels its own predecessors rather than piling up.
   pcall(pull_inlay_hints_quiet, rec)
+  -- Semantic tokens are pull-model on exactly the same terms (Arc 1c).
+  pcall(pull_semantic_tokens_quiet, rec)
 end
 
 local function flush_did_change_for(rec)
@@ -447,6 +452,47 @@ function pull_inlay_hints_quiet(rec)
   end)
 end
 
+local function server_supports_semantic_tokens(sid)
+  local ok, caps = pcall(pmacs.lsp.capabilities, sid)
+  if not ok or not caps then return false end
+  local p = caps.semanticTokensProvider
+  return p ~= nil and p ~= false
+end
+
+-- Arc 1c. Semantic tokens are pull-model, exactly like inlay hints: the
+-- server never volunteers them, and the store only fills from a
+-- `textDocument/semanticTokens/*` response. Until now the ONLY automatic
+-- pull was in reply to a server-initiated `workspace/semanticTokens
+-- /refresh` --- which most servers never send --- so semantic styling
+-- silently never appeared unless the user ran `M-x lsp.semantic-tokens`
+-- by hand. Attach and edit-flush now pull it, the same two points that
+-- already pull inlay hints.
+--
+-- Assigns the forward-declared local above (a fresh `local function`
+-- here would shadow it, leaving `flush_did_change`'s upvalue nil).
+function pull_semantic_tokens_quiet(rec)
+  if not rec or not server_is_initialized(rec.server) then return end
+  if not server_supports_semantic_tokens(rec.server) then return end
+  -- The server must see the current text before computing token
+  -- positions against it. A no-op when called from `flush_did_change`
+  -- itself (the pending entry is removed before the send).
+  flush_did_change_for(rec)
+  -- Delta when we hold a `resultId` (the server only returns one when it
+  -- supports delta), full otherwise --- matching `pmacs.lsp
+  -- .semantic_tokens()`. Never clear the store first: a delta splices
+  -- against the retained raw stream.
+  local prev = pmacs.semantic_tokens.result_id(rec.server, rec.uri)
+  pmacs.async(function()
+    pcall(function()
+      if prev then
+        pmacs.lsp.request_semantic_tokens_delta(rec.server, rec.uri, prev):await()
+      else
+        pmacs.lsp.request_semantic_tokens(rec.server, rec.uri):await()
+      end
+    end)
+  end)
+end
+
 -- M_B1: buffers that already had an `LspStyleView` overlay pushed,
 -- so the after-load / on-demand attach paths don't stack duplicate
 -- overlays. Mirrors `highlighted_buffers` in `syntax.lua`; the entry
@@ -514,6 +560,9 @@ local function attach_buffer(buf)
     if ok and attached then diag_viewed_buffers[key] = true end
   end
   pull_inlay_hints_quiet(rec)
+  -- Arc 1c: the LspStyleView was just attached above, but nothing ever
+  -- filled the semantic-token store it reads. Pull once on attach.
+  pull_semantic_tokens_quiet(rec)
   return rec
 end
 
@@ -589,6 +638,67 @@ pmacs.hook.add("buffer.after-switch", function()
   if ok_d and attached_d then diag_viewed_buffers[key] = true end
 end)
 
+-- Arc 1d: signature-help auto-trigger ----------------------------------
+--
+-- `buffer.after-edit` carries no payload, so a *typed character* is
+-- reconstructed from state exactly the way `completion.lua` does (Q#C9):
+-- same buffer, cursor advanced by exactly one byte. Paste, undo, kill,
+-- and remote CRDT edits produce any other delta and never auto-trigger.
+-- (Trigger characters are ASCII, so a one-byte advance is sound.)
+local last_typed = { key = nil, cursor = nil }
+
+local function char_before(buf, cursor)
+  if cursor <= 0 then return nil end
+  local ok, s = pcall(function() return buf:slice(cursor - 1, cursor) end)
+  if not ok or type(s) ~= "string" or #s ~= 1 then return nil end
+  return s
+end
+
+-- The set of characters that should (re)open signature help, as the
+-- server declares them. `retriggerCharacters` (usually `,`) refreshes an
+-- open call's active parameter. A provider that declares neither still
+-- gets the universal pair, which is what `(` auto-trigger means in
+-- practice; no provider means no auto-trigger at all.
+local function signature_trigger_chars(sid)
+  local ok, caps = pcall(pmacs.lsp.capabilities, sid)
+  if not ok or not caps then return nil end
+  local p = caps.signatureHelpProvider
+  if not p or p == false then return nil end
+  local chars = {}
+  for _, c in ipairs(p.triggerCharacters or {}) do chars[c] = true end
+  for _, c in ipairs(p.retriggerCharacters or {}) do chars[c] = true end
+  if next(chars) == nil then
+    chars["("] = true
+    chars[","] = true
+  end
+  return chars
+end
+
+-- Like `pmacs.lsp.signature_help_at_cursor`, but silent: an auto-trigger
+-- that announced "no signature help" on every `(` in a comment would be
+-- unusable. Only a real signature reaches the status line.
+local function signature_help_quiet(rec)
+  if not server_is_initialized(rec.server) then return end
+  -- The server must see the character we just typed before it can tell
+  -- us which parameter we are inside of.
+  flush_did_change_for(rec)
+  local line = pmacs.editor.cursor_line()
+  local col = pmacs.editor.cursor_col()
+  pmacs.signature.clear(rec.server, rec.uri)
+  pmacs.async(function()
+    local ok = pcall(function()
+      pmacs.lsp.request_signature_help(rec.server, rec.uri, line, col):await()
+    end)
+    if not ok then return end
+    local help = pmacs.signature.current(rec.server, rec.uri)
+    if not help or not help.signatures or #help.signatures == 0 then return end
+    local active = help.signatures[(help.active_signature or 0) + 1]
+    if active and active.label then
+      pmacs.editor.set_status("LSP: " .. active.label)
+    end
+  end)
+end
+
 pmacs.hook.add("buffer.after-edit", function()
   local buf = pmacs.window.buffer()
   if not buf then return end
@@ -600,6 +710,13 @@ pmacs.hook.add("buffer.after-edit", function()
   -- O(file) didChange send below is coalesced: render families
   -- anchored to pre-edit positions are hidden from this edit on.
   pcall(pmacs.lsp._mark_document_stale, rec.uri)
+  -- Arc 1d: did the user just type a signature trigger character?
+  -- Recorded before the early-outs below so the snapshot stays accurate
+  -- for the *next* edit even when this one doesn't trigger.
+  local cursor = pmacs.editor.cursor()
+  local prev_key, prev_cursor = last_typed.key, last_typed.cursor
+  last_typed.key, last_typed.cursor = key, cursor
+  local typed_one = key == prev_key and prev_cursor and cursor - prev_cursor == 1
   local now = pmacs.editor.monotonic_ms()
   local pending = pending_did_change[key]
   if pending and pending.rec == rec then
@@ -607,6 +724,14 @@ pmacs.hook.add("buffer.after-edit", function()
   else
     pending_did_change[key] = { rec = rec, first_ms = now, last_ms = now }
   end
+  -- Fire *after* queuing the pending didChange: `signature_help_quiet`
+  -- flushes it, so the server sees the character we are asking about.
+  if not typed_one then return end
+  local ch = char_before(buf, cursor)
+  if not ch then return end
+  local triggers = signature_trigger_chars(rec.server)
+  if not (triggers and triggers[ch]) then return end
+  pcall(signature_help_quiet, rec)
 end)
 
 -- Async request surface (T M4.5 async bridge). The Rust manager
@@ -1148,8 +1273,15 @@ local function handle_server_requests()
           pcall(unregister_file_watchers, sid,
             ev.params and ev.params.unregisterations)
         elseif ev.kind == "initialized" then
+          -- Buffers attach before the server finishes initializing, so
+          -- the pulls in `attach_buffer` are no-ops for the FIRST file
+          -- (their `server_is_initialized` guard is false). This is the
+          -- site that actually lands them. Inlay hints were pulled here;
+          -- semantic tokens were not, which is why semantic styling never
+          -- appeared on the file that started the server (Arc 1c).
           repull_for_attachments(sid, function(_, _, rec)
             pull_inlay_hints_quiet(rec)
+            pull_semantic_tokens_quiet(rec)
           end)
         end
       end

src/bin/pmacs_fake_lsp.rs

@@ -32,6 +32,10 @@
 //!   `rootUri` received in `initialize` to the file named by
 //!   `PMACS_FAKE_LSP_ROOT_SINK`, so a test can assert the
 //!   auto-attach path derives the project root from the opened file.
+//! * If launched with `PMACS_FAKE_LSP_MODE=sighelp`: additionally
+//!   advertises `signatureHelpProvider` with `(` / `,` triggers, so a
+//!   test can drive the Arc 1d auto-trigger. Every other mode omits the
+//!   capability and therefore never auto-triggers.
 
 use std::collections::HashMap;
 use std::io::{self, Read, Write};
@@ -154,6 +158,16 @@ fn main() {
                     resp["result"]["capabilities"]["renameProvider"] =
                         serde_json::json!({ "prepareProvider": true });
                 }
+                // Arc 1d: advertise signature help only in `sighelp`, so
+                // every other mode keeps the no-auto-trigger path (the
+                // `textDocument/signatureHelp` arm below still answers
+                // the manual `M-x lsp.signature-help` in any mode).
+                if mode == "sighelp" {
+                    resp["result"]["capabilities"]["signatureHelpProvider"] = serde_json::json!({
+                        "triggerCharacters": ["("],
+                        "retriggerCharacters": [","]
+                    });
+                }
                 // T M4.5 hardening `rooturi`: record the `rootUri` the
                 // client sent in `initialize` to a side-channel file
                 // (env `PMACS_FAKE_LSP_ROOT_SINK`). Lets a test prove

tests/m4_acceptance.rs

@@ -4297,6 +4297,240 @@ fn m4_19_semantic_tokens_refresh_repulls_via_server_request() {
     );
 }
 
+/// Arc 1c — semantic tokens auto-pull **on attach**.
+///
+/// Regression for a shipped bug: semantic tokens are pull-model, but the
+/// only automatic pull was in reply to a server-initiated
+/// `workspace/semanticTokens/refresh`. Most servers never send one, so
+/// semantic styling silently never appeared unless the user ran
+/// `M-x lsp.semantic-tokens` by hand — while inlay hints, on the very
+/// same pull model, were pulled on attach and on edit-flush.
+///
+/// The **default** fake advertises `semanticTokensProvider` and never
+/// sends a refresh, which is exactly the broken case.
+#[test]
+fn arc1c_semantic_tokens_auto_pull_on_attach() {
+    use pmacs::editor::EditorState;
+
+    let dir = tempfile::tempdir().expect("tempdir");
+    let a_path = dir.path().join("a.rs");
+    std::fs::write(&a_path, b"fn a() {}\n\n").expect("write a");
+    let a_disp = a_path.display().to_string();
+
+    let mut state = EditorState::new();
+    let fake = fake_lsp_path();
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.lsp.config.rust = {{ command = '{fake}' }}"))
+        .exec()
+        .expect("override rust config");
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
+        .exec()
+        .expect("open a.rs");
+
+    let flag = format!(
+        "(function() \
+           local sid \
+           for _,r in ipairs(pmacs.lsp.list()) do \
+             if r.state and r.state.kind=='initialized' then sid=r.id end \
+           end \
+           if not sid then return false end \
+           local t = pmacs.semantic_tokens.tokens(sid, 'file://{a_disp}') \
+           return t ~= nil and #t > 0 \
+         end)()"
+    );
+    assert!(
+        pump_lua_flag(&mut state, &flag, 5),
+        "attach never auto-pulled semantic tokens (no manual call, no server refresh)"
+    );
+}
+
+/// Arc 1c — semantic tokens re-pull **on edit-flush**, the second point
+/// inlay hints already pulled from. Clears the store, types a character,
+/// and waits for the debounced `didChange` flush to refill it.
+#[test]
+fn arc1c_semantic_tokens_repull_after_edit_flush() {
+    use pmacs::editor::EditorState;
+
+    let dir = tempfile::tempdir().expect("tempdir");
+    let a_path = dir.path().join("a.rs");
+    std::fs::write(&a_path, b"fn a() {}\n\n").expect("write a");
+    let a_disp = a_path.display().to_string();
+
+    let mut state = EditorState::new();
+    let fake = fake_lsp_path();
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.lsp.config.rust = {{ command = '{fake}' }}"))
+        .exec()
+        .expect("override rust config");
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
+        .exec()
+        .expect("open a.rs");
+
+    let has_tokens = format!(
+        "(function() \
+           local sid \
+           for _,r in ipairs(pmacs.lsp.list()) do \
+             if r.state and r.state.kind=='initialized' then sid=r.id end \
+           end \
+           if not sid then return false end \
+           local t = pmacs.semantic_tokens.tokens(sid, 'file://{a_disp}') \
+           return t ~= nil and #t > 0 \
+         end)()"
+    );
+    assert!(pump_lua_flag(&mut state, &has_tokens, 5), "attach pull");
+
+    // Empty the store, then type — the flush must refill it.
+    state
+        .lua_host
+        .lua()
+        .load(format!(
+            "for _,r in ipairs(pmacs.lsp.list()) do \
+               if r.state and r.state.kind=='initialized' then \
+                 pmacs.semantic_tokens.clear(r.id, 'file://{a_disp}') \
+               end \
+             end"
+        ))
+        .exec()
+        .expect("clear the token store");
+    state.dispatch_key(
+        pmacs::protocol::FrontendId::LOCAL,
+        KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE),
+    );
+    assert!(
+        pump_lua_flag(&mut state, &has_tokens, 5),
+        "edit-flush never re-pulled semantic tokens"
+    );
+}
+
+/// Arc 1d — signature help auto-triggers on a server-declared trigger
+/// character. Typing `(` (a one-byte cursor advance, the same typed-char
+/// signature `completion.lua` uses) surfaces the active signature.
+#[test]
+fn arc1d_signature_help_auto_triggers_on_trigger_char() {
+    use pmacs::editor::EditorState;
+
+    let dir = tempfile::tempdir().expect("tempdir");
+    let a_path = dir.path().join("a.rs");
+    std::fs::write(&a_path, b"\n").expect("write a");
+    let a_disp = a_path.display().to_string();
+
+    let mut state = EditorState::new();
+    let fake = fake_lsp_path();
+    state
+        .lua_host
+        .lua()
+        .load(format!(
+            "pmacs.lsp.config.rust = {{
+               command = '{fake}',
+               env = {{ PMACS_FAKE_LSP_MODE = 'sighelp' }},
+             }}"
+        ))
+        .exec()
+        .expect("override rust config");
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
+        .exec()
+        .expect("open a.rs");
+    let initialized = "(function() \
+           for _,r in ipairs(pmacs.lsp.list()) do \
+             if r.state and r.state.kind=='initialized' then return true end \
+           end \
+           return false \
+         end)()";
+    assert!(pump_lua_flag(&mut state, initialized, 5), "server init");
+
+    // The first keystroke only seeds the typed-char snapshot; the second
+    // is the trigger. (A trigger char cannot fire off the very first edit
+    // in a buffer, which is correct: there is no prior cursor to compare.)
+    for c in ['f', '('] {
+        state.dispatch_key(
+            pmacs::protocol::FrontendId::LOCAL,
+            KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE),
+        );
+    }
+    let deadline = Instant::now() + Duration::from_secs(5);
+    let mut saw = false;
+    while Instant::now() < deadline {
+        state.tick_processes();
+        state.tick_lsp();
+        state.tick_async();
+        if state.core.borrow().status.contains("fn echo(") {
+            saw = true;
+            break;
+        }
+    }
+    assert!(saw, "typing `(` did not auto-trigger signature help");
+}
+
+/// Arc 1d — an ordinary character does **not** auto-trigger, and neither
+/// does a multi-byte edit (paste/undo/remote): only the one-byte typed
+/// signature does. Guards against a signature request on every keystroke.
+#[test]
+fn arc1d_signature_help_does_not_trigger_on_ordinary_typing() {
+    use pmacs::editor::EditorState;
+
+    let dir = tempfile::tempdir().expect("tempdir");
+    let a_path = dir.path().join("a.rs");
+    std::fs::write(&a_path, b"\n").expect("write a");
+    let a_disp = a_path.display().to_string();
+
+    let mut state = EditorState::new();
+    let fake = fake_lsp_path();
+    state
+        .lua_host
+        .lua()
+        .load(format!(
+            "pmacs.lsp.config.rust = {{
+               command = '{fake}',
+               env = {{ PMACS_FAKE_LSP_MODE = 'sighelp' }},
+             }}"
+        ))
+        .exec()
+        .expect("override rust config");
+    state
+        .lua_host
+        .lua()
+        .load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
+        .exec()
+        .expect("open a.rs");
+    let initialized = "(function() \
+           for _,r in ipairs(pmacs.lsp.list()) do \
+             if r.state and r.state.kind=='initialized' then return true end \
+           end \
+           return false \
+         end)()";
+    assert!(pump_lua_flag(&mut state, initialized, 5), "server init");
+
+    for c in ['f', 'o', 'o'] {
+        state.dispatch_key(
+            pmacs::protocol::FrontendId::LOCAL,
+            KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE),
+        );
+    }
+    let deadline = Instant::now() + Duration::from_secs(2);
+    while Instant::now() < deadline {
+        state.tick_processes();
+        state.tick_lsp();
+        state.tick_async();
+        assert!(
+            !state.core.borrow().status.contains("fn echo("),
+            "ordinary typing must not request signature help"
+        );
+    }
+}
+
 /// T M4.5 — `textDocument/semanticTokens/range` through the Lua
 /// surface. Same decode path as `/full`, scoped to a range; the
 /// fake returns one token.