-
Notifications
You must be signed in to change notification settings - Fork 1k
TextEdit to TextReplacement for Notebook in NES #377
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?
Conversation
return undefined; | ||
} | ||
return this.edit.get(workspaceDocument.id.toUri()).map(toInternalTextEdit); | ||
if (workspaceDocument.kind === 'textDocument') { | ||
return edit.get(workspaceDocument.id.toUri()).map(e => toInternalTextEdit(e.range, e.newText)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very similar to
I think the best is to create a separate file with these helpers that just accepts the IVSCodeObservable.
E.g. toDiagnostics(vscode.Diagnostic): Diagnostic
toTextReplacement(vscode.TextEdit): TextReplacement
etc
vscode-copilot-chat/src/extension/inlineEdits/vscode-node/features/diagnosticsCompletionProcessor.ts
Lines 311 to 320 in 1492647
const diagnostics = workspaceDocument.kind === 'textDocument' ? | |
this._languageDiagnosticsService | |
.getDiagnostics(workspaceDocument.textDocument.uri) : | |
workspaceDocument.notebook.getCells().flatMap(cell => this._languageDiagnosticsService | |
.getDiagnostics(cell.document.uri) | |
.flatMap(diagnostic => workspaceDocument.projectDiagnostics(cell.document, [diagnostic]))); | |
const availableDiagnostics = diagnostics | |
.map(diagnostic => Diagnostic.fromVSCodeDiagnostic(diagnostic)) | |
.filter(diagnostic => diagnostic.severity !== DiagnosticSeverity.Information) | |
.filter(diagnostic => diagnostic.severity !== DiagnosticSeverity.Hint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be others,
I want to first get an idea of what all of these additional changes are, before refactoring these or introducing other layers/helpers.
Yes this is just building up debt, but will fix these just as i did earlier with few others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe IVSCodeDocument
should offer a nice abstraction here - consumers (such as this one) shouldn't care if its a notebook or normal text document and the API should be such that they cannot use it wrongly easily. That abstraction should offer diagnostics and fixes for IObservableDocuments.
I believe kind
should disappear with that debt-week cleanup.
No description provided.