Skip to content

Commit

Permalink
Show progress indication when saving an editor takes long (fix #193230)…
Browse files Browse the repository at this point in the history
… (#214128)
  • Loading branch information
bpasero authored Jun 3, 2024
1 parent 7e451df commit b5bd304
Show file tree
Hide file tree
Showing 15 changed files with 105 additions and 92 deletions.
25 changes: 22 additions & 3 deletions src/vs/workbench/services/textfile/common/textFileEditorModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { IAccessibilityService } from 'vs/platform/accessibility/common/accessib
import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IMarkdownString } from 'vs/base/common/htmlContent';
import { IProgress, IProgressService, IProgressStep, ProgressLocation } from 'vs/platform/progress/common/progress';

interface IBackupMetaData extends IWorkingCopyBackupMeta {
mtime: number;
Expand Down Expand Up @@ -123,7 +124,8 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
@ILanguageDetectionService languageDetectionService: ILanguageDetectionService,
@IAccessibilityService accessibilityService: IAccessibilityService,
@IPathService private readonly pathService: IPathService,
@IExtensionService private readonly extensionService: IExtensionService
@IExtensionService private readonly extensionService: IExtensionService,
@IProgressService private readonly progressService: IProgressService
) {
super(modelService, languageService, languageDetectionService, accessibilityService);

Expand Down Expand Up @@ -756,7 +758,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
options.reason = SaveReason.EXPLICIT;
}

let versionId = this.versionId;
const versionId = this.versionId;
this.trace(`doSave(${versionId}) - enter with versionId ${versionId}`);

// Return early if saved from within save participant to break recursion
Expand Down Expand Up @@ -818,6 +820,22 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil

const saveCancellation = new CancellationTokenSource();

return this.progressService.withProgress({
title: localize('saveParticipants', "Saving '{0}'", this.name),
location: ProgressLocation.Window,
cancellable: true,
delay: this.isDirty() ? 3000 : 5000,
type: 'loading'
}, progress => {
return this.doSaveSequential(versionId, options, progress, saveCancellation);
}, () => {
saveCancellation.cancel();
}).finally(() => {
saveCancellation.dispose();
});
}

private doSaveSequential(versionId: number, options: ITextFileSaveAsOptions, progress: IProgress<IProgressStep>, saveCancellation: CancellationTokenSource): Promise<void> {
return this.saveSequentializer.run(versionId, (async () => {

// A save participant can still change the model now and since we are so close to saving
Expand Down Expand Up @@ -852,7 +870,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
if (!saveCancellation.token.isCancellationRequested) {
this.ignoreSaveFromSaveParticipants = true;
try {
await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, saveCancellation.token);
await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, progress, saveCancellation.token);
} finally {
this.ignoreSaveFromSaveParticipants = false;
}
Expand Down Expand Up @@ -898,6 +916,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
// Save to Disk. We mark the save operation as currently running with
// the latest versionId because it might have changed from a save
// participant triggering
progress.report({ message: localize('saveTextFile', "Writing into file...") });
this.trace(`doSave(${versionId}) - before write()`);
const lastResolvedFileStat = assertIsDefined(this.lastResolvedFileStat);
const resolvedTextFileEditorModel = this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { extname, joinPath } from 'vs/base/common/resources';
import { createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel';
import { PLAINTEXT_EXTENSION, PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress';

export class TextFileEditorModelManager extends Disposable implements ITextFileEditorModelManager {

Expand Down Expand Up @@ -540,8 +541,8 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
return this.saveParticipants.addSaveParticipant(participant);
}

runSaveParticipants(model: ITextFileEditorModel, context: IStoredFileWorkingCopySaveParticipantContext, token: CancellationToken): Promise<void> {
return this.saveParticipants.participate(model, context, token);
runSaveParticipants(model: ITextFileEditorModel, context: IStoredFileWorkingCopySaveParticipantContext, progress: IProgress<IProgressStep>, token: CancellationToken): Promise<void> {
return this.saveParticipants.participate(model, context, progress, token);
}

//#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { localize } from 'vs/nls';
import { raceCancellation } from 'vs/base/common/async';
import { CancellationTokenSource, CancellationToken } from 'vs/base/common/cancellation';
import { CancellationToken } from 'vs/base/common/cancellation';
import { ILogService } from 'vs/platform/log/common/log';
import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/progress';
import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress';
import { ITextFileSaveParticipant, ITextFileEditorModel, ITextFileSaveParticipantContext } from 'vs/workbench/services/textfile/common/textfiles';
import { IDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle';
import { insert } from 'vs/base/common/arrays';
Expand All @@ -17,7 +16,6 @@ export class TextFileSaveParticipant extends Disposable {
private readonly saveParticipants: ITextFileSaveParticipant[] = [];

constructor(
@IProgressService private readonly progressService: IProgressService,
@ILogService private readonly logService: ILogService
) {
super();
Expand All @@ -29,40 +27,26 @@ export class TextFileSaveParticipant extends Disposable {
return toDisposable(() => remove());
}

participate(model: ITextFileEditorModel, context: ITextFileSaveParticipantContext, token: CancellationToken): Promise<void> {
const cts = new CancellationTokenSource(token);
async participate(model: ITextFileEditorModel, context: ITextFileSaveParticipantContext, progress: IProgress<IProgressStep>, token: CancellationToken): Promise<void> {

return this.progressService.withProgress({
title: localize('saveParticipants', "Saving '{0}'", model.name),
location: ProgressLocation.Notification,
cancellable: true,
delay: model.isDirty() ? 3000 : 5000
}, async progress => {
// undoStop before participation
model.textEditorModel?.pushStackElement();

// undoStop before participation
model.textEditorModel?.pushStackElement();

for (const saveParticipant of this.saveParticipants) {
if (cts.token.isCancellationRequested || !model.textEditorModel /* disposed */) {
break;
}
for (const saveParticipant of this.saveParticipants) {
if (token.isCancellationRequested || !model.textEditorModel /* disposed */) {
break;
}

try {
const promise = saveParticipant.participate(model, context, progress, cts.token);
await raceCancellation(promise, cts.token);
} catch (err) {
this.logService.error(err);
}
try {
const promise = saveParticipant.participate(model, context, progress, token);
await raceCancellation(promise, token);
} catch (err) {
this.logService.error(err);
}
}

// undoStop after participation
model.textEditorModel?.pushStackElement();
}, () => {
// user cancel
cts.cancel();
}).finally(() => {
cts.dispose();
});
// undoStop after participation
model.textEditorModel?.pushStackElement();
}

override dispose(): void {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/textfile/common/textfiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ export interface ITextFileEditorModelManager {
/**
* Runs the registered save participants on the provided model.
*/
runSaveParticipants(model: ITextFileEditorModel, context: ITextFileSaveParticipantContext, token: CancellationToken): Promise<void>;
runSaveParticipants(model: ITextFileEditorModel, context: ITextFileSaveParticipantContext, progress: IProgress<IProgressStep>, token: CancellationToken): Promise<void>;

/**
* Waits for the model to be ready to be disposed. There may be conditions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { Schemas } from 'vs/base/common/network';
import { IDecorationData, IDecorationsProvider, IDecorationsService } from 'vs/workbench/services/decorations/common/decorations';
import { Codicon } from 'vs/base/common/codicons';
import { listErrorForeground } from 'vs/platform/theme/common/colorRegistry';
import { IProgressService } from 'vs/platform/progress/common/progress';

export interface IFileWorkingCopyManager<S extends IStoredFileWorkingCopyModel, U extends IUntitledFileWorkingCopyModel> extends IBaseFileWorkingCopyManager<S | U, IFileWorkingCopy<S | U>> {

Expand Down Expand Up @@ -162,7 +163,8 @@ export class FileWorkingCopyManager<S extends IStoredFileWorkingCopyModel, U ext
@IPathService private readonly pathService: IPathService,
@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
@IDialogService private readonly dialogService: IDialogService,
@IDecorationsService private readonly decorationsService: IDecorationsService
@IDecorationsService private readonly decorationsService: IDecorationsService,
@IProgressService progressService: IProgressService
) {
super();

Expand All @@ -172,7 +174,7 @@ export class FileWorkingCopyManager<S extends IStoredFileWorkingCopyModel, U ext
this.storedWorkingCopyModelFactory,
fileService, lifecycleService, labelService, logService, workingCopyFileService,
workingCopyBackupService, uriIdentityService, filesConfigurationService, workingCopyService,
notificationService, workingCopyEditorService, editorService, elevatedFileService
notificationService, workingCopyEditorService, editorService, elevatedFileService, progressService
));

// Untitled file working copies manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { IElevatedFileService } from 'vs/workbench/services/files/common/elevate
import { IResourceWorkingCopy, ResourceWorkingCopy } from 'vs/workbench/services/workingCopy/common/resourceWorkingCopy';
import { IFileWorkingCopy, IFileWorkingCopyModel, IFileWorkingCopyModelFactory, SnapshotContext } from 'vs/workbench/services/workingCopy/common/fileWorkingCopy';
import { IMarkdownString } from 'vs/base/common/htmlContent';
import { IProgress, IProgressService, IProgressStep, ProgressLocation } from 'vs/platform/progress/common/progress';

/**
* Stored file specific working copy model factory.
Expand Down Expand Up @@ -359,7 +360,8 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
@INotificationService private readonly notificationService: INotificationService,
@IWorkingCopyEditorService private readonly workingCopyEditorService: IWorkingCopyEditorService,
@IEditorService private readonly editorService: IEditorService,
@IElevatedFileService private readonly elevatedFileService: IElevatedFileService
@IElevatedFileService private readonly elevatedFileService: IElevatedFileService,
@IProgressService private readonly progressService: IProgressService
) {
super(resource, fileService);

Expand Down Expand Up @@ -865,7 +867,7 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
options.reason = SaveReason.EXPLICIT;
}

let versionId = this.versionId;
const versionId = this.versionId;
this.trace(`doSave(${versionId}) - enter with versionId ${versionId}`);

// Return early if saved from within save participant to break recursion
Expand Down Expand Up @@ -929,6 +931,22 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend

const saveCancellation = new CancellationTokenSource();

return this.progressService.withProgress({
title: localize('saveParticipants', "Saving '{0}'", this.name),
location: ProgressLocation.Window,
cancellable: true,
delay: this.isDirty() ? 3000 : 5000,
type: 'loading'
}, progress => {
return this.doSaveSequential(versionId, options, progress, saveCancellation);
}, () => {
saveCancellation.cancel();
}).finally(() => {
saveCancellation.dispose();
});
}

private doSaveSequential(versionId: number, options: IStoredFileWorkingCopySaveAsOptions, progress: IProgress<IProgressStep>, saveCancellation: CancellationTokenSource): Promise<void> {
return this.saveSequentializer.run(versionId, (async () => {

// A save participant can still change the working copy now
Expand Down Expand Up @@ -964,7 +982,7 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
if (!saveCancellation.token.isCancellationRequested) {
this.ignoreSaveFromSaveParticipants = true;
try {
await this.workingCopyFileService.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, saveCancellation.token);
await this.workingCopyFileService.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, progress, saveCancellation.token);
} finally {
this.ignoreSaveFromSaveParticipants = false;
}
Expand Down Expand Up @@ -1004,6 +1022,7 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
// Save to Disk. We mark the save operation as currently running with
// the latest versionId because it might have changed from a save
// participant triggering
progress.report({ message: localize('saveTextFile', "Writing into file...") });
this.trace(`doSave(${versionId}) - before write()`);
const lastResolvedFileStat = assertIsDefined(this.lastResolvedFileStat);
const resolvedFileWorkingCopy = this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/wo
import { isWeb } from 'vs/base/common/platform';
import { onUnexpectedError } from 'vs/base/common/errors';
import { SnapshotContext } from 'vs/workbench/services/workingCopy/common/fileWorkingCopy';
import { IProgressService } from 'vs/platform/progress/common/progress';

/**
* The only one that should be dealing with `IStoredFileWorkingCopy` and handle all
Expand Down Expand Up @@ -186,7 +187,8 @@ export class StoredFileWorkingCopyManager<M extends IStoredFileWorkingCopyModel>
@INotificationService private readonly notificationService: INotificationService,
@IWorkingCopyEditorService private readonly workingCopyEditorService: IWorkingCopyEditorService,
@IEditorService private readonly editorService: IEditorService,
@IElevatedFileService private readonly elevatedFileService: IElevatedFileService
@IElevatedFileService private readonly elevatedFileService: IElevatedFileService,
@IProgressService private readonly progressService: IProgressService
) {
super(fileService, logService, workingCopyBackupService);

Expand Down Expand Up @@ -532,7 +534,7 @@ export class StoredFileWorkingCopyManager<M extends IStoredFileWorkingCopyModel>
async options => { await this.resolve(resource, { ...options, reload: { async: false } }); },
this.fileService, this.logService, this.workingCopyFileService, this.filesConfigurationService,
this.workingCopyBackupService, this.workingCopyService, this.notificationService, this.workingCopyEditorService,
this.editorService, this.elevatedFileService
this.editorService, this.elevatedFileService, this.progressService
);

workingCopyResolve = workingCopy.resolve(resolveOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { localize } from 'vs/nls';
import { raceCancellation } from 'vs/base/common/async';
import { CancellationTokenSource, CancellationToken } from 'vs/base/common/cancellation';
import { CancellationToken } from 'vs/base/common/cancellation';
import { ILogService } from 'vs/platform/log/common/log';
import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/progress';
import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress';
import { IDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle';
import { insert } from 'vs/base/common/arrays';
import { IStoredFileWorkingCopySaveParticipant, IStoredFileWorkingCopySaveParticipantContext } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
Expand All @@ -20,7 +19,6 @@ export class StoredFileWorkingCopySaveParticipant extends Disposable {
get length(): number { return this.saveParticipants.length; }

constructor(
@IProgressService private readonly progressService: IProgressService,
@ILogService private readonly logService: ILogService
) {
super();
Expand All @@ -32,41 +30,26 @@ export class StoredFileWorkingCopySaveParticipant extends Disposable {
return toDisposable(() => remove());
}

participate(workingCopy: IStoredFileWorkingCopy<IStoredFileWorkingCopyModel>, context: IStoredFileWorkingCopySaveParticipantContext, token: CancellationToken): Promise<void> {
const cts = new CancellationTokenSource(token);
async participate(workingCopy: IStoredFileWorkingCopy<IStoredFileWorkingCopyModel>, context: IStoredFileWorkingCopySaveParticipantContext, progress: IProgress<IProgressStep>, token: CancellationToken): Promise<void> {

return this.progressService.withProgress({
title: localize('saveParticipants', "Saving '{0}'", workingCopy.name),
location: ProgressLocation.Notification,
cancellable: true,
delay: workingCopy.isDirty() ? 3000 : 5000
}, async progress => {
// undoStop before participation
workingCopy.model?.pushStackElement();

// undoStop before participation
workingCopy.model?.pushStackElement();

for (const saveParticipant of this.saveParticipants) {
if (cts.token.isCancellationRequested || workingCopy.isDisposed()) {
break;
}

try {
const promise = saveParticipant.participate(workingCopy, context, progress, cts.token);
await raceCancellation(promise, cts.token);
} catch (err) {
this.logService.warn(err);
}
for (const saveParticipant of this.saveParticipants) {
if (token.isCancellationRequested || workingCopy.isDisposed()) {
break;
}

// undoStop after participation
workingCopy.model?.pushStackElement();
try {
const promise = saveParticipant.participate(workingCopy, context, progress, token);
await raceCancellation(promise, token);
} catch (err) {
this.logService.warn(err);
}
}

// Cleanup
cts.dispose();
}, () => {
// user cancel
cts.dispose(true);
});
// undoStop after participation
workingCopy.model?.pushStackElement();
}

override dispose(): void {
Expand Down
Loading

0 comments on commit b5bd304

Please sign in to comment.