Skip to content

Commit

Permalink
simplify save flow (microsoft#230883)
Browse files Browse the repository at this point in the history
  • Loading branch information
jrieken authored Oct 9, 2024
1 parent a016ec9 commit 8329e3a
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 22 deletions.
40 changes: 19 additions & 21 deletions src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -78,40 +79,37 @@ 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;
}

const agentName = chatAgentService.getDefaultAgent(ChatAgentLocation.EditingSession)?.fullName;
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);
}
});
}
Expand Down Expand Up @@ -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
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
}
}
Expand All @@ -71,6 +78,10 @@ export class TextFileSaveParticipant extends Disposable {
model.textEditorModel?.pushStackElement();

cts.dispose();

if (bubbleCancel) {
throw new CancellationError();
}
}

override dispose(): void {
Expand Down

0 comments on commit 8329e3a

Please sign in to comment.