Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

TextEdit to TextReplacement for Notebook in NES #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8585389
TextEdit to TextReplacement for Notebook in NES
DonJayamanne Jul 28, 2025
80a05c4
Merge branch 'main' into NESNotebookDiag
DonJayamanne Jul 29, 2025
2b66d61
Fix formatting
DonJayamanne Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions ...on/inlineEdits/vscode-node/features/diagnosticsBasedCompletions/diagnosticsCompletions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Range } from '../../../../../util/vs/editor/common/core/range';
import { OffsetRange } from '../../../../../util/vs/editor/common/core/ranges/offsetRange';
import { INextEditDisplayLocation } from '../../../node/nextEditResult';
import { IVSCodeObservableDocument, IVSCodeObservableNotebookDocument, IVSCodeObservableTextDocument } from '../../parts/vscodeWorkspace';
import { coalesce } from '../../../../../util/vs/base/common/arrays';

export interface IDiagnosticCodeAction {
edit: TextReplacement;
Expand Down Expand Up @@ -312,10 +313,21 @@ export class CodeAction {
}

getEditForWorkspaceDocument(workspaceDocument: IVSCodeObservableDocument): TextReplacement[] | undefined {
if (!this.edit) {
const edit = this.edit;
if (!edit) {
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));
Copy link
Collaborator Author

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);

Copy link
Collaborator Author

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.

Copy link
Member

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.

} else if (workspaceDocument.kind === 'notebookDocument') {
const edits = coalesce(workspaceDocument.notebook.getCells().flatMap(cell => {
return edit.get(cell.document.uri).map(e => {
const range = workspaceDocument.toRange(cell.document, e.range);
return range ? toInternalTextEdit(range, e.newText) : undefined;
});
}));
return edits.length ? edits : undefined;
}
}

getDiagnosticsReferencedInCommand(): Diagnostic[] {
Expand Down Expand Up @@ -377,8 +389,8 @@ export function toExternalPosition(position: Position): vscode.Position {
return new vscode.Position(position.lineNumber - 1, position.column - 1);
}

export function toInternalTextEdit(edit: vscode.TextEdit): TextReplacement {
return new TextReplacement(toInternalRange(edit.range), edit.newText);
export function toInternalTextEdit(range: vscode.Range, newText: string): TextReplacement {
return new TextReplacement(toInternalRange(range), newText);
}

export function toExternalTextEdit(edit: TextReplacement): vscode.TextEdit {
Expand Down
21 changes: 15 additions & 6 deletions src/extension/inlineEdits/vscode-node/features/diagnosticsCompletionProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { AnyDiagnosticCompletionItem, AnyDiagnosticCompletionProvider } from './
import { AsyncDiagnosticCompletionProvider } from './diagnosticsBasedCompletions/asyncDiagnosticsCompletionProvider';
import { Diagnostic, DiagnosticCompletionItem, DiagnosticInlineEditRequestLogContext, DiagnosticSeverity, distanceToClosestDiagnostic, IDiagnosticCompletionProvider, log, logList, sortDiagnosticsByDistance, toInternalPosition } from './diagnosticsBasedCompletions/diagnosticsCompletions';
import { ImportDiagnosticCompletionItem, ImportDiagnosticCompletionProvider } from './diagnosticsBasedCompletions/importDiagnosticsCompletionProvider';
import { isNotebookCell } from '../../../../util/common/notebooks';

interface IDiagnosticsCompletionState<T extends DiagnosticCompletionItem = DiagnosticCompletionItem> {
completionItem: T | null;
Expand Down Expand Up @@ -226,9 +227,13 @@ export class DiagnosticsCompletionProcessor extends Disposable {

this._rejectionCollector = new RejectionCollector(this._workspace, s => this._tracer.trace(s));

const isValidEditor = (editor: vscode.TextEditor | undefined): editor is vscode.TextEditor => {
return !!editor && (isNotebookCell(editor.document.uri) || isEditorFromEditorGrid(editor));
};

this._register(this._languageDiagnosticsService.onDidChangeDiagnostics(async e => {
const activeEditor = this._tabsAndEditorsService.activeTextEditor;
if (!activeEditor || !isEditorFromEditorGrid(activeEditor)) {
if (!isValidEditor(activeEditor)) {
return;
}

Expand All @@ -241,8 +246,7 @@ export class DiagnosticsCompletionProcessor extends Disposable {
}));

this._register(this._tabsAndEditorsService.onDidChangeActiveTextEditor(async e => {
const activeEditor = e;
if (!activeEditor || !isEditorFromEditorGrid(activeEditor)) {
if (!isValidEditor(e)) {
return;
}

Expand All @@ -251,7 +255,7 @@ export class DiagnosticsCompletionProcessor extends Disposable {

this._register(vscode.window.onDidChangeTextEditorSelection(async e => {
const activeEditor = this._tabsAndEditorsService.activeTextEditor;
if (!activeEditor || !isEditorFromEditorGrid(activeEditor)) {
if (!isValidEditor(activeEditor)) {
return;
}

Expand Down Expand Up @@ -291,9 +295,14 @@ export class DiagnosticsCompletionProcessor extends Disposable {
const workspaceDocument = this._workspace.getDocumentByTextDocument(activeTextEditor.document);
if (!workspaceDocument) { return; }

const log = new DiagnosticInlineEditRequestLogContext();
const range = new vscode.Range(activeTextEditor.selection.active, activeTextEditor.selection.active);
const selection = workspaceDocument.toRange(activeTextEditor.document, range);
if (!selection) {
return;
}

const cursor = toInternalPosition(activeTextEditor.selection.active);
const cursor = toInternalPosition(selection.start);
const log = new DiagnosticInlineEditRequestLogContext();

const { availableDiagnostics, relevantDiagnostics } = this._getDiagnostics(workspaceDocument, cursor, log);
const diagnosticsSorted = sortDiagnosticsByDistance(relevantDiagnostics, cursor);
Expand Down
15 changes: 14 additions & 1 deletion src/extension/inlineEdits/vscode-node/parts/vscodeWorkspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ export interface IVSCodeObservableTextDocument extends IObservableDocument {
fromOffsetRange(textDocument: TextDocument, range: OffsetRange): Range | undefined;
fromRange(textDocument: TextDocument, range: Range): Range | undefined;
toOffsetRange(textDocument: TextDocument, range: Range): OffsetRange | undefined;
toRange(textDocument: TextDocument, range: Range): Range | undefined;
}

abstract class AbstractVSCodeObservableDocument {
Expand Down Expand Up @@ -445,6 +446,10 @@ class VSCodeObservableTextDocument extends AbstractVSCodeObservableDocument impl
return new OffsetRange(textDocument.offsetAt(range.start), textDocument.offsetAt(range.end));
}

toRange(_textDocument: TextDocument, range: Range): Range | undefined {
return range;
}

fromRange(_textDocument: TextDocument, range: Range): Range | undefined {
return range;
}
Expand All @@ -466,6 +471,7 @@ export interface IVSCodeObservableNotebookDocument extends IObservableDocument {
fromRange(textDocument: TextDocument, range: Range): Range | undefined;
fromRange(range: Range): [TextDocument, Range][];
projectDiagnostics(cell: TextDocument, diagnostics: readonly Diagnostic[]): Diagnostic[];
toRange(textDocument: TextDocument, range: Range): Range | undefined;
}

class VSCodeObservableNotebookDocument extends AbstractVSCodeObservableDocument implements IVSCodeObservableNotebookDocument {
Expand Down Expand Up @@ -523,6 +529,14 @@ class VSCodeObservableNotebookDocument extends AbstractVSCodeObservableDocument
}
return toAltDiagnostics(this.altNotebook, cell, diagnostics);
}
toRange(textDocument: TextDocument, range: Range): Range | undefined {
const cell = this.altNotebook.getCell(textDocument);
if (!cell) {
return undefined;
}
const ranges = this.altNotebook.toAltRange(cell, [range]);
return ranges.length > 0 ? ranges[0] : undefined;
}
}

export type IVSCodeObservableDocument = IVSCodeObservableTextDocument | IVSCodeObservableNotebookDocument;
Expand Down Expand Up @@ -691,4 +705,3 @@ export function editFromTextDocumentContentChangeEvents(events: readonly TextDoc
const replacementsInApplicationOrder = events.map(e => StringReplacement.replace(OffsetRange.ofStartAndLength(e.rangeOffset, e.rangeLength), e.text));
return StringEdit.composeSequentialReplacements(replacementsInApplicationOrder);
}