Skip to content

Commit 0c9dd26

Browse files
DonJayamanneDonJayamanne
authored
Rename file to fix typo and fixes to undo/redo (microsoft#234053)
* Rename file to fix typo * Fixes to undo/redo and support new menu * Hide menu
1 parent c3215a7 commit 0c9dd26

File tree

5 files changed

+87
-51
lines changed

5 files changed

+87
-51
lines changed

‎src/vs/workbench/contrib/chat/browser/chatEditorOverlay.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { Codicon } from '../../../../base/common/codicons.js';
2525
import { assertType } from '../../../../base/common/types.js';
2626
import { localize } from '../../../../nls.js';
2727
import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js';
28+
import { ctxNotebookHasEditorModification } from '../../notebook/browser/chatEdit/notebookChatEditController.js';
2829

2930
class ChatEditorOverlayWidget implements IOverlayWidget {
3031

@@ -246,14 +247,15 @@ class ChatEditorOverlayWidget implements IOverlayWidget {
246247
}
247248
}
248249

249-
const navigationBearingFakeActionId = 'chatEditor.navigation.bearings';
250+
export const navigationBearingFakeActionId = 'chatEditor.navigation.bearings';
250251

251252
MenuRegistry.appendMenuItem(MenuId.ChatEditingEditorContent, {
252253
command: {
253254
id: navigationBearingFakeActionId,
254255
title: localize('label', "Navigation Status"),
255256
precondition: ContextKeyExpr.false(),
256257
},
258+
when: ctxNotebookHasEditorModification.negate(),
257259
group: 'navigate',
258260
order: -1
259261
});

‎src/vs/workbench/contrib/notebook/browser/chatEdit/notebookChatActionsOverlay.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ export class NotebookChatActionsOverlay extends Disposable {
7878
menuOptions: { renderShortTitle: true },
7979
actionViewItemProvider: (action, options) => {
8080
const that = this;
81-
8281
if (action.id === 'chatEditor.action.accept' || action.id === 'chatEditor.action.reject') {
8382
return new class extends ActionViewItem {
8483
private readonly _reveal = this._store.add(new MutableDisposable());

‎src/vs/workbench/contrib/notebook/browser/chatEdit/notebookChatEditController.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

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

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

80+
let notebookSynchronizer: IReference<NotebookModelSynchronizer>;
8081
const entryObs = derived((r) => {
8182
const session = this._chatEditingService.currentEditingSessionObs.read(r);
8283
const model = notebookModel.read(r);
8384
if (!model || !session) {
8485
return;
8586
}
86-
const entry = session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
87+
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
88+
}).recomputeInitiallyAndOnChange(this._store);
89+
8790

88-
if (!entry || entry.state.read(r) !== WorkingSetEntryState.Modified) {
91+
this._register(autorun(r => {
92+
const entry = entryObs.read(r);
93+
const model = notebookModel.read(r);
94+
if (!entry || !model || entry.state.read(r) !== WorkingSetEntryState.Modified) {
8995
clearDecorators();
90-
return;
9196
}
92-
return entry;
93-
}).recomputeInitiallyAndOnChange(this._store);
94-
97+
}));
9598

96-
const snapshotCreated = observableValue<boolean>('snapshotCreated', false);
9799
const notebookDiffInfo = derivedWithStore(this, (r, store) => {
98100
const entry = entryObs.read(r);
99101
const model = notebookModel.read(r);
100102
if (!entry || !model) {
103+
// If entry is undefined, then revert the changes to the notebook.
104+
if (notebookSynchronizer && model) {
105+
notebookSynchronizer.object.revert();
106+
}
101107
return observableValue<{
102108
cellDiff: CellDiffInfo[];
103109
modelVersion: number;
104110
} | undefined>('DefaultDiffIno', undefined);
105111
}
106-
const notebookSynchronizer = store.add(this.synchronizerFactory.getOrCreate(model, entry));
107112

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

112116
return notebookSynchronizer.object.diffInfo;

‎src/vs/workbench/contrib/notebook/browser/chatEdit/notebookSynronizer.ts renamed to ‎src/vs/workbench/contrib/notebook/browser/chatEdit/notebookSynchronizer.ts

Lines changed: 67 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
*--------------------------------------------------------------------------------------------*/
55

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

3030

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

3333
export interface INotebookModelSynchronizerFactory {
3434
readonly _serviceBrand: undefined;
35-
getOrCreate(model: NotebookTextModel, entry: IModifiedFileEntry): IReference<NotebookModelSynchronizer>;
35+
getOrCreate(model: NotebookTextModel): IReference<NotebookModelSynchronizer>;
3636
}
3737

3838
class NotebookModelSynchronizerReferenceCollection extends ReferenceCollection<NotebookModelSynchronizer> {
3939
constructor(@IInstantiationService private readonly instantiationService: IInstantiationService) {
4040
super();
4141
}
42-
protected override createReferencedObject(_key: string, model: NotebookTextModel, entry: IModifiedFileEntry): NotebookModelSynchronizer {
43-
return this.instantiationService.createInstance(NotebookModelSynchronizer, model, entry);
42+
protected override createReferencedObject(_key: string, model: NotebookTextModel): NotebookModelSynchronizer {
43+
return this.instantiationService.createInstance(NotebookModelSynchronizer, model);
4444
}
4545
protected override destroyReferencedObject(_key: string, object: NotebookModelSynchronizer): void {
4646
object.dispose();
@@ -54,8 +54,8 @@ export class NotebookModelSynchronizerFactory implements INotebookModelSynchroni
5454
this._data = instantiationService.createInstance(NotebookModelSynchronizerReferenceCollection);
5555
}
5656

57-
getOrCreate(model: NotebookTextModel, entry: IModifiedFileEntry): IReference<NotebookModelSynchronizer> {
58-
return this._data.acquire(entry.modifiedURI.toString(), model, entry);
57+
getOrCreate(model: NotebookTextModel): IReference<NotebookModelSynchronizer> {
58+
return this._data.acquire(model.uri.toString(), model);
5959
}
6060
}
6161

@@ -70,7 +70,7 @@ export class NotebookModelSynchronizer extends Disposable {
7070
private isEditFromUs: boolean = false;
7171
constructor(
7272
private readonly model: NotebookTextModel,
73-
public readonly entry: IModifiedFileEntry,
73+
@IChatEditingService _chatEditingService: IChatEditingService,
7474
@INotebookService private readonly notebookService: INotebookService,
7575
@IChatService chatService: IChatService,
7676
@INotebookLoggingService private readonly logService: INotebookLoggingService,
@@ -81,51 +81,78 @@ export class NotebookModelSynchronizer extends Disposable {
8181
) {
8282
super();
8383

84+
const entryObs = derived((r) => {
85+
const session = _chatEditingService.currentEditingSessionObs.read(r);
86+
if (!session) {
87+
return;
88+
}
89+
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri));
90+
}).recomputeInitiallyAndOnChange(this._store);
91+
92+
8493
this._register(chatService.onDidPerformUserAction(async e => {
94+
const entry = entryObs.read(undefined);
95+
if (!entry) {
96+
return;
97+
}
8598
if (e.action.kind === 'chatEditingSessionAction' && !e.action.hasRemainingEdits && isEqual(e.action.uri, entry.modifiedURI)) {
8699
if (e.action.outcome === 'accepted') {
87-
await this.accept();
100+
await this.accept(entry);
88101
await this.createSnapshot();
89102
this._diffInfo.set(undefined, undefined);
90103
}
91104
else if (e.action.outcome === 'rejected') {
92-
if (await this.revert()) {
93-
this._diffInfo.set(undefined, undefined);
94-
}
105+
await this.revertImpl();
95106
}
96107
}
97108
}));
98109

99-
const cancellationTokenStore = this._register(new DisposableStore());
100-
let cancellationToken = cancellationTokenStore.add(new CancellationTokenSource());
101110
const updateNotebookModel = (entry: IModifiedFileEntry, token: CancellationToken) => {
102111
this.throttledUpdateNotebookModel.trigger(() => this.updateNotebookModel(entry, token));
103112
};
104-
const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
105-
this._register(modifiedModel.onDidChangeContent(async () => {
106-
cancellationTokenStore.clear();
107-
if (!modifiedModel.isDisposed() && !entry.originalModel.isDisposed() && modifiedModel.getValue() === entry.originalModel.getValue()) {
108-
if (await this.revert()) {
109-
this._diffInfo.set(undefined, undefined);
110-
}
113+
114+
this._register(autorun(async (r) => {
115+
const entry = entryObs.read(r);
116+
if (!entry) {
117+
return;
118+
}
119+
if (entry.state.read(r) === WorkingSetEntryState.Rejected) {
120+
await this.revertImpl();
121+
}
122+
}));
123+
124+
let snapshotCreated = false;
125+
this._register(autorunWithStore((r, store) => {
126+
const entry = entryObs.read(r);
127+
if (!entry) {
111128
return;
112129
}
113-
cancellationToken = cancellationTokenStore.add(new CancellationTokenSource());
130+
if (!snapshotCreated) {
131+
this.createSnapshot();
132+
snapshotCreated = true;
133+
}
134+
135+
const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
136+
let cancellationToken = store.add(new CancellationTokenSource());
137+
store.add(modifiedModel.onDidChangeContent(async () => {
138+
if (!modifiedModel.isDisposed() && !entry.originalModel.isDisposed() && modifiedModel.getValue() !== entry.originalModel.getValue()) {
139+
cancellationToken = store.add(new CancellationTokenSource());
140+
updateNotebookModel(entry, cancellationToken.token);
141+
}
142+
}));
143+
114144
updateNotebookModel(entry, cancellationToken.token);
115145
}));
146+
116147
this._register(model.onDidChangeContent(() => {
117148
// Track changes from the user.
118149
if (!this.isEditFromUs && this.snapshot) {
119150
this.snapshot.dirty = true;
120151
}
121152
}));
122-
123-
updateNotebookModel(entry, cancellationToken.token);
124-
125-
126153
}
127154

128-
public async createSnapshot() {
155+
private async createSnapshot() {
129156
const [serializer, ref] = await Promise.all([
130157
this.getNotebookSerializer(),
131158
this.notebookModelResolverService.resolve(this.model.uri)
@@ -173,12 +200,16 @@ export class NotebookModelSynchronizer extends Disposable {
173200
}
174201
}
175202

176-
private async revert(): Promise<boolean> {
203+
public async revert() {
204+
await this.revertImpl();
205+
}
206+
207+
private async revertImpl(): Promise<void> {
177208
if (!this.snapshot) {
178-
return false;
209+
return;
179210
}
180211
await this.updateNotebook(this.snapshot.bytes, !this.snapshot.dirty);
181-
return true;
212+
this._diffInfo.set(undefined, undefined);
182213
}
183214

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

202-
private async accept() {
203-
const modifiedModel = (this.entry as ChatEditingModifiedFileEntry).modifiedModel;
233+
private async accept(entry: IModifiedFileEntry) {
234+
const modifiedModel = (entry as ChatEditingModifiedFileEntry).modifiedModel;
204235
const content = modifiedModel.getValue();
205236
await this.updateNotebook(VSBuffer.fromString(content), false);
206237
}
@@ -211,9 +242,9 @@ export class NotebookModelSynchronizer extends Disposable {
211242
}
212243

213244
private _originalModel?: Promise<NotebookTextModel>;
214-
private async getOriginalModel(): Promise<NotebookTextModel> {
245+
private async getOriginalModel(entry: IModifiedFileEntry): Promise<NotebookTextModel> {
215246
if (!this._originalModel) {
216-
this._originalModel = this.originalModelRefFactory.getOrCreate(this.entry, this.model.viewType).then(ref => this._register(ref).object);
247+
this._originalModel = this.originalModelRefFactory.getOrCreate(entry, this.model.viewType).then(ref => this._register(ref).object);
217248
}
218249
return this._originalModel;
219250
}
@@ -225,7 +256,7 @@ export class NotebookModelSynchronizer extends Disposable {
225256
if (!modelWithChatEdits || token.isCancellationRequested || !currentModel) {
226257
return;
227258
}
228-
const originalModel = await this.getOriginalModel();
259+
const originalModel = await this.getOriginalModel(entry);
229260
// This is the total diff from the original model to the model with chat edits.
230261
const cellDiffInfo = (await this.computeDiff(originalModel, modelWithChatEdits, token))?.cellDiffInfo;
231262
// This is the diff from the current model to the model with chat edits.

‎src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ import { getFormattedNotebookMetadataJSON } from '../common/model/notebookMetada
133133
import { NotebookChatEditorControllerContrib } from './chatEdit/notebookChatEditController.js';
134134
import { registerNotebookContribution } from './notebookEditorExtensions.js';
135135
import { INotebookOriginalModelReferenceFactory, NotebookOriginalModelReferenceFactory } from './chatEdit/notebookOriginalModelRefFactory.js';
136-
import { INotebookModelSynchronizerFactory, NotebookModelSynchronizerFactory } from './chatEdit/notebookSynronizer.js';
136+
import { INotebookModelSynchronizerFactory, NotebookModelSynchronizerFactory } from './chatEdit/notebookSynchronizer.js';
137137
import { INotebookOriginalCellModelFactory, OriginalNotebookCellModelFactory } from './chatEdit/notebookOriginalCellModelFactory.js';
138138

139139
/*--------------------------------------------------------------------------------------------- */

0 commit comments

Comments
 (0)