From 5a92e63dbcd2d5333327577b71f2dc2c35c4d7b3 Mon Sep 17 00:00:00 2001 From: aamunger Date: Thu, 25 May 2023 14:21:09 -0700 Subject: [PATCH 1/6] test no veto when scratchpads should backup --- .../workingCopyBackupTracker.test.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts index e2398e4ae67..b8a7e58d6d1 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts @@ -423,6 +423,49 @@ suite('WorkingCopyBackupTracker (native)', function () { await cleanup(); }); + test('onWillShutdown - scratchpads - no veto if backed up', async function () { + const { accessor, cleanup } = await createTracker(); + + class TestBackupWorkingCopy extends TestWorkingCopy { + + constructor(resource: URI) { + super(resource); + + accessor.workingCopyService.registerWorkingCopy(this); + } + + override capabilities = WorkingCopyCapabilities.Untitled | WorkingCopyCapabilities.Scratchpad; + + override isDirty(): boolean { + return false; + } + + override isModified(): boolean { + return true; + } + } + + accessor.filesConfigurationService.testOnFilesConfigurationChange({ files: { hotExit: HotExitConfiguration.ON_EXIT } }); + accessor.nativeHostService.windowCount = Promise.resolve(2); + // Set cancel to force a veto if hot exit does not trigger + accessor.fileDialogService.setConfirmResult(ConfirmResult.CANCEL); + + const resource = toResource.call(this, '/path/custom.txt'); + new TestBackupWorkingCopy(resource); + + const event = new TestBeforeShutdownEvent(); + event.reason = ShutdownReason.CLOSE; + accessor.lifecycleService.fireBeforeShutdown(event); + + const veto = await event.value; + assert.ok(!veto); + + const finalVeto = await event.finalValue?.(); + assert.ok(!finalVeto); // assert the tracker uses the internal finalVeto API + + await cleanup(); + }); + test('onWillShutdown - pending backup operations canceled and tracker suspended/resumsed', async function () { const { accessor, tracker, cleanup } = await createTracker(); From 4cb8a4874605849c950c8c02f401392b641f4fa1 Mon Sep 17 00:00:00 2001 From: aamunger Date: Thu, 25 May 2023 14:52:47 -0700 Subject: [PATCH 2/6] test hot exit for scratchpads --- .../workingCopyBackupTracker.ts | 38 ++-- .../workingCopyBackupTracker.test.ts | 201 ++++++++++++++---- 2 files changed, 175 insertions(+), 64 deletions(-) diff --git a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts index 622cbc8c48d..c6712a6f89a 100644 --- a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts +++ b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts @@ -114,10 +114,10 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp // Trigger backup if configured and enabled for shutdown reason let backups: IWorkingCopy[] = []; let backupError: Error | undefined = undefined; - const backup = await this.shouldBackupBeforeShutdown(reason); - if (backup) { + const copiesToBackup = await this.shouldBackupBeforeShutdown(reason, modifiedWorkingCopies); + if (copiesToBackup.length > 0) { try { - const backupResult = await this.backupBeforeShutdown(modifiedWorkingCopies); + const backupResult = await this.backupBeforeShutdown(copiesToBackup); backups = backupResult.backups; backupError = backupResult.error; @@ -162,12 +162,12 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp } } - private async shouldBackupBeforeShutdown(reason: ShutdownReason): Promise { - let backup: boolean | undefined; + private async shouldBackupBeforeShutdown(reason: ShutdownReason, modifiedWorkingCopies: readonly IWorkingCopy[]): Promise { + if (!this.filesConfigurationService.isHotExitEnabled) { - backup = false; // never backup when hot exit is disabled via settings + return []; // never backup when hot exit is disabled via settings } else if (this.environmentService.isExtensionDevelopment) { - backup = true; // always backup closing extension development window without asking to speed up debugging + return modifiedWorkingCopies; // always backup closing extension development window without asking to speed up debugging } else { // When quit is requested skip the confirm callback and attempt to backup all workspaces. @@ -178,33 +178,29 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp switch (reason) { case ShutdownReason.CLOSE: if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { - backup = true; // backup if a folder is open and onExitAndWindowClose is configured + return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured } else if (await this.nativeHostService.getWindowCount() > 1 || isMacintosh) { - backup = false; // do not backup if a window is closed that does not cause quitting of the application + if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration !== HotExitConfiguration.OFF) { + return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpad working copies + } + return []; // do not backup if a window is closed that does not cause quitting of the application } else { - backup = true; // backup if last window is closed on win/linux where the application quits right after + return modifiedWorkingCopies; // backup if last window is closed on win/linux where the application quits right after } - break; - case ShutdownReason.QUIT: - backup = true; // backup because next start we restore all backups - break; + return modifiedWorkingCopies; // backup because next start we restore all backups case ShutdownReason.RELOAD: - backup = true; // backup because after window reload, backups restore - break; + return modifiedWorkingCopies; // backup because after window reload, backups restore case ShutdownReason.LOAD: if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { - backup = true; // backup if a folder is open and onExitAndWindowClose is configured + return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured } else { - backup = false; // do not backup because we are switching contexts + return []; // do not backup because we are switching contexts } - break; } } - - return backup; } private showErrorDialog(msg: string, workingCopies: readonly IWorkingCopy[], error?: Error): void { diff --git a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts index b8a7e58d6d1..666beb46edc 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts @@ -423,49 +423,6 @@ suite('WorkingCopyBackupTracker (native)', function () { await cleanup(); }); - test('onWillShutdown - scratchpads - no veto if backed up', async function () { - const { accessor, cleanup } = await createTracker(); - - class TestBackupWorkingCopy extends TestWorkingCopy { - - constructor(resource: URI) { - super(resource); - - accessor.workingCopyService.registerWorkingCopy(this); - } - - override capabilities = WorkingCopyCapabilities.Untitled | WorkingCopyCapabilities.Scratchpad; - - override isDirty(): boolean { - return false; - } - - override isModified(): boolean { - return true; - } - } - - accessor.filesConfigurationService.testOnFilesConfigurationChange({ files: { hotExit: HotExitConfiguration.ON_EXIT } }); - accessor.nativeHostService.windowCount = Promise.resolve(2); - // Set cancel to force a veto if hot exit does not trigger - accessor.fileDialogService.setConfirmResult(ConfirmResult.CANCEL); - - const resource = toResource.call(this, '/path/custom.txt'); - new TestBackupWorkingCopy(resource); - - const event = new TestBeforeShutdownEvent(); - event.reason = ShutdownReason.CLOSE; - accessor.lifecycleService.fireBeforeShutdown(event); - - const veto = await event.value; - assert.ok(!veto); - - const finalVeto = await event.finalValue?.(); - assert.ok(!finalVeto); // assert the tracker uses the internal finalVeto API - - await cleanup(); - }); - test('onWillShutdown - pending backup operations canceled and tracker suspended/resumsed', async function () { const { accessor, tracker, cleanup } = await createTracker(); @@ -608,6 +565,109 @@ suite('WorkingCopyBackupTracker (native)', function () { }); }); + suite('"onExit" setting - scratchpad', () => { + test('should hot exit (reason: CLOSE, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.CLOSE, false, true, false); + }); + test('should hot exit (reason: CLOSE, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.CLOSE, false, false, false); + }); + test('should hot exit (reason: CLOSE, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.CLOSE, true, true, false); + }); + test('should NOT hot exit (reason: CLOSE, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.CLOSE, true, false, true); + }); + test('should hot exit (reason: QUIT, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.QUIT, false, true, false); + }); + test('should hot exit (reason: QUIT, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.QUIT, false, false, false); + }); + test('should hot exit (reason: QUIT, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.QUIT, true, true, false); + }); + test('should hot exit (reason: QUIT, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.QUIT, true, false, false); + }); + test('should hot exit (reason: RELOAD, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.RELOAD, false, true, false); + }); + test('should hot exit (reason: RELOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.RELOAD, false, false, false); + }); + test('should hot exit (reason: RELOAD, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.RELOAD, true, true, false); + }); + test('should hot exit (reason: RELOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.RELOAD, true, false, false); + }); + test('should NOT hot exit (reason: LOAD, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, true, true); + }); + test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, false, true); + }); + test('should NOT hot exit (reason: LOAD, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, true, true); + }); + test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, false, true); + }); + }); + + suite('"onExitAndWindowClose" setting - scratchpad', () => { + test('should hot exit (reason: CLOSE, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.CLOSE, false, true, false); + }); + test('should hot exit (reason: CLOSE, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.CLOSE, false, false, !!isMacintosh); + }); + test('should hot exit (reason: CLOSE, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.CLOSE, true, true, false); + }); + test('should NOT hot exit (reason: CLOSE, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.CLOSE, true, false, true); + }); + test('should hot exit (reason: QUIT, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.QUIT, false, true, false); + }); + test('should hot exit (reason: QUIT, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.QUIT, false, false, false); + }); + test('should hot exit (reason: QUIT, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.QUIT, true, true, false); + }); + test('should hot exit (reason: QUIT, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.QUIT, true, false, false); + }); + test('should hot exit (reason: RELOAD, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.RELOAD, false, true, false); + }); + test('should hot exit (reason: RELOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.RELOAD, false, false, false); + }); + test('should hot exit (reason: RELOAD, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.RELOAD, true, true, false); + }); + test('should hot exit (reason: RELOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.RELOAD, true, false, false); + }); + test('should hot exit (reason: LOAD, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, true, false); + }); + test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, false, true); + }); + test('should hot exit (reason: LOAD, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, true, false); + }); + test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, false, true); + }); + }); + + async function hotExitTest(this: any, setting: string, shutdownReason: ShutdownReason, multipleWindows: boolean, workspace: boolean, shouldVeto: boolean): Promise { const { accessor, cleanup } = await createTracker(); @@ -647,5 +707,60 @@ suite('WorkingCopyBackupTracker (native)', function () { await cleanup(); } + + async function scratchpadHotExitTest(this: any, setting: string, shutdownReason: ShutdownReason, multipleWindows: boolean, workspace: boolean, shouldVeto: boolean): Promise { + const { accessor, cleanup } = await createTracker(); + + class TestBackupWorkingCopy extends TestWorkingCopy { + + constructor(resource: URI) { + super(resource); + + accessor.workingCopyService.registerWorkingCopy(this); + } + + override capabilities = WorkingCopyCapabilities.Untitled | WorkingCopyCapabilities.Scratchpad; + + override isDirty(): boolean { + return false; + } + + override isModified(): boolean { + return true; + } + } + + // Set hot exit config + accessor.filesConfigurationService.testOnFilesConfigurationChange({ files: { hotExit: setting } }); + + // Set empty workspace if required + if (!workspace) { + accessor.contextService.setWorkspace(new Workspace('empty:1508317022751')); + } + + // Set multiple windows if required + if (multipleWindows) { + accessor.nativeHostService.windowCount = Promise.resolve(2); + } + + // Set cancel to force a veto if hot exit does not trigger + accessor.fileDialogService.setConfirmResult(ConfirmResult.CANCEL); + + const resource = toResource.call(this, '/path/custom.txt'); + new TestBackupWorkingCopy(resource); + + const event = new TestBeforeShutdownEvent(); + event.reason = shutdownReason; + accessor.lifecycleService.fireBeforeShutdown(event); + + const veto = await event.value; + assert.ok(typeof event.finalValue === 'function'); // assert the tracker uses the internal finalVeto API + assert.strictEqual(accessor.workingCopyBackupService.discardedBackups.length, 0); // When hot exit is set, backups should never be cleaned since the confirm result is cancel + assert.strictEqual(veto, shouldVeto); + + await cleanup(); + + await cleanup(); + } }); }); From f39299b594093fa367bf626344f99132e626f2c2 Mon Sep 17 00:00:00 2001 From: aamunger Date: Fri, 26 May 2023 10:14:07 -0700 Subject: [PATCH 3/6] backup and recover scratchpads for exitreason=load --- .../workingCopyBackupTracker.ts | 2 +- .../workingCopyBackupTracker.test.ts | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts index c6712a6f89a..3d16ec0ca77 100644 --- a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts +++ b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts @@ -197,7 +197,7 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured } else { - return []; // do not backup because we are switching contexts + return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpads because we are switching contexts } } } diff --git a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts index 666beb46edc..9fbb82e850d 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts @@ -602,17 +602,17 @@ suite('WorkingCopyBackupTracker (native)', function () { test('should hot exit (reason: RELOAD, windows: multiple, empty workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.RELOAD, true, false, false); }); - test('should NOT hot exit (reason: LOAD, windows: single, workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, true, true); + test('should hot exit (reason: LOAD, windows: single, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, true, false); }); - test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, false, true); + test('should hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, false, false); }); - test('should NOT hot exit (reason: LOAD, windows: multiple, workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, true, true); + test('should hot exit (reason: LOAD, windows: multiple, workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, true, false); }); - test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, false, true); + test('should hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, false, false); }); }); @@ -656,14 +656,14 @@ suite('WorkingCopyBackupTracker (native)', function () { test('should hot exit (reason: LOAD, windows: single, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, true, false); }); - test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, false, true); + test('should hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, false, false); }); test('should hot exit (reason: LOAD, windows: multiple, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, true, false); }); - test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, false, true); + test('should hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, false, false); }); }); From b4940e6446802611a3b1ca63930ecc6b56407ce2 Mon Sep 17 00:00:00 2001 From: aamunger Date: Fri, 26 May 2023 11:08:35 -0700 Subject: [PATCH 4/6] hygiene --- .../electron-sandbox/workingCopyBackupTracker.ts | 10 +++++++--- .../workingCopyBackupTracker.test.ts | 16 ++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts index 3d16ec0ca77..e15698707a4 100644 --- a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts +++ b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts @@ -194,10 +194,14 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp return modifiedWorkingCopies; // backup because after window reload, backups restore case ShutdownReason.LOAD: - if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { - return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured + if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY) { + if (this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { + return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured + } else { + return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpads because we are switching contexts + } } else { - return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpads because we are switching contexts + return []; // do not backup because we are switching contexts from an empty workspace } } } diff --git a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts index 9fbb82e850d..dedbe3c0f79 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-sandbox/workingCopyBackupTracker.test.ts @@ -605,14 +605,14 @@ suite('WorkingCopyBackupTracker (native)', function () { test('should hot exit (reason: LOAD, windows: single, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, true, false); }); - test('should hot exit (reason: LOAD, windows: single, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, false, false); + test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, false, false, true); }); test('should hot exit (reason: LOAD, windows: multiple, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, true, false); }); - test('should hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, false, false); + test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT, ShutdownReason.LOAD, true, false, true); }); }); @@ -656,14 +656,14 @@ suite('WorkingCopyBackupTracker (native)', function () { test('should hot exit (reason: LOAD, windows: single, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, true, false); }); - test('should hot exit (reason: LOAD, windows: single, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, false, false); + test('should NOT hot exit (reason: LOAD, windows: single, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, false, false, true); }); test('should hot exit (reason: LOAD, windows: multiple, workspace)', function () { return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, true, false); }); - test('should hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { - return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, false, false); + test('should NOT hot exit (reason: LOAD, windows: multiple, empty workspace)', function () { + return scratchpadHotExitTest.call(this, HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE, ShutdownReason.LOAD, true, false, true); }); }); From ba19b49b7a008488d967e16f5b0c4964f6d61ec2 Mon Sep 17 00:00:00 2001 From: aamunger Date: Fri, 26 May 2023 11:11:48 -0700 Subject: [PATCH 5/6] fix comment --- .../workingCopy/electron-sandbox/workingCopyBackupTracker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts index e15698707a4..115124f9a2c 100644 --- a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts +++ b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts @@ -201,7 +201,7 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpads because we are switching contexts } } else { - return []; // do not backup because we are switching contexts from an empty workspace + return []; // do not backup because we are switching contexts with no folder open } } } From 2aa298a06f121b56e9dc90361d3008512dd18d02 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 27 May 2023 09:44:41 +0200 Subject: [PATCH 6/6] :lipstick: --- .../workingCopyBackupTracker.ts | 76 ++++++++++--------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts index 115124f9a2c..0db78001751 100644 --- a/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts +++ b/src/vs/workbench/services/workingCopy/electron-sandbox/workingCopyBackupTracker.ts @@ -114,10 +114,10 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp // Trigger backup if configured and enabled for shutdown reason let backups: IWorkingCopy[] = []; let backupError: Error | undefined = undefined; - const copiesToBackup = await this.shouldBackupBeforeShutdown(reason, modifiedWorkingCopies); - if (copiesToBackup.length > 0) { + const modifiedWorkingCopiesToBackup = await this.shouldBackupBeforeShutdown(reason, modifiedWorkingCopies); + if (modifiedWorkingCopiesToBackup.length > 0) { try { - const backupResult = await this.backupBeforeShutdown(copiesToBackup); + const backupResult = await this.backupBeforeShutdown(modifiedWorkingCopiesToBackup); backups = backupResult.backups; backupError = backupResult.error; @@ -163,47 +163,51 @@ export class NativeWorkingCopyBackupTracker extends WorkingCopyBackupTracker imp } private async shouldBackupBeforeShutdown(reason: ShutdownReason, modifiedWorkingCopies: readonly IWorkingCopy[]): Promise { - if (!this.filesConfigurationService.isHotExitEnabled) { return []; // never backup when hot exit is disabled via settings - } else if (this.environmentService.isExtensionDevelopment) { + } + + if (this.environmentService.isExtensionDevelopment) { return modifiedWorkingCopies; // always backup closing extension development window without asking to speed up debugging - } else { + } - // When quit is requested skip the confirm callback and attempt to backup all workspaces. - // When quit is not requested the confirm callback should be shown when the window being - // closed is the only VS Code window open, except for on Mac where hot exit is only - // ever activated when quit is requested. + switch (reason) { - switch (reason) { - case ShutdownReason.CLOSE: - if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { - return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured - } else if (await this.nativeHostService.getWindowCount() > 1 || isMacintosh) { - if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration !== HotExitConfiguration.OFF) { - return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpad working copies - } - return []; // do not backup if a window is closed that does not cause quitting of the application - } else { - return modifiedWorkingCopies; // backup if last window is closed on win/linux where the application quits right after - } - case ShutdownReason.QUIT: - return modifiedWorkingCopies; // backup because next start we restore all backups + // Window Close + case ShutdownReason.CLOSE: + if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY && this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { + return modifiedWorkingCopies; // backup if a workspace/folder is open and onExitAndWindowClose is configured + } - case ShutdownReason.RELOAD: - return modifiedWorkingCopies; // backup because after window reload, backups restore - - case ShutdownReason.LOAD: + if (isMacintosh || await this.nativeHostService.getWindowCount() > 1) { if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY) { - if (this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { - return modifiedWorkingCopies; // backup if a folder is open and onExitAndWindowClose is configured - } else { - return modifiedWorkingCopies.filter(wc => wc.capabilities & WorkingCopyCapabilities.Scratchpad); // only backup scratchpads because we are switching contexts - } - } else { - return []; // do not backup because we are switching contexts with no folder open + return modifiedWorkingCopies.filter(modifiedWorkingCopy => modifiedWorkingCopy.capabilities & WorkingCopyCapabilities.Scratchpad); // backup scratchpads automatically to avoid user confirmation } - } + + return []; // do not backup if a window is closed that does not cause quitting of the application + } + + return modifiedWorkingCopies; // backup if last window is closed on win/linux where the application quits right after + + // Application Quit + case ShutdownReason.QUIT: + return modifiedWorkingCopies; // backup because next start we restore all backups + + // Window Reload + case ShutdownReason.RELOAD: + return modifiedWorkingCopies; // backup because after window reload, backups restore + + // Workspace Change + case ShutdownReason.LOAD: + if (this.contextService.getWorkbenchState() !== WorkbenchState.EMPTY) { + if (this.filesConfigurationService.hotExitConfiguration === HotExitConfiguration.ON_EXIT_AND_WINDOW_CLOSE) { + return modifiedWorkingCopies; // backup if a workspace/folder is open and onExitAndWindowClose is configured + } + + return modifiedWorkingCopies.filter(modifiedWorkingCopy => modifiedWorkingCopy.capabilities & WorkingCopyCapabilities.Scratchpad); // backup scratchpads automatically to avoid user confirmation + } + + return []; // do not backup because we are switching contexts with no workspace/folder open } }