From 8329e3a760c3e1bcf0cc71fb6a9ea2de1e9bdb66 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 9 Oct 2024 12:20:25 +0200 Subject: [PATCH] simplify save flow (#230883) --- .../contrib/chat/browser/chatEditorSaving.ts | 40 +++++++++---------- .../textfile/common/textFileEditorModel.ts | 6 +++ .../common/textFileSaveParticipant.ts | 13 +++++- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts index a1991fc9fea26..0f1cc24f54e2d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Queue } from '../../../../base/common/async.js'; +import { CancellationError } from '../../../../base/common/errors.js'; import { Disposable, DisposableMap, DisposableStore, MutableDisposable } from '../../../../base/common/lifecycle.js'; import { localize, localize2 } from '../../../../nls.js'; import { Action2, registerAction2 } from '../../../../platform/actions/common/actions.js'; @@ -20,7 +21,7 @@ import { IChatEditingService, IChatEditingSession, WorkingSetEntryState } from ' import { CHAT_CATEGORY } from './actions/chatActions.js'; -const _storageKey = 'workbench.chat.editorSaving'; +const _storageKey = 'workbench.chat.saveWithAiGeneratedChanges'; export class ChatEditorSaving extends Disposable implements IWorkbenchContribution { @@ -48,8 +49,8 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi store.clear(); - const alwaysAcceptOnSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); - if (alwaysAcceptOnSave) { + const alwaysSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); + if (alwaysSave) { return; } @@ -78,9 +79,8 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi await queue.queue(async () => { // this might have changed in the meantime and there is checked again and acted upon - const alwaysAcceptOnSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); - if (alwaysAcceptOnSave) { - await session.accept(workingCopy.resource); + const alwaysSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); + if (alwaysSave) { return; } @@ -88,30 +88,28 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi const filelabel = labelService.getUriBasenameLabel(workingCopy.resource); const message = agentName - ? localize('message.1', "Do you want to accept the changes {0} made in {1}?", agentName, filelabel) - : localize('message.2', "Do you want to accept the changes chat made in {1}?", filelabel); + ? localize('message.1', "Do you want to save the changes {0} made in {1}?", agentName, filelabel) + : localize('message.2', "Do you want to save the changes chat made in {0}?", filelabel); const result = await this._dialogService.confirm({ message, detail: localize('detail', "AI-generated changes may be incorect and should be reviewed before saving.", agentName), - primaryButton: localize('save', "Accept & Save"), - cancelButton: localize('discard', "Discard & Save"), + primaryButton: localize('save', "Save"), + cancelButton: localize('discard', "Cancel"), checkbox: { - label: localize('config', "Always accept edits when saving"), + label: localize('config', "Always save with AI-generated changes"), checked: false } }); - if (result.confirmed) { - await session.accept(workingCopy.resource); - - if (result.checkboxChecked) { - // remember choice - this._storageService.store(_storageKey, true, StorageScope.PROFILE, StorageTarget.USER); - } + if (!result.confirmed) { + // cancel the save + throw new CancellationError(); + } - } else { - await session.reject(workingCopy.resource); + if (result.checkboxChecked) { + // remember choice + this._storageService.store(_storageKey, true, StorageScope.PROFILE, StorageTarget.USER); } }); } @@ -159,7 +157,7 @@ registerAction2(class extends Action2 { constructor() { super({ id: 'workbench.action.resetChatEditorSaving', - title: localize2('resetChatEditorSaving', "Reset Choise for 'Always accept edits when saving'"), + title: localize2('resetChatEditorSaving', "Reset Choise for 'Always save with AI-generated changes'"), category: CHAT_CATEGORY, f1: true }); diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index febcf153c8b04..31b4827edfb0f 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -34,6 +34,7 @@ import { PLAINTEXT_LANGUAGE_ID } from '../../../../editor/common/languages/modes import { IExtensionService } from '../../extensions/common/extensions.js'; import { IMarkdownString } from '../../../../base/common/htmlContent.js'; import { IProgress, IProgressService, IProgressStep, ProgressLocation } from '../../../../platform/progress/common/progress.js'; +import { isCancellationError } from '../../../../base/common/errors.js'; interface IBackupMetaData extends IWorkingCopyBackupMeta { mtime: number; @@ -870,6 +871,11 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.ignoreSaveFromSaveParticipants = true; try { await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, progress, saveCancellation.token); + } catch (err) { + if (isCancellationError(err) && !saveCancellation.token.isCancellationRequested) { + // participant wants to cancel this operation + saveCancellation.cancel(); + } } finally { this.ignoreSaveFromSaveParticipants = false; } diff --git a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts index 80f5d3a27cbd4..521e78198e8aa 100644 --- a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts +++ b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts @@ -12,7 +12,7 @@ import { IDisposable, Disposable, toDisposable } from '../../../../base/common/l import { LinkedList } from '../../../../base/common/linkedList.js'; import { localize } from '../../../../nls.js'; import { NotificationPriority } from '../../../../platform/notification/common/notification.js'; -import { isCancellationError } from '../../../../base/common/errors.js'; +import { CancellationError, isCancellationError } from '../../../../base/common/errors.js'; export class TextFileSaveParticipant extends Disposable { @@ -42,6 +42,8 @@ export class TextFileSaveParticipant extends Disposable { message: localize('saveParticipants1', "Running Code Actions and Formatters...") }); + let bubbleCancel = false; + // create an "inner" progress to allow to skip over long running save participants await this.progressService.withProgress({ priority: NotificationPriority.URGENT, @@ -60,6 +62,11 @@ export class TextFileSaveParticipant extends Disposable { } catch (err) { if (!isCancellationError(err)) { this.logService.error(err); + } else if (!cts.token.isCancellationRequested) { + // we see a cancellation error BUT the token didn't signal it + // this means the participant wants the save operation to be cancelled + cts.cancel(); + bubbleCancel = true; } } } @@ -71,6 +78,10 @@ export class TextFileSaveParticipant extends Disposable { model.textEditorModel?.pushStackElement(); cts.dispose(); + + if (bubbleCancel) { + throw new CancellationError(); + } } override dispose(): void {