Handle notebook cell editors getting their models swapped resulting in weird Editor viewzone issues (#234236)

* Fixes to learking notebook cell viewzones

* More fixes

* Misc
This commit is contained in:
Don Jayamanne 2024-11-20 15:10:36 +11:00 коммит произвёл GitHub
Родитель 864bce76f2
Коммит 8a4b2bb49b
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
4 изменённых файлов: 207 добавлений и 83 удалений

Просмотреть файл

@ -5,7 +5,7 @@
import { isEqual } from '../../../../../../base/common/resources.js';
import { Disposable, DisposableStore, dispose, toDisposable } from '../../../../../../base/common/lifecycle.js';
import { autorun, derived } from '../../../../../../base/common/observable.js';
import { autorun, autorunWithStore, derived, observableFromEvent } from '../../../../../../base/common/observable.js';
import { IChatEditingService, ChatEditingSessionState } from '../../../../chat/common/chatEditingService.js';
import { NotebookTextModel } from '../../../common/model/notebookTextModel.js';
import { INotebookEditor } from '../../notebookBrowser.js';
@ -31,15 +31,17 @@ import { createTrustedTypesPolicy } from '../../../../../../base/browser/trusted
import { splitLines } from '../../../../../../base/common/strings.js';
import { DefaultLineHeight } from '../../diff/diffElementViewModel.js';
import { INotebookOriginalCellModelFactory } from './notebookOriginalCellModelFactory.js';
import { DetailedLineRangeMapping } from '../../../../../../editor/common/diff/rangeMapping.js';
export class NotebookCellDiffDecorator extends DisposableStore {
private readonly _decorations = this.editor.createDecorationsCollection();
private _viewZones: string[] = [];
private readonly throttledDecorator = new ThrottledDelayer(100);
private readonly throttledDecorator = new ThrottledDelayer(50);
private diffForPreviouslyAppliedDecorators?: IDocumentDiff;
private readonly perEditorDisposables = this.add(new DisposableStore());
constructor(
public readonly editor: ICodeEditor,
notebookEditor: INotebookEditor,
public readonly modifiedCell: NotebookCellTextModel,
public readonly originalCell: NotebookCellTextModel,
@IChatEditingService private readonly _chatEditingService: IChatEditingService,
@ -48,42 +50,74 @@ export class NotebookCellDiffDecorator extends DisposableStore {
) {
super();
this.add(this.editor.onDidChangeModel(() => {
this._clearRendering();
this.update();
}));
this.add(this.editor.onDidChangeModelContent(() => this.update()));
this.add(this.editor.onDidChangeConfiguration((e) => {
if (e.hasChanged(EditorOption.fontInfo) || e.hasChanged(EditorOption.lineHeight)) {
this.update();
const onDidChangeVisibleRanges = observableFromEvent(notebookEditor.onDidChangeVisibleRanges, () => notebookEditor.visibleRanges);
const editorObs = derived((r) => {
const visibleRanges = onDidChangeVisibleRanges.read(r);
const visibleCellHandles = visibleRanges.map(range => notebookEditor.getCellsInRange(range)).flat().map(c => c.handle);
if (!visibleCellHandles.includes(modifiedCell.handle)) {
return;
}
const editor = notebookEditor.codeEditors.find(item => item[0].handle === modifiedCell.handle)?.[1];
if (editor?.getModel() !== this.modifiedCell.textModel) {
return;
}
return editor;
});
this.add(autorunWithStore((r, store) => {
const editor = editorObs.read(r);
this.perEditorDisposables.clear();
if (editor) {
store.add(editor.onDidChangeModel(() => {
this.perEditorDisposables.clear();
}));
store.add(editor.onDidChangeModelContent(() => {
this.update(editor);
}));
store.add(editor.onDidChangeConfiguration((e) => {
if (e.hasChanged(EditorOption.fontInfo) || e.hasChanged(EditorOption.lineHeight)) {
this.update(editor);
}
}));
this.update(editor);
}
}));
const shouldBeReadOnly = derived(this, r => {
const editor = editorObs.read(r);
if (!editor) {
return false;
}
const value = this._chatEditingService.currentEditingSessionObs.read(r);
if (!value || value.state.read(r) !== ChatEditingSessionState.StreamingEdits) {
return false;
}
return value.entries.read(r).some(e => isEqual(e.modifiedURI, this.editor.getModel()?.uri));
return value.entries.read(r).some(e => isEqual(e.modifiedURI, editor.getModel()?.uri));
});
let actualReadonly: boolean | undefined;
let actualDeco: 'off' | 'editable' | 'on' | undefined;
this.add(autorun(r => {
this.add(autorun((r) => {
const editor = editorObs.read(r);
if (!editor) {
return;
}
const value = shouldBeReadOnly.read(r);
if (value) {
actualReadonly ??= this.editor.getOption(EditorOption.readOnly);
actualDeco ??= this.editor.getOption(EditorOption.renderValidationDecorations);
actualReadonly ??= editor.getOption(EditorOption.readOnly);
actualDeco ??= editor.getOption(EditorOption.renderValidationDecorations);
this.editor.updateOptions({
editor.updateOptions({
readOnly: true,
renderValidationDecorations: 'off'
});
} else {
if (actualReadonly !== undefined && actualDeco !== undefined) {
this.editor.updateOptions({
editor.updateOptions({
readOnly: actualReadonly,
renderValidationDecorations: actualDeco
});
@ -92,35 +126,29 @@ export class NotebookCellDiffDecorator extends DisposableStore {
}
}
}));
this.update();
}
override dispose(): void {
this._clearRendering();
super.dispose();
public update(editor: ICodeEditor): void {
this.throttledDecorator.trigger(() => this._updateImpl(editor));
}
public update(): void {
this.throttledDecorator.trigger(() => this._updateImpl());
}
private async _updateImpl() {
private async _updateImpl(editor: ICodeEditor) {
if (this.isDisposed) {
return;
}
if (this.editor.getOption(EditorOption.inDiffEditor)) {
this._clearRendering();
if (editor.getOption(EditorOption.inDiffEditor)) {
this.perEditorDisposables.clear();
return;
}
const model = this.editor.getModel();
if (!model) {
this._clearRendering();
const model = editor.getModel();
if (!model || model !== this.modifiedCell.textModel) {
this.perEditorDisposables.clear();
return;
}
const originalModel = this.getOrCreateOriginalModel();
const originalModel = this.getOrCreateOriginalModel(editor);
if (!originalModel) {
this._clearRendering();
this.perEditorDisposables.clear();
return;
}
const version = model.getVersionId();
@ -131,31 +159,23 @@ export class NotebookCellDiffDecorator extends DisposableStore {
'advanced'
);
if (this.isDisposed) {
return;
}
if (diff && originalModel && model === this.editor.getModel() && this.editor.getModel()?.getVersionId() === version) {
this._updateWithDiff(originalModel, diff);
if (diff && !diff.identical && originalModel && model === editor.getModel() && editor.getModel()?.getVersionId() === version) {
this._updateWithDiff(editor, originalModel, diff);
} else {
this._clearRendering();
this.perEditorDisposables.clear();
}
}
private _clearRendering() {
this.editor.changeViewZones((viewZoneChangeAccessor) => {
for (const id of this._viewZones) {
viewZoneChangeAccessor.removeZone(id);
}
});
this._viewZones = [];
this._decorations.clear();
}
private _originalModel?: ITextModel;
private getOrCreateOriginalModel() {
private getOrCreateOriginalModel(editor: ICodeEditor) {
if (!this._originalModel) {
const model = this.editor.getModel();
const model = editor.getModel();
if (!model) {
return;
}
@ -164,7 +184,24 @@ export class NotebookCellDiffDecorator extends DisposableStore {
return this._originalModel;
}
private _updateWithDiff(originalModel: ITextModel | undefined, diff: IDocumentDiff): void {
private _updateWithDiff(editor: ICodeEditor, originalModel: ITextModel | undefined, diff: IDocumentDiff): void {
if (areDiffsEqual(diff, this.diffForPreviouslyAppliedDecorators)) {
return;
}
const decorations = editor.createDecorationsCollection();
this.perEditorDisposables.add(toDisposable(() => {
editor.changeViewZones((viewZoneChangeAccessor) => {
for (const id of this._viewZones) {
viewZoneChangeAccessor.removeZone(id);
}
});
this._viewZones = [];
decorations.clear();
this.diffForPreviouslyAppliedDecorators = undefined;
}));
this.diffForPreviouslyAppliedDecorators = diff;
const chatDiffAddDecoration = ModelDecorationOptions.createDynamic({
...diffAddDecoration,
stickiness: TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges
@ -184,7 +221,7 @@ export class NotebookCellDiffDecorator extends DisposableStore {
const addedDecoration = createOverviewDecoration(overviewRulerAddedForeground, minimapGutterAddedBackground);
const deletedDecoration = createOverviewDecoration(overviewRulerDeletedForeground, minimapGutterDeletedBackground);
this.editor.changeViewZones((viewZoneChangeAccessor) => {
editor.changeViewZones((viewZoneChangeAccessor) => {
for (const id of this._viewZones) {
viewZoneChangeAccessor.removeZone(id);
}
@ -192,7 +229,7 @@ export class NotebookCellDiffDecorator extends DisposableStore {
const modifiedDecorations: IModelDeltaDecoration[] = [];
const mightContainNonBasicASCII = originalModel?.mightContainNonBasicASCII();
const mightContainRTL = originalModel?.mightContainRTL();
const renderOptions = RenderOptions.fromEditor(this.editor);
const renderOptions = RenderOptions.fromEditor(editor);
for (const diffEntry of diff.changes) {
const originalRange = diffEntry.original;
@ -258,7 +295,7 @@ export class NotebookCellDiffDecorator extends DisposableStore {
}
}
this._decorations.set(modifiedDecorations);
decorations.set(modifiedDecorations);
});
}
}
@ -370,12 +407,12 @@ export class NotebookDeletedCellDecorator extends Disposable implements INoteboo
return height;
}));
Array.from(this.deletedCellInfos.keys()).sort().forEach((originalIndex) => {
Array.from(this.deletedCellInfos.keys()).sort((a, b) => a - b).forEach((originalIndex) => {
const previousDeletedCell = this.deletedCellInfos.get(originalIndex - 1);
if (previousDeletedCell) {
const deletedCell = this.deletedCellInfos.get(originalIndex);
if (deletedCell) {
deletedCell.offset = previousDeletedCell.height;
deletedCell.offset = previousDeletedCell.height + previousDeletedCell.offset;
}
}
});
@ -477,3 +514,72 @@ export class NotebookDeletedCellWidget extends Disposable {
return totalHeight;
}
}
function areDiffsEqual(a: IDocumentDiff | undefined, b: IDocumentDiff | undefined): boolean {
if (a && b) {
if (a.changes.length !== b.changes.length) {
return false;
}
if (a.moves.length !== b.moves.length) {
return false;
}
if (!areLineRangeMappinsEqual(a.changes, b.changes)) {
return false;
}
if (!a.moves.some((move, i) => {
const bMove = b.moves[i];
if (!areLineRangeMappinsEqual(move.changes, bMove.changes)) {
return true;
}
if (move.lineRangeMapping.changedLineCount !== bMove.lineRangeMapping.changedLineCount) {
return true;
}
if (!move.lineRangeMapping.modified.equals(bMove.lineRangeMapping.modified)) {
return true;
}
if (!move.lineRangeMapping.original.equals(bMove.lineRangeMapping.original)) {
return true;
}
return false;
})) {
return false;
}
return true;
} else if (!a && !b) {
return true;
} else {
return false;
}
}
function areLineRangeMappinsEqual(a: readonly DetailedLineRangeMapping[], b: readonly DetailedLineRangeMapping[]): boolean {
if (a.length !== b.length) {
return false;
}
if (a.some((c, i) => {
const bChange = b[i];
if (c.changedLineCount !== bChange.changedLineCount) {
return true;
}
if ((c.innerChanges || []).length !== (bChange.innerChanges || []).length) {
return true;
}
if ((c.innerChanges || []).some((innerC, innerIdx) => {
const bInnerC = bChange.innerChanges![innerIdx];
if (!innerC.modifiedRange.equalsRange(bInnerC.modifiedRange)) {
return true;
}
if (!innerC.originalRange.equalsRange(bInnerC.originalRange)) {
return true;
}
return false;
})) {
return true;
}
return false;
})) {
return false;
}
return true;
}

Просмотреть файл

@ -178,7 +178,7 @@ class NextPreviousChangeActionRunner extends ActionRunner {
const viewModel = this.notebookEditor.getViewModel();
const activeCell = this.notebookEditor.activeCellAndCodeEditor;
const cellDiff = this.cellDiffInfo.read(undefined);
if (!viewModel || !activeCell || !cellDiff || !cellDiff.length) {
if (!viewModel || !cellDiff?.length || (!activeCell && this.focusedDiff.read(undefined))) {
return this.goToNextEntry();
}

Просмотреть файл

@ -63,7 +63,7 @@ class NotebookChatEditorController extends Disposable {
const notebookModel = observableFromEvent(this.notebookEditor.onDidChangeModel, e => e);
const originalModel = observableValue<NotebookTextModel | undefined>('originalModel', undefined);
const viewModelAttached = observableFromEvent(this.notebookEditor.onDidAttachViewModel, () => !!this.notebookEditor.getViewModel());
const onDidChangeVisibleRanges = debouncedObservable2(observableFromEvent(this.notebookEditor.onDidChangeVisibleRanges, () => this.notebookEditor.visibleRanges), 100);
const onDidChangeVisibleRanges = debouncedObservable2(observableFromEvent(this.notebookEditor.onDidChangeVisibleRanges, () => this.notebookEditor.visibleRanges), 50);
const decorators = new Map<NotebookCellTextModel, NotebookCellDiffDecorator>();
let updatedCellDecoratorsOnceBefore = false;
@ -154,32 +154,36 @@ class NotebookChatEditorController extends Disposable {
}
updatedCellDecoratorsOnceBefore = true;
const validDiffDecorators = new Set<NotebookCellDiffDecorator>();
diffInfo.cellDiff.forEach((diff) => {
if (diff.type === 'modified') {
const modifiedCell = modified.cells[diff.modifiedCellIndex];
const originalCell = original.cells[diff.originalCellIndex];
const editor = this.notebookEditor.codeEditors.find(([vm,]) => vm.handle === modifiedCell.handle)?.[1];
if (editor && (decorators.get(modifiedCell)?.editor !== editor ||
decorators.get(modifiedCell)?.modifiedCell !== modifiedCell ||
decorators.get(modifiedCell)?.originalCell !== originalCell)) {
decorators.get(modifiedCell)?.dispose();
const decorator = this.instantiationService.createInstance(NotebookCellDiffDecorator, editor, modifiedCell, originalCell);
decorators.set(modifiedCell, decorator);
this._register(editor.onDidDispose(() => {
decorator.dispose();
if (decorators.get(modifiedCell) === decorator) {
decorators.delete(modifiedCell);
}
}));
if (editor) {
const currentDecorator = decorators.get(modifiedCell);
if ((currentDecorator?.modifiedCell !== modifiedCell || currentDecorator?.originalCell !== originalCell)) {
currentDecorator?.dispose();
const decorator = this.instantiationService.createInstance(NotebookCellDiffDecorator, notebookEditor, modifiedCell, originalCell);
decorators.set(modifiedCell, decorator);
validDiffDecorators.add(decorator);
this._register(editor.onDidDispose(() => {
decorator.dispose();
if (decorators.get(modifiedCell) === decorator) {
decorators.delete(modifiedCell);
}
}));
} else if (currentDecorator) {
validDiffDecorators.add(currentDecorator);
}
}
}
});
const visibleEditors = new Set(this.notebookEditor.codeEditors.map(e => e[1]));
// Dispose old decorators
decorators.forEach((v, cell) => {
if (!visibleEditors.has(v.editor)) {
if (!validDiffDecorators.has(v)) {
v.dispose();
decorators.delete(cell);
}

Просмотреть файл

@ -26,6 +26,10 @@ import { IChatService } from '../../../../chat/common/chatService.js';
import { createDecorator, IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
import { INotebookOriginalModelReferenceFactory } from './notebookOriginalModelRefFactory.js';
import { autorun, autorunWithStore, derived, IObservable, observableValue } from '../../../../../../base/common/observable.js';
import { IModelService } from '../../../../../../editor/common/services/model.js';
import { NotebookCellTextModel } from '../../../common/model/notebookCellTextModel.js';
import { Event } from '../../../../../../base/common/event.js';
import { TextModel } from '../../../../../../editor/common/model/textModel.js';
export const INotebookModelSynchronizerFactory = createDecorator<INotebookModelSynchronizerFactory>('INotebookModelSynchronizerFactory');
@ -73,6 +77,7 @@ export class NotebookModelSynchronizer extends Disposable {
@IChatEditingService _chatEditingService: IChatEditingService,
@INotebookService private readonly notebookService: INotebookService,
@IChatService chatService: IChatService,
@IModelService private readonly modelService: IModelService,
@INotebookLoggingService private readonly logService: INotebookLoggingService,
@IConfigurationService private readonly configurationService: IConfigurationService,
@INotebookEditorWorkerService private readonly notebookEditorWorkerService: INotebookEditorWorkerService,
@ -277,9 +282,12 @@ export class NotebookModelSynchronizer extends Disposable {
// First Delete.
const deletedIndexes: number[] = [];
cellDiffInfoToApplyEdits.reverse().forEach(diff => {
await Promise.all(cellDiffInfoToApplyEdits.reverse().map(async diff => {
if (diff.type === 'delete') {
deletedIndexes.push(diff.originalCellIndex);
const cell = currentModel.cells[diff.originalCellIndex];
// Ensure the models of these cells have been loaded before we delete them.
await this.waitForCellModelToBeAvailable(cell);
edits.push({
editType: CellEditType.Replace,
index: diff.originalCellIndex,
@ -287,7 +295,7 @@ export class NotebookModelSynchronizer extends Disposable {
count: 1
});
}
});
}));
if (edits.length) {
currentModel.applyEdits(edits, true, undefined, () => undefined, undefined, false);
edits.length = 0;
@ -327,18 +335,17 @@ export class NotebookModelSynchronizer extends Disposable {
}
// Finally update
cellDiffInfoToApplyEdits.forEach(diff => {
await Promise.all(cellDiffInfoToApplyEdits.map(async diff => {
if (diff.type === 'modified') {
const cell = currentModel.cells[diff.originalCellIndex];
const textModel = cell.textModel;
if (textModel) {
const newText = modelWithChatEdits.cells[diff.modifiedCellIndex].getValue();
textModel.pushEditOperations(null, [
EditOperation.replace(textModel.getFullModelRange(), newText)
], () => null);
}
// Ensure the models of these cells have been loaded before we update them.
const textModel = await this.waitForCellModelToBeAvailable(cell);
const newText = modelWithChatEdits.cells[diff.modifiedCellIndex].getValue();
textModel.pushEditOperations(null, [
EditOperation.replace(textModel.getFullModelRange(), newText)
], () => null);
}
});
}));
if (edits.length) {
currentModel.applyEdits(edits, true, undefined, () => undefined, undefined, false);
@ -351,6 +358,13 @@ export class NotebookModelSynchronizer extends Disposable {
}
private previousUriOfModelForDiff?: URI;
private async waitForCellModelToBeAvailable(cell: NotebookCellTextModel): Promise<TextModel> {
if (cell.textModel) {
return cell.textModel;
}
await Event.toPromise(this.modelService.onModelAdded);
return this.waitForCellModelToBeAvailable(cell);
}
private async getModifiedModelForDiff(entry: IModifiedFileEntry, token: CancellationToken): Promise<NotebookTextModel | undefined> {
const text = (entry as ChatEditingModifiedFileEntry).modifiedModel.getValue();
const bytes = VSBuffer.fromString(text);