Stream: contributing

Topic: lsp: suggested improvements/refactors

view this post on Zulip Lukas Juhrich (Aug 01 2026 at 17:16):

Hey folks, I've noticed a few rough edges when trying to add some lsp features.
So I've asked cursor for a list of sensible small refactorings / bugfixes in the LSP.

  1. Does this list make sense? Anything plain wrong in there?
  2. Can I just tackle the first thing (missing textDocument/didClose) and give a PR?

:robot: summary:

experimental-lsp: small hand-offs for immediate UX

Did a read-only pass over src/lsp/. Biggest user pain today isn’t missing features so much as: sync full-check blocking the message loop (Helix close/shutdown timeout), UTF-16 advertised but query paths treating columns as bytes, and a few silent-wrong / silent-hang paths.

Proposed small, shippable refactors — each as a repro-first hand-off (pick one up as AI or human):

Do first (S):

  1. Wire didClose — sticky diags + buffer leak (openClose: true but no handler). Closing a tab never clears the store or publishes empty diagnostics, so squiggles and memory stick around for the session.
  2. Never drop request replies — failed handlers only log → client hangs on that id. Once we know the request id we should always best-effort sendError, otherwise the editor waits until timeout while the server is still alive.
  3. Null = miss, not failure — hover/def map errors to result: null. A real checker failure currently looks identical to “nothing under the cursor,” so you can’t tell broken analysis from an empty hover.

Do next (M, biggest bang):

  1. Unify position math + real UTF-16 — edits correct, queries wrong on non-ASCII. didChange already converts UTF-16 properly, but hover/goto/completion still treat character as a byte offset (and some handlers duplicate that math with a 4096-line cap).
  2. Delete AGENTS-forbidden salvage — text-token hover / wrong-module env / same-text highlights. When CIR or path lookup misses we guess (same name, another package root, textual token match), which can return the wrong answer instead of an honest empty result.
  3. Defer syntax check off the loop — didOpen mustn’t block shutdown. The single-threaded loop runs a full buildResolvingMain inside open/change before it can read the next message, so close/quit times out even though shutdown itself is cheap.

Also known (not separate docs): barefoot JSON walks are factorable in Zig (fromJson already exists in protocol.zig for some types); full coop-cancel is a later step after (6). Shared typed params would cut the copy-paste tax for every new caret feature without changing runtime UX.

Suggested order: 01 → 02/03 → 04 → 06 (parallel 05). After 04 + shared params parsing, new caret features get much cheaper — you mostly write the CIR query, not encoding/JSON plumbing again.

view this post on Zulip Lukas Juhrich (Aug 01 2026 at 17:18):

prompt (if anyone's interested)

Last updated: Aug 12 2026 at 12:35 UTC