feat(lsp): add go-to actions and improve client lifecycle - #2701
Conversation
Greptile SummaryThe PR adds go-to navigation, delays disposal of idle LSP clients, normalizes SFTP document URIs, and introduces handling for server-pushed workspace edits. The repeated-edit fix retains every entry but does not preserve sequential same-document edit semantics.
Confidence Score: 4/5The PR is not yet safe to merge because repeated same-document workspace edits can be applied against the wrong document snapshot. The workspace/applyEdit fix preserves repeated entries by concatenating them, but converting and dispatching all ranges against one pre-edit document loses the ordered state transitions required by later entries. Files Needing Attention: src/cm/lsp/transport.ts, src/cm/lsp/textEditUtils.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Server as LSP Server
participant Transport as workspace/applyEdit handler
participant Utils as applyTextEdits
participant Editor as CodeMirror Editor
Server->>Transport: documentChanges entry 1 for URI
Server->>Transport: documentChanges entry 2 for same URI
Transport->>Transport: Concatenate both edit arrays
Transport->>Utils: Apply combined edits
Utils->>Utils: Resolve every range against syncedDoc
Utils->>Editor: Dispatch one transaction
Note over Utils,Editor: Later-entry ranges do not see entry 1's resulting state
Reviews (2): Last reviewed commit: "feat(lsp): add go-to-definition, code ac..." | 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.
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.
go to def and similar fonction have been added, a new interceptFileLink method has been created to solve an FileUriExposedException you may get if taping the signature link on the hover. if the link is a website, it skips and let the normal behavior occur ( open a web browser page ) if the link is a file, it modifies the uri so the tap behave like a go to instead of crashing the whole app. There are notably also some fixes for code actions, rename...ect, now they use the new lspPostionToOffset that uses clamping
2e7c60b to
8a7ce5b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
| if (edit.documentChanges) { | ||
| for (const change of edit.documentChanges) { | ||
| if ("edits" in change) { | ||
| const uri = change.textDocument.uri; | ||
| changesByUri[uri] = [...(changesByUri[uri] ?? []), | ||
| ...change.edits]; |
There was a problem hiding this comment.
When a server sends multiple ordered documentChanges entries for the same URI and a later entry uses the document state produced by an earlier entry, this code concatenates them and resolves every range against the same plugin.syncedDoc. The later edit therefore modifies the wrong text or overlaps an earlier change and causes the workspace-edit transaction to fail.
Knowledge Base Used: LSP Integration
2 participants
Adds Go to Definition, Declaration, Type Definition, and Implementation to the LSP features.
This PR is based on PR #2700