Skip to content

Commit c7ae222

Browse files
jriekenjrieken
authored
no more asyncTransaction usage in chat land (microsoft#250831)
fixes microsoft#248827
1 parent 9ab3157 commit c7ae222

File tree

6 files changed

+75
-86
lines changed

6 files changed

+75
-86
lines changed

‎src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,11 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
491491
return undefined;
492492
}
493493

494-
protected override async _doAccept(tx: ITransaction | undefined): Promise<void> {
494+
protected override async _doAccept(): Promise<void> {
495495
this.originalModel.setValue(this.modifiedModel.createSnapshot());
496-
this._diffInfo.set(nullDocumentDiff, tx);
496+
this._diffInfo.set(nullDocumentDiff, undefined);
497497
this._edit = StringEdit.empty;
498-
await this._collapse(tx);
498+
this._multiDiffEntryDelegate.collapse(undefined);
499499

500500
const config = this._fileConfigService.getAutoSaveConfiguration(this.modifiedURI);
501501
if (!config.autoSave || !this._textFileService.isDirty(this.modifiedURI)) {
@@ -513,7 +513,7 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
513513
}
514514
}
515515

516-
protected override async _doReject(tx: ITransaction | undefined): Promise<void> {
516+
protected override async _doReject(): Promise<void> {
517517
if (this.createdInRequestId === this._telemetryInfo.requestId) {
518518
if (isTextFileEditorModel(this._docFileEditorModel)) {
519519
await this._docFileEditorModel.revert({ soft: true });
@@ -527,7 +527,7 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
527527
// and so that an intermediate saved state gets reverted
528528
await this._docFileEditorModel.save({ reason: SaveReason.EXPLICIT, skipSaveParticipants: true });
529529
}
530-
await this._collapse(tx);
530+
this._multiDiffEntryDelegate.collapse(undefined);
531531
}
532532
}
533533

@@ -543,9 +543,6 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
543543
}
544544
}
545545

546-
private async _collapse(transaction: ITransaction | undefined): Promise<void> {
547-
this._multiDiffEntryDelegate.collapse(transaction);
548-
}
549546

550547
protected _createEditorIntegration(editor: IEditorPane): IModifiedFileEntryEditorIntegration {
551548
const codeEditor = getCodeEditor(editor.getControl());

‎src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { Emitter } from '../../../../../base/common/event.js';
88
import { Disposable, DisposableMap, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js';
99
import { Schemas } from '../../../../../base/common/network.js';
1010
import { clamp } from '../../../../../base/common/numbers.js';
11-
import { autorun, derived, IObservable, ITransaction, observableValue, observableValueOpts } from '../../../../../base/common/observable.js';
11+
import { autorun, derived, IObservable, ITransaction, observableValue, observableValueOpts, transaction } from '../../../../../base/common/observable.js';
1212
import { URI } from '../../../../../base/common/uri.js';
1313
import { StringEdit } from '../../../../../editor/common/core/edits/stringEdit.js';
1414
import { TextEdit } from '../../../../../editor/common/languages.js';
@@ -161,7 +161,7 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
161161

162162
const remain = Math.round(future - Date.now());
163163
if (remain <= 0) {
164-
this.accept(undefined);
164+
this.accept();
165165
} else {
166166
const handle = setTimeout(update, 100);
167167
this._autoAcceptCtrl.set(new AutoAcceptControl(acceptTimeout, remain, () => {
@@ -206,34 +206,38 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
206206
this._telemetryInfo = telemetryInfo;
207207
}
208208

209-
async accept(tx: ITransaction | undefined): Promise<void> {
209+
async accept(): Promise<void> {
210210
if (this._stateObs.get() !== ModifiedFileEntryState.Modified) {
211211
// already accepted or rejected
212212
return;
213213
}
214214

215-
await this._doAccept(tx);
216-
this._stateObs.set(ModifiedFileEntryState.Accepted, tx);
217-
this._autoAcceptCtrl.set(undefined, tx);
215+
await this._doAccept();
216+
transaction(tx => {
217+
this._stateObs.set(ModifiedFileEntryState.Accepted, tx);
218+
this._autoAcceptCtrl.set(undefined, tx);
219+
});
218220

219221
this._notifyAction('accepted');
220222
}
221223

222-
protected abstract _doAccept(tx: ITransaction | undefined): Promise<void>;
224+
protected abstract _doAccept(): Promise<void>;
223225

224-
async reject(tx: ITransaction | undefined): Promise<void> {
226+
async reject(): Promise<void> {
225227
if (this._stateObs.get() !== ModifiedFileEntryState.Modified) {
226228
// already accepted or rejected
227229
return;
228230
}
229231

230232
this._notifyAction('rejected');
231-
await this._doReject(tx);
232-
this._stateObs.set(ModifiedFileEntryState.Rejected, tx);
233-
this._autoAcceptCtrl.set(undefined, tx);
233+
await this._doReject();
234+
transaction(tx => {
235+
this._stateObs.set(ModifiedFileEntryState.Rejected, tx);
236+
this._autoAcceptCtrl.set(undefined, tx);
237+
});
234238
}
235239

236-
protected abstract _doReject(tx: ITransaction | undefined): Promise<void>;
240+
protected abstract _doReject(): Promise<void>;
237241

238242
protected _notifyAction(outcome: 'accepted' | 'rejected' | 'userModified') {
239243
this._chatService.notifyUserAction({
@@ -283,18 +287,18 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
283287

284288
abstract acceptAgentEdits(uri: URI, edits: (TextEdit | ICellEditOperation)[], isLastEdits: boolean, responseModel: IChatResponseModel): Promise<void>;
285289

286-
async acceptStreamingEditsEnd(tx: ITransaction) {
287-
this._resetEditsState(tx);
290+
async acceptStreamingEditsEnd() {
291+
this._resetEditsState(undefined);
288292

289293
if (await this._areOriginalAndModifiedIdentical()) {
290294
// ACCEPT if identical
291-
await this.accept(tx);
295+
await this.accept();
292296
}
293297
}
294298

295299
protected abstract _areOriginalAndModifiedIdentical(): Promise<boolean>;
296300

297-
protected _resetEditsState(tx: ITransaction): void {
301+
protected _resetEditsState(tx: ITransaction | undefined): void {
298302
this._isCurrentlyBeingModifiedByObs.set(undefined, tx);
299303
this._rewriteRatioObs.set(0, tx);
300304
this._waitsForLastEdits.set(false, tx);

‎src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedNotebookEntry.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -412,12 +412,12 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie
412412
}
413413
}
414414

415-
protected override async _doAccept(tx: ITransaction | undefined): Promise<void> {
416-
this.updateCellDiffInfo([], tx);
415+
protected override async _doAccept(): Promise<void> {
416+
this.updateCellDiffInfo([], undefined);
417417
const snapshot = createSnapshot(this.modifiedModel, this.transientOptions, this.configurationService);
418418
restoreSnapshot(this.originalModel, snapshot);
419419
this.initializeModelsFromDiff();
420-
await this._collapse(tx);
420+
await this._collapse(undefined);
421421

422422
const config = this._fileConfigService.getAutoSaveConfiguration(this.modifiedURI);
423423
if (this.modifiedModel.uri.scheme !== Schemas.untitled && (!config.autoSave || !this.notebookResolver.isDirty(this.modifiedURI))) {
@@ -436,8 +436,8 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie
436436
}
437437
}
438438

439-
protected override async _doReject(tx: ITransaction | undefined): Promise<void> {
440-
this.updateCellDiffInfo([], tx);
439+
protected override async _doReject(): Promise<void> {
440+
this.updateCellDiffInfo([], undefined);
441441
if (this.createdInRequestId === this._telemetryInfo.requestId) {
442442
await this._applyEdits(async () => {
443443
await this.modifiedResourceRef.object.revert({ soft: true });
@@ -455,7 +455,7 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie
455455
}
456456
});
457457
this.initializeModelsFromDiff();
458-
await this._collapse(tx);
458+
await this._collapse(undefined);
459459
}
460460
}
461461

‎src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingSession.ts

Lines changed: 40 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { Emitter } from '../../../../../base/common/event.js';
1212
import { Iterable } from '../../../../../base/common/iterator.js';
1313
import { Disposable, DisposableStore, dispose } from '../../../../../base/common/lifecycle.js';
1414
import { ResourceMap } from '../../../../../base/common/map.js';
15-
import { asyncTransaction, autorun, derived, derivedOpts, IObservable, IReader, ITransaction, ObservablePromise, observableValue, transaction } from '../../../../../base/common/observable.js';
15+
import { autorun, derived, derivedOpts, IObservable, IReader, ITransaction, ObservablePromise, observableValue, transaction } from '../../../../../base/common/observable.js';
1616
import { isEqual } from '../../../../../base/common/resources.js';
1717
import { URI } from '../../../../../base/common/uri.js';
1818
import { IBulkEditService } from '../../../../../editor/browser/services/bulkEditService.js';
@@ -224,9 +224,9 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
224224
for (const [uri, content] of restoredSessionState.initialFileContents) {
225225
this._initialFileContents.set(uri, content);
226226
}
227-
await asyncTransaction(async tx => {
228-
this._pendingSnapshot = restoredSessionState.pendingSnapshot;
229-
await this._restoreSnapshot(restoredSessionState.recentSnapshot, tx, false);
227+
this._pendingSnapshot = restoredSessionState.pendingSnapshot;
228+
await this._restoreSnapshot(restoredSessionState.recentSnapshot, false);
229+
transaction(async tx => {
230230
this._linearHistory.set(restoredSessionState.linearHistory, tx);
231231
this._linearHistoryIndex.set(restoredSessionState.linearHistoryIndex, tx);
232232
this._state.set(ChatEditingSessionState.Idle, tx);
@@ -491,10 +491,8 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
491491
const stopRef = this._findEditStop(requestId, stopId);
492492
if (stopRef) {
493493
this._ensurePendingSnapshot();
494-
await asyncTransaction(async tx => {
495-
this._linearHistoryIndex.set(stopRef.historyIndex, tx);
496-
await this._restoreSnapshot(stopRef.stop, tx);
497-
});
494+
this._linearHistoryIndex.set(stopRef.historyIndex, undefined);
495+
await this._restoreSnapshot(stopRef.stop);
498496
this._updateRequestHiddenState();
499497
}
500498
} else {
@@ -507,7 +505,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
507505
}
508506
}
509507

510-
private async _restoreSnapshot({ entries }: IChatEditingSessionStop, tx: ITransaction | undefined, restoreResolvedToDisk = true): Promise<void> {
508+
private async _restoreSnapshot({ entries }: IChatEditingSessionStop, restoreResolvedToDisk = true): Promise<void> {
511509

512510
// Reset all the files which are modified in this session state
513511
// but which are not found in the snapshot
@@ -528,7 +526,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
528526
entriesArr.push(entry);
529527
}
530528

531-
this._entriesObs.set(entriesArr, tx);
529+
this._entriesObs.set(entriesArr, undefined);
532530
}
533531

534532
remove(...uris: URI[]): void {
@@ -562,37 +560,32 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
562560
async accept(...uris: URI[]): Promise<void> {
563561
this._assertNotDisposed();
564562

565-
await asyncTransaction(async tx => {
566-
567-
if (uris.length === 0) {
568-
await Promise.all(this._entriesObs.get().map(entry => entry.accept(tx)));
569-
}
563+
if (uris.length === 0) {
564+
await Promise.all(this._entriesObs.get().map(entry => entry.accept()));
565+
}
570566

571-
for (const uri of uris) {
572-
const entry = this._entriesObs.get().find(e => isEqual(e.modifiedURI, uri));
573-
if (entry) {
574-
await entry.accept(tx);
575-
}
567+
for (const uri of uris) {
568+
const entry = this._entriesObs.get().find(e => isEqual(e.modifiedURI, uri));
569+
if (entry) {
570+
await entry.accept();
576571
}
577-
});
572+
}
578573
this._accessibilitySignalService.playSignal(AccessibilitySignal.editsKept, { allowManyInParallel: true });
579574
}
580575

581576
async reject(...uris: URI[]): Promise<void> {
582577
this._assertNotDisposed();
583578

584-
await asyncTransaction(async tx => {
585-
if (uris.length === 0) {
586-
await Promise.all(this._entriesObs.get().map(entry => entry.reject(tx)));
587-
}
579+
if (uris.length === 0) {
580+
await Promise.all(this._entriesObs.get().map(entry => entry.reject()));
581+
}
588582

589-
for (const uri of uris) {
590-
const entry = this._entriesObs.get().find(e => isEqual(e.modifiedURI, uri));
591-
if (entry) {
592-
await entry.reject(tx);
593-
}
583+
for (const uri of uris) {
584+
const entry = this._entriesObs.get().find(e => isEqual(e.modifiedURI, uri));
585+
if (entry) {
586+
await entry.reject();
594587
}
595-
});
588+
}
596589
this._accessibilitySignalService.playSignal(AccessibilitySignal.editsUndone, { allowManyInParallel: true });
597590
}
598591

@@ -738,10 +731,8 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
738731
}
739732

740733
this._ensurePendingSnapshot();
741-
await asyncTransaction(async tx => {
742-
await this._restoreSnapshot(previousSnapshot.stop, tx);
743-
this._linearHistoryIndex.set(newIndex, tx);
744-
});
734+
await this._restoreSnapshot(previousSnapshot.stop);
735+
this._linearHistoryIndex.set(newIndex, undefined);
745736
this._updateRequestHiddenState();
746737
}
747738

@@ -756,10 +747,8 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
756747
if (!nextSnapshot) {
757748
return;
758749
}
759-
await asyncTransaction(async tx => {
760-
await this._restoreSnapshot(nextSnapshot, tx);
761-
this._linearHistoryIndex.set(newIndex, tx);
762-
});
750+
await this._restoreSnapshot(nextSnapshot);
751+
this._linearHistoryIndex.set(newIndex, undefined);
763752
this._updateRequestHiddenState();
764753
}
765754

@@ -809,7 +798,7 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
809798
*
810799
* @param next If true, this will edit the snapshot _after_ the undo stop
811800
*/
812-
private ensureEditInUndoStopMatches(requestId: string, undoStop: string | undefined, entry: AbstractChatEditingModifiedFileEntry, next: boolean, tx: ITransaction) {
801+
private ensureEditInUndoStopMatches(requestId: string, undoStop: string | undefined, entry: AbstractChatEditingModifiedFileEntry, next: boolean, tx: ITransaction | undefined) {
813802
const history = this._linearHistory.get();
814803
const snapIndex = history.findIndex(s => s.requestId === requestId);
815804
if (snapIndex === -1) {
@@ -871,20 +860,19 @@ export class ChatEditingSession extends Disposable implements IChatEditingSessio
871860
}
872861

873862
private async _resolve(requestId: string, undoStop: string | undefined, resource: URI): Promise<void> {
874-
await asyncTransaction(async (tx) => {
875-
const hasOtherTasks = Iterable.some(this._streamingEditLocks.keys(), k => k !== resource.toString());
876-
if (!hasOtherTasks) {
877-
this._state.set(ChatEditingSessionState.Idle, tx);
878-
}
879863

880-
const entry = this._getEntry(resource);
881-
if (!entry) {
882-
return;
883-
}
864+
const hasOtherTasks = Iterable.some(this._streamingEditLocks.keys(), k => k !== resource.toString());
865+
if (!hasOtherTasks) {
866+
this._state.set(ChatEditingSessionState.Idle, undefined);
867+
}
884868

885-
this.ensureEditInUndoStopMatches(requestId, undoStop, entry, /* next= */ true, tx);
886-
return entry.acceptStreamingEditsEnd(tx);
887-
});
869+
const entry = this._getEntry(resource);
870+
if (!entry) {
871+
return;
872+
}
873+
874+
this.ensureEditInUndoStopMatches(requestId, undoStop, entry, /* next= */ true, undefined);
875+
return entry.acceptStreamingEditsEnd();
888876

889877
}
890878

‎src/vs/workbench/contrib/chat/common/chatEditingService.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { CancellationToken } from '../../../../base/common/cancellation.js';
77
import { Event } from '../../../../base/common/event.js';
88
import { IDisposable } from '../../../../base/common/lifecycle.js';
9-
import { IObservable, IReader, ITransaction } from '../../../../base/common/observable.js';
9+
import { IObservable, IReader } from '../../../../base/common/observable.js';
1010
import { URI } from '../../../../base/common/uri.js';
1111
import { TextEdit } from '../../../../editor/common/languages.js';
1212
import { localize } from '../../../../nls.js';
@@ -217,8 +217,8 @@ export interface IModifiedFileEntry {
217217

218218
readonly waitsForLastEdits: IObservable<boolean>;
219219

220-
accept(transaction: ITransaction | undefined): Promise<void>;
221-
reject(transaction: ITransaction | undefined): Promise<void>;
220+
accept(): Promise<void>;
221+
reject(): Promise<void>;
222222

223223
reviewMode: IObservable<boolean>;
224224
autoAcceptController: IObservable<{ total: number; remaining: number; cancel(): void } | undefined>;

‎src/vs/workbench/contrib/chat/test/browser/chatEditingService.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ suite('ChatEditingService', function () {
166166

167167
await unset;
168168

169-
await entry.reject(undefined);
169+
await entry.reject();
170170

171171
model.dispose();
172172
});

0 commit comments

Comments
 (0)