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
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.
Greptile SummaryThe PR centralizes bounds-safe LSP text-edit conversion and expands LSP initialization, workspace-edit, URI-normalization, and idle-client behavior.
Confidence Score: 3/5The PR should not merge until server-initiated workspace edits are bound to the originating LSP client and unsupported resource operations are handled or rejected atomically. The new workspace-edit path can map edits through another client's document state in multi-server sessions and can report success after dropping requested file operations. Files Needing Attention: src/cm/lsp/transport.ts, src/cm/lsp/clientManager.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Server as LSP Server
participant Transport
participant View as EditorView
participant Plugin as Selected LSPPlugin
Server->>Transport: workspace/applyEdit
Transport->>View: locate/open target URI
Transport->>Plugin: LSPPlugin.get(view)
Plugin->>Plugin: map positions via syncedDoc/unsyncedChanges
Plugin->>View: dispatch text changes
Transport-->>Server: ApplyWorkspaceEditResponse
Reviews (1): 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}
| const plugin = LSPPlugin.get(view); | ||
| if (!plugin) { | ||
| failures.push(uri); | ||
| continue; | ||
| } | ||
|
|
||
| const applied = applyTextEdits(plugin, view, edits); |
There was a problem hiding this comment.
When multiple LSP clients are attached to a document, LSPPlugin.get(view) does not select the client that sent workspace/applyEdit, so positions are mapped through another client's syncedDoc and unsyncedChanges, causing edits to target incorrect offsets, be skipped, or fail file lookup.
Knowledge Base Used: LSP Integration
| ctx: TransportContext, | ||
| ): Promise<{ applied: boolean; failureReason?: string }> { | ||
| if (!edit) return { applied: false, failureReason: "No edit provided" }; | ||
|
|
||
| const changesByUri: Record<string, TextEdit[]> = | ||
| edit.changes ?? |
There was a problem hiding this comment.
When documentChanges contains CreateFile, RenameFile, or DeleteFile operations, this filter discards them and evaluates success using only the remaining text edits, so the client can return applied: true after executing only part of the requested workspace edit.
Knowledge Base Used: LSP Integration
1 participant
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