-
Notifications
You must be signed in to change notification settings - Fork 0
fix handling of multiple sequential edits #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
38047fe
e5fdedd
fffe077
e1bcfab
c4e1fe2
b615a94
14d5c39
cbc2dfe
21a467c
e1eeb87
c895c9d
e4921fe
acc1f22
d6f2245
5cf052d
094b488
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| import { Edit, Point } from 'tree-sitter'; | ||
| import { Point } from 'tree-sitter'; | ||
| import { DidChangeTextDocumentParams } from 'vscode-languageserver'; | ||
| import { TextDocumentChangeEvent } from 'vscode-languageserver/lib/common/textDocuments'; | ||
| import { NotificationHandler } from 'vscode-languageserver-protocol'; | ||
| import { TextDocument } from 'vscode-languageserver-textdocument'; | ||
| import { SyntaxTreeManager } from '../context/syntaxtree/SyntaxTreeManager'; | ||
| import { CloudFormationFileType, Document } from '../document/Document'; | ||
| import { createEdit } from '../document/DocumentUtils'; | ||
| import { LspDocuments } from '../protocol/LspDocuments'; | ||
|
|
@@ -35,26 +34,9 @@ export function didOpenHandler(components: ServerComponents): (event: TextDocume | |
| } | ||
| } | ||
|
|
||
| components.cfnLintService.lintDelayed(content, uri, LintTrigger.OnOpen).catch((reason) => { | ||
| // Handle cancellation gracefully - user might have closed/changed the document | ||
| if (reason instanceof CancellationError) { | ||
| // Do nothing - cancellation is expected behavior | ||
| } else { | ||
| log.error(reason, `Linting error for ${uri}`); | ||
| } | ||
| }); | ||
| triggerValidation(components, content, uri, LintTrigger.OnOpen, ValidationTrigger.OnOpen); | ||
|
|
||
| // Trigger Guard validation | ||
| components.guardService.validateDelayed(content, uri, ValidationTrigger.OnOpen).catch((reason) => { | ||
| // Handle cancellation gracefully - user might have closed/changed the document | ||
| if (reason instanceof Error && reason.message.includes('Request cancelled')) { | ||
| // Do nothing | ||
| } else { | ||
| log.error(reason, `Guard validation error for ${uri}`); | ||
| } | ||
| }); | ||
|
|
||
| components.documentManager.sendDocumentMetadata(0); | ||
| components.documentManager.sendDocumentMetadata(); | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -64,66 +46,72 @@ export function didChangeHandler( | |
| ): NotificationHandler<DidChangeTextDocumentParams> { | ||
| return (params) => { | ||
| const documentUri = params.textDocument.uri; | ||
| const version = params.textDocument.version; | ||
| const textDocument = documents.documents.get(documentUri); | ||
|
|
||
| if (!textDocument) { | ||
| log.error(`No document found for file with changes ${documentUri}`); | ||
| return; | ||
| } | ||
|
|
||
| const content = textDocument.getText(); | ||
| const changes = params.contentChanges; | ||
| try { | ||
| let hasFullDocumentChange = false; | ||
| for (const change of changes) { | ||
| if ('range' in change) { | ||
| // This is an incremental change with a specific range | ||
| const start: Point = { | ||
| row: change.range.start.line, | ||
| column: change.range.start.character, | ||
| }; | ||
| const end: Point = { | ||
| row: change.range.end.line, | ||
| column: change.range.end.character, | ||
| }; | ||
|
|
||
| const { edit } = createEdit(content, change.text, start, end); | ||
| updateSyntaxTree(components.syntaxTreeManager, textDocument, edit); | ||
| } else { | ||
| hasFullDocumentChange = true; | ||
| } | ||
| } | ||
| // This is the document AFTER changes | ||
| const document = new Document(textDocument); | ||
| const finalContent = document.getText(); | ||
|
|
||
| if (hasFullDocumentChange) { | ||
| components.syntaxTreeManager.add(documentUri, content); | ||
| const tree = components.syntaxTreeManager.getSyntaxTree(documentUri); | ||
|
|
||
| // Short-circuit if this is not a template (anymore) | ||
| if (document.cfnFileType === CloudFormationFileType.Other) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our current detection isn't perfect and we have lots of Completion tests that expect valid completions on templates even when |
||
| if (tree) { | ||
| // Clean-up if was but no longer is a template | ||
| components.syntaxTreeManager.deleteSyntaxTree(documentUri); | ||
| } | ||
| } catch (error) { | ||
| log.error(error, `Error updating tree ${documentUri}`); | ||
| // Create a new tree if partial updates fail | ||
| components.syntaxTreeManager.add(documentUri, content); | ||
| components.documentManager.sendDocumentMetadata(); | ||
| return; | ||
| } | ||
|
|
||
| // Trigger cfn-lint validation | ||
| components.cfnLintService.lintDelayed(content, documentUri, LintTrigger.OnChange, true).catch((reason) => { | ||
| // Handle both getTextDocument and linting errors | ||
| if (reason instanceof CancellationError) { | ||
| // Do nothing - cancellation is expected behavior | ||
| } else { | ||
| log.error(reason, `Error in didChange processing for ${documentUri}`); | ||
| if (tree) { | ||
| // This starts as the text BEFORE changes | ||
| let currentContent = tree.content(); | ||
| try { | ||
| const changes = params.contentChanges; | ||
| for (const change of changes) { | ||
| if ('range' in change) { | ||
| // Incremental change | ||
| const start: Point = { | ||
| row: change.range.start.line, | ||
| column: change.range.start.character, | ||
| }; | ||
| const end: Point = { | ||
| row: change.range.end.line, | ||
| column: change.range.end.character, | ||
| }; | ||
| const { edit, newContent } = createEdit(currentContent, change.text, start, end); | ||
| components.syntaxTreeManager.updateWithEdit(documentUri, newContent, edit); | ||
| currentContent = newContent; | ||
| } else { | ||
| // Full document change | ||
| components.syntaxTreeManager.add(documentUri, change.text); | ||
| currentContent = change.text; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| log.error({ error, uri: documentUri, version }, 'Error updating tree - recreating'); | ||
| components.syntaxTreeManager.add(documentUri, finalContent); | ||
| } | ||
| }); | ||
| } else { | ||
| // If we don't have a tree yet, just parse the final document | ||
| components.syntaxTreeManager.add(documentUri, finalContent); | ||
| } | ||
|
|
||
| // Trigger Guard validation | ||
| components.guardService | ||
| .validateDelayed(content, documentUri, ValidationTrigger.OnChange, true) | ||
| .catch((reason) => { | ||
| // Handle both getTextDocument and validation errors | ||
| if (reason instanceof Error && reason.message.includes('Request cancelled')) { | ||
| // Do nothing | ||
| } else { | ||
| log.error(reason, `Error in Guard didChange processing for ${documentUri}`); | ||
| } | ||
| }); | ||
| triggerValidation( | ||
| components, | ||
| finalContent, | ||
| documentUri, | ||
| LintTrigger.OnChange, | ||
| ValidationTrigger.OnChange, | ||
| true, | ||
| ); | ||
|
|
||
| // Republish validation diagnostics if available | ||
| const validationDetails = components.validationManager | ||
|
|
@@ -171,40 +159,33 @@ export function didSaveHandler(components: ServerComponents): (event: TextDocume | |
| const documentUri = event.document.uri; | ||
| const documentContent = event.document.getText(); | ||
|
|
||
| // Trigger cfn-lint validation | ||
| components.cfnLintService.lintDelayed(documentContent, documentUri, LintTrigger.OnSave).catch((reason) => { | ||
| if (reason instanceof CancellationError) { | ||
| // Do nothing - cancellation is expected behavior | ||
| } else { | ||
| log.error(reason, `Linting error for ${documentUri}`); | ||
| } | ||
| }); | ||
|
|
||
| // Trigger Guard validation | ||
| components.guardService | ||
| .validateDelayed(documentContent, documentUri, ValidationTrigger.OnSave) | ||
| .catch((reason) => { | ||
| if (reason instanceof Error && reason.message.includes('Request cancelled')) { | ||
| // Do nothing | ||
| } else { | ||
| log.error(reason, `Guard validation error for ${documentUri}`); | ||
| } | ||
| }); | ||
| triggerValidation(components, documentContent, documentUri, LintTrigger.OnSave, ValidationTrigger.OnSave); | ||
|
|
||
| components.documentManager.sendDocumentMetadata(0); | ||
| }; | ||
| } | ||
|
|
||
| function updateSyntaxTree(syntaxTreeManager: SyntaxTreeManager, textDocument: TextDocument, edit: Edit) { | ||
| const uri = textDocument.uri; | ||
| const document = new Document(textDocument); | ||
| if (syntaxTreeManager.getSyntaxTree(uri)) { | ||
| if (document.cfnFileType === CloudFormationFileType.Other) { | ||
| syntaxTreeManager.deleteSyntaxTree(uri); | ||
| function triggerValidation( | ||
| components: ServerComponents, | ||
| content: string, | ||
| uri: string, | ||
| lintTrigger: LintTrigger, | ||
| validationTrigger: ValidationTrigger, | ||
| debounce?: boolean, | ||
| ): void { | ||
| components.cfnLintService.lintDelayed(content, uri, lintTrigger, debounce).catch((reason) => { | ||
| if (reason instanceof CancellationError) { | ||
| // Do nothing - cancellation is expected behavior | ||
| } else { | ||
| log.error(reason, `Linting error for ${uri}`); | ||
| } | ||
| }); | ||
|
|
||
| components.guardService.validateDelayed(content, uri, validationTrigger, debounce).catch((reason) => { | ||
| if (reason instanceof Error && reason.message.includes('Request cancelled')) { | ||
| // Do nothing | ||
| } else { | ||
| syntaxTreeManager.updateWithEdit(uri, document.contents(), edit); | ||
| log.error(reason, `Guard validation error for ${uri}`); | ||
| } | ||
| } else { | ||
| syntaxTreeManager.addWithTypes(uri, document.contents(), document.documentType, document.cfnFileType); | ||
| } | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.