Skip to content

Commit

Permalink
Rename file to fix typo and fixes to undo/redo (#234053)
Browse files Browse the repository at this point in the history 
* Rename file to fix typo

* Fixes to undo/redo and support new menu

* Hide menu
  • Loading branch information
@DonJayamanne
DonJayamanne authored Nov 18, 2024
1 parent c3215a7 commit 0c9dd26
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 51 deletions.
4 changes: 3 additions & 1 deletion src/vs/workbench/contrib/chat/browser/chatEditorOverlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { Codicon } from '../../../../base/common/codicons.js';
import { assertType } from '../../../../base/common/types.js';
import { localize } from '../../../../nls.js';
import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js';
import { ctxNotebookHasEditorModification } from '../../notebook/browser/chatEdit/notebookChatEditController.js';

class ChatEditorOverlayWidget implements IOverlayWidget {

Expand Down Expand Up @@ -246,14 +247,15 @@ class ChatEditorOverlayWidget implements IOverlayWidget {
}
}

const navigationBearingFakeActionId = 'chatEditor.navigation.bearings';
export const navigationBearingFakeActionId = 'chatEditor.navigation.bearings';

MenuRegistry.appendMenuItem(MenuId.ChatEditingEditorContent, {
command: {
id: navigationBearingFakeActionId,
title: localize('label', "Navigation Status"),
precondition: ContextKeyExpr.false(),
},
when: ctxNotebookHasEditorModification.negate(),
group: 'navigate',
order: -1
});
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/contrib/notebook/browser/chatEdit/notebookChatActionsOverlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export class NotebookChatActionsOverlay extends Disposable {
menuOptions: { renderShortTitle: true },
actionViewItemProvider: (action, options) => {
const that = this;

if (action.id === 'chatEditor.action.accept' || action.id === 'chatEditor.action.reject') {
return new class extends ActionViewItem {
private readonly _reveal = this._store.add(new MutableDisposable());
Expand Down
28 changes: 16 additions & 12 deletions src/vs/workbench/contrib/notebook/browser/chatEdit/notebookChatEditController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { isEqual } from '../../../../../base/common/resources.js';
import { Disposable, dispose, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js';
import { Disposable, dispose, IDisposable, IReference, toDisposable } from '../../../../../base/common/lifecycle.js';
import { autorun, derived, derivedWithStore, observableFromEvent, observableValue } from '../../../../../base/common/observable.js';
import { IChatEditingService, WorkingSetEntryState } from '../../../chat/common/chatEditingService.js';
import { NotebookTextModel } from '../../common/model/notebookTextModel.js';
Expand All @@ -14,7 +14,7 @@ import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
import { NotebookCellTextModel } from '../../common/model/notebookCellTextModel.js';
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
import { NotebookDeletedCellDecorator, NotebookInsertedCellDecorator, NotebookCellDiffDecorator } from './notebookCellDecorators.js';
import { INotebookModelSynchronizerFactory } from './notebookSynronizer.js';
import { INotebookModelSynchronizerFactory, NotebookModelSynchronizer } from './notebookSynchronizer.js';
import { INotebookOriginalModelReferenceFactory } from './notebookOriginalModelRefFactory.js';
import { debouncedObservable2 } from '../../../../../base/common/observableInternal/utils.js';
import { CellDiffInfo } from '../diff/notebookDiffViewModel.js';
Expand Down Expand Up @@ -77,36 +77,40 @@ class NotebookChatEditorController extends Disposable {

this._register(toDisposable(() => clearDecorators()));

let notebookSynchronizer: IReference<NotebookModelSynchronizer>;
const entryObs = derived((r) => {
const session = this._chatEditingService.currentEditingSessionObs.read(r);
const model = notebookModel.read(r);
if (!model || !session) {
return;
}
const entry = session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
}).recomputeInitiallyAndOnChange(this._store);


if (!entry || entry.state.read(r) !== WorkingSetEntryState.Modified) {
this._register(autorun(r => {
const entry = entryObs.read(r);
const model = notebookModel.read(r);
if (!entry || !model || entry.state.read(r) !== WorkingSetEntryState.Modified) {
clearDecorators();
return;
}
return entry;
}).recomputeInitiallyAndOnChange(this._store);

}));

const snapshotCreated = observableValue<boolean>('snapshotCreated', false);
const notebookDiffInfo = derivedWithStore(this, (r, store) => {
const entry = entryObs.read(r);
const model = notebookModel.read(r);
if (!entry || !model) {
// If entry is undefined, then revert the changes to the notebook.
if (notebookSynchronizer && model) {
notebookSynchronizer.object.revert();
}
return observableValue<{
cellDiff: CellDiffInfo[];
modelVersion: number;
} | undefined>('DefaultDiffIno', undefined);
}
const notebookSynchronizer = store.add(this.synchronizerFactory.getOrCreate(model, entry));

// Initialize the observables.
notebookSynchronizer.object.createSnapshot().finally(() => snapshotCreated.set(true, undefined));
notebookSynchronizer = notebookSynchronizer || this._register(this.synchronizerFactory.getOrCreate(model));
this.originalModelRefFactory.getOrCreate(entry, model.viewType).then(ref => originalModel.set(this._register(ref).object, undefined));

return notebookSynchronizer.object.diffInfo;
Expand Down
103 changes: 67 additions & 36 deletions ...ok/browser/chatEdit/notebookSynronizer.ts → .../browser/chatEdit/notebookSynchronizer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import { isEqual } from '../../../../../base/common/resources.js';
import { Disposable, DisposableStore, IReference, ReferenceCollection } from '../../../../../base/common/lifecycle.js';
import { IModifiedFileEntry } from '../../../chat/common/chatEditingService.js';
import { Disposable, IReference, ReferenceCollection } from '../../../../../base/common/lifecycle.js';
import { IChatEditingService, IModifiedFileEntry, WorkingSetEntryState } from '../../../chat/common/chatEditingService.js';
import { INotebookService } from '../../common/notebookService.js';
import { bufferToStream, VSBuffer } from '../../../../../base/common/buffer.js';
import { NotebookTextModel } from '../../common/model/notebookTextModel.js';
Expand All @@ -25,22 +25,22 @@ import { SaveReason } from '../../../../common/editor.js';
import { IChatService } from '../../../chat/common/chatService.js';
import { createDecorator, IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
import { INotebookOriginalModelReferenceFactory } from './notebookOriginalModelRefFactory.js';
import { IObservable, observableValue } from '../../../../../base/common/observable.js';
import { autorun, autorunWithStore, derived, IObservable, observableValue } from '../../../../../base/common/observable.js';


export const INotebookModelSynchronizerFactory = createDecorator<INotebookModelSynchronizerFactory>('INotebookModelSynchronizerFactory');

export interface INotebookModelSynchronizerFactory {
readonly _serviceBrand: undefined;
getOrCreate(model: NotebookTextModel, entry: IModifiedFileEntry): IReference<NotebookModelSynchronizer>;
getOrCreate(model: NotebookTextModel): IReference<NotebookModelSynchronizer>;
}

class NotebookModelSynchronizerReferenceCollection extends ReferenceCollection<NotebookModelSynchronizer> {
constructor(@IInstantiationService private readonly instantiationService: IInstantiationService) {
super();
}
protected override createReferencedObject(_key: string, model: NotebookTextModel, entry: IModifiedFileEntry): NotebookModelSynchronizer {
return this.instantiationService.createInstance(NotebookModelSynchronizer, model, entry);
protected override createReferencedObject(_key: string, model: NotebookTextModel): NotebookModelSynchronizer {
return this.instantiationService.createInstance(NotebookModelSynchronizer, model);
}
protected override destroyReferencedObject(_key: string, object: NotebookModelSynchronizer): void {
object.dispose();
Expand All @@ -54,8 +54,8 @@ export class NotebookModelSynchronizerFactory implements INotebookModelSynchroni
this._data = instantiationService.createInstance(NotebookModelSynchronizerReferenceCollection);
}

getOrCreate(model: NotebookTextModel, entry: IModifiedFileEntry): IReference<NotebookModelSynchronizer> {
return this._data.acquire(entry.modifiedURI.toString(), model, entry);
getOrCreate(model: NotebookTextModel): IReference<NotebookModelSynchronizer> {
return this._data.acquire(model.uri.toString(), model);
}
}

Expand All @@ -70,7 +70,7 @@ export class NotebookModelSynchronizer extends Disposable {
private isEditFromUs: boolean = false;
constructor(
private readonly model: NotebookTextModel,
public readonly entry: IModifiedFileEntry,
@IChatEditingService _chatEditingService: IChatEditingService,
@INotebookService private readonly notebookService: INotebookService,
@IChatService chatService: IChatService,
@INotebookLoggingService private readonly logService: INotebookLoggingService,
Expand All @@ -81,51 +81,78 @@ export class NotebookModelSynchronizer extends Disposable {
) {
super();

const entryObs = derived((r) => {
const session = _chatEditingService.currentEditingSessionObs.read(r);
if (!session) {
return;
}
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
}).recomputeInitiallyAndOnChange(this._store);


this._register(chatService.onDidPerformUserAction(async e => {
const entry = entryObs.read(undefined);
if (!entry) {
return;
}
if (e.action.kind === 'chatEditingSessionAction' && !e.action.hasRemainingEdits && isEqual(e.action.uri, entry.modifiedURI)) {
if (e.action.outcome === 'accepted') {
await this.accept();
await this.accept(entry);
await this.createSnapshot();
this._diffInfo.set(undefined, undefined);
}
else if (e.action.outcome === 'rejected') {
if (await this.revert()) {
this._diffInfo.set(undefined, undefined);
}
await this.revertImpl();
}
}
}));

const cancellationTokenStore = this._register(new DisposableStore());
let cancellationToken = cancellationTokenStore.add(new CancellationTokenSource());
const updateNotebookModel = (entry: IModifiedFileEntry, token: CancellationToken) => {
this.throttledUpdateNotebookModel.trigger(() => this.updateNotebookModel(entry, token));
};
const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
this._register(modifiedModel.onDidChangeContent(async () => {
cancellationTokenStore.clear();
if (!modifiedModel.isDisposed() && !entry.originalModel.isDisposed() && modifiedModel.getValue() === entry.originalModel.getValue()) {
if (await this.revert()) {
this._diffInfo.set(undefined, undefined);
}

this._register(autorun(async (r) => {
const entry = entryObs.read(r);
if (!entry) {
return;
}
if (entry.state.read(r) === WorkingSetEntryState.Rejected) {
await this.revertImpl();
}
}));

let snapshotCreated = false;
this._register(autorunWithStore((r, store) => {
const entry = entryObs.read(r);
if (!entry) {
return;
}
cancellationToken = cancellationTokenStore.add(new CancellationTokenSource());
if (!snapshotCreated) {
this.createSnapshot();
snapshotCreated = true;
}

const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
let cancellationToken = store.add(new CancellationTokenSource());
store.add(modifiedModel.onDidChangeContent(async () => {
if (!modifiedModel.isDisposed() && !entry.originalModel.isDisposed() && modifiedModel.getValue() !== entry.originalModel.getValue()) {
cancellationToken = store.add(new CancellationTokenSource());
updateNotebookModel(entry, cancellationToken.token);
}
}));

updateNotebookModel(entry, cancellationToken.token);
}));

this._register(model.onDidChangeContent(() => {
// Track changes from the user.
if (!this.isEditFromUs && this.snapshot) {
this.snapshot.dirty = true;
}
}));

updateNotebookModel(entry, cancellationToken.token);


}

public async createSnapshot() {
private async createSnapshot() {
const [serializer, ref] = await Promise.all([
this.getNotebookSerializer(),
this.notebookModelResolverService.resolve(this.model.uri)
Expand Down Expand Up @@ -173,12 +200,16 @@ export class NotebookModelSynchronizer extends Disposable {
}
}

private async revert(): Promise<boolean> {
public async revert() {
await this.revertImpl();
}

private async revertImpl(): Promise<void> {
if (!this.snapshot) {
return false;
return;
}
await this.updateNotebook(this.snapshot.bytes, !this.snapshot.dirty);
return true;
this._diffInfo.set(undefined, undefined);
}

private async updateNotebook(bytes: VSBuffer, save: boolean) {
Expand All @@ -199,8 +230,8 @@ export class NotebookModelSynchronizer extends Disposable {
}
}

private async accept() {
const modifiedModel = (this.entry as ChatEditingModifiedFileEntry).modifiedModel;
private async accept(entry: IModifiedFileEntry) {
const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
const content = modifiedModel.getValue();
await this.updateNotebook(VSBuffer.fromString(content), false);
}
Expand All @@ -211,9 +242,9 @@ export class NotebookModelSynchronizer extends Disposable {
}

private _originalModel?: Promise<NotebookTextModel>;
private async getOriginalModel(): Promise<NotebookTextModel> {
private async getOriginalModel(entry: IModifiedFileEntry): Promise<NotebookTextModel> {
if (!this._originalModel) {
this._originalModel = this.originalModelRefFactory.getOrCreate(this.entry, this.model.viewType).then(ref => this._register(ref).object);
this._originalModel = this.originalModelRefFactory.getOrCreate(entry, this.model.viewType).then(ref => this._register(ref).object);
}
return this._originalModel;
}
Expand All @@ -225,7 +256,7 @@ export class NotebookModelSynchronizer extends Disposable {
if (!modelWithChatEdits || token.isCancellationRequested || !currentModel) {
return;
}
const originalModel = await this.getOriginalModel();
const originalModel = await this.getOriginalModel(entry);
// This is the total diff from the original model to the model with chat edits.
const cellDiffInfo = (await this.computeDiff(originalModel, modelWithChatEdits, token))?.cellDiffInfo;
// This is the diff from the current model to the model with chat edits.
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ import { getFormattedNotebookMetadataJSON } from '../common/model/notebookMetada
import { NotebookChatEditorControllerContrib } from './chatEdit/notebookChatEditController.js';
import { registerNotebookContribution } from './notebookEditorExtensions.js';
import { INotebookOriginalModelReferenceFactory, NotebookOriginalModelReferenceFactory } from './chatEdit/notebookOriginalModelRefFactory.js';
import { INotebookModelSynchronizerFactory, NotebookModelSynchronizerFactory } from './chatEdit/notebookSynronizer.js';
import { INotebookModelSynchronizerFactory, NotebookModelSynchronizerFactory } from './chatEdit/notebookSynchronizer.js';
import { INotebookOriginalCellModelFactory, OriginalNotebookCellModelFactory } from './chatEdit/notebookOriginalCellModelFactory.js';

/*--------------------------------------------------------------------------------------------- */
Expand Down

0 comments on commit 0c9dd26

Please sign in to comment.