LSP: no-op middleware onDefinition replaces the real definition handler

[nealrichardson]

Bug

In apps/lsp/src/index.ts, the server registers a real definition handler, and then calls middlewareRegister(connection), which registers a second onDefinition handler that returns null:

// apps/lsp/src/index.ts:186
connection.onDefinition(async (params, token) => {
  ...
  return mdLs?.getDefinition(document, params.position, token);
});

// apps/lsp/src/index.ts:197
middlewareRegister(connection);

// apps/lsp/src/middleware.ts:46
connection.onDefinition(async () => {
  return null;
});

In vscode-jsonrpc, registering a request handler replaces the previous one for that method, so the no-op wins. The Quarto LSP's own definition provider (apps/lsp/src/service/providers/definitions.ts) is never called.

Impact

On the client, embeddedGoToDefinitionProvider (apps/vscode/src/lsp/client.ts:476) handles positions inside code cells, and otherwise calls next(...). That falls through to the server, which now always returns null. So in .qmd prose, Go to Definition is broken for:

• reference-style links → their [ref]: url definition
• headers (the provider returns the header's own location)

(VS Code's built-in markdown extension may cover some of this for .md, but not for .qmd.)

History

• 3d4ee144 (2023-02-08) added the no-op onDefinition to enable client middleware for code cells.
• 21e441ed (2023-05-21) consolidated the no-ops into middleware.ts.
• 07129a6a (2023-05-22), "integrate quarto lsp w/ mdls lsp", added the real handler in index.ts. It didn't remove the no-op, which is registered afterwards.

So it looks unintentional: the no-op existed only to advertise the capability, which the real handler now does on its own.

Fix

Remove the onDefinition handler and definitionProvider: true from middleware.ts. index.ts already advertises definitionProvider: true and registers the real handler, and client middleware still intercepts it. Then check that Go to Definition works both inside code cells (via the virtual doc) and on a reference link / header in prose.

It's worth checking the other no-ops in middleware.ts for the same pattern. Right now only definition has a real handler too.

Found during the dependency-upgrade research.