feat(lsp): fixing LspToPosition via clamping and moving it to helper - #2700
feat(lsp): fixing LspToPosition via clamping and moving it to helper#2700gat0sy wants to merge 1 commit into
Conversation
Greptile SummaryThe PR centralizes bounds-safe LSP text-edit application and expands initialization, workspace, configuration, URI-normalization, and workspace-edit support. The workspace-edit client-selection fix remains broken because its helper references an out-of-scope server and can still fall back to an unrelated plugin.
Confidence Score: 3/5The PR is not safe to merge because workspace edits currently fail on the out-of-scope server reference and can still be mapped through the wrong client after that is corrected. The module-level workspace-edit helper evaluates Files Needing Attention: src/cm/lsp/transport.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Server as Requesting LSP server
participant Transport
participant Workspace
participant View as Target EditorView
participant Other as Unrelated LSP plugin
Server->>Transport: workspace/applyEdit
Transport->>Workspace: getFile/displayFile(uri)
Workspace-->>Transport: target view
Transport->>View: getAll plugins
Transport->>Transport: evaluate server.id
alt server identifier is available
alt requesting server plugin exists
View-->>Transport: matching plugin
else requesting server plugin missing
View-->>Transport: allPlugins[0]
Transport->>Other: map positions and apply edits
end
else server is out of scope
Transport-->>Server: applied false / internal error
end
Reviews (2): Last reviewed commit: "feat(lsp): fixing LspToPosition adding a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
LspToPosition threw range error on format error. We attempt to fix it here with by clamping so we get the correct line count between the client and server. applyTextEdit as also been extracted so both transport and client manager can import it from the helper.
This comment was marked as outdated.
This comment was marked as outdated.
| const plugin = | ||
| allPlugins.find( | ||
| (p) => (p.client as { | ||
| __acodeServerId?: string | ||
| }).__acodeServerId === server.id, | ||
| ) ?? allPlugins[0]; |
There was a problem hiding this comment.
When the target view lacks the plugin belonging to the requesting server, this fallback selects allPlugins[0], causing the edit to be mapped through another client's synchronized document and pending changes and therefore target incorrect offsets.
Knowledge Base Used: LSP Integration
| (p) => (p.client as { | ||
| __acodeServerId?: string | ||
| }).__acodeServerId === server.id, |
There was a problem hiding this comment.
When workspace/applyEdit reaches this module-level helper, server is not in scope, so evaluating server.id throws and the handler returns an internal-error response without applying the workspace edit.
Knowledge Base Used: LSP Integration
2 participants
Fix lspPositionToOffset when LSP servers return EOF positions one line past the document via clamping
Extract lspPositionToOffset() and applyTextEdits() into a shared helper so clientManager and transport use the same bounds-safe logic of the new lspPositionToOffset
Send workspaceFolders when rootUri is available
Declare missing applyEdit, workspaceFolders, and code action resolve capabilities
Re-send workspace/didChangeConfiguration after initialization
Normalize SFTP URIs before sending them to LSP servers ( Assuming the LSP runs on the same machine as the sftp directory )
These fixes improve compatibility with servers that rely on workspace folders, applyEdit, resolvable code actions, post-init configuration, or full-document edits. They may need some polishing.
Note: CodeActions feature times out if more than one lsp are running at the same time when using the built-In LSPs