diff --git a/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts index a63840db4e6..71b53f1ab25 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugConfigurationManager.test.ts @@ -20,8 +20,8 @@ import { UriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentitySe import { ConfigurationManager } from 'vs/workbench/contrib/debug/browser/debugConfigurationManager'; import { DebugConfigurationProviderTriggerKind, IAdapterManager, IConfig, IDebugAdapterExecutable, IDebugSession } from 'vs/workbench/contrib/debug/common/debug'; import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences'; -import { TestHistoryService, TestQuickInputService } from 'vs/workbench/test/browser/workbenchTestServices'; -import { TestContextService, TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; +import { TestQuickInputService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestHistoryService, TestContextService, TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; suite('debugConfigurationManager', () => { const configurationProviderType = 'custom-type'; diff --git a/src/vs/workbench/contrib/search/browser/quickTextSearch/textSearchQuickAccess.ts b/src/vs/workbench/contrib/search/browser/quickTextSearch/textSearchQuickAccess.ts index 15a07cb0675..3691e98ec1d 100644 --- a/src/vs/workbench/contrib/search/browser/quickTextSearch/textSearchQuickAccess.ts +++ b/src/vs/workbench/contrib/search/browser/quickTextSearch/textSearchQuickAccess.ts @@ -125,12 +125,15 @@ export class TextSearchQuickAccess extends PickerQuickAccessProvider { // disable and re-enable history service so that we can ignore this history entry - this._historyService.shouldIgnoreActiveEditorChange = true; - await this._editorService.openEditor({ - resource: itemMatch.parent().resource, - options: { preserveFocus: true, revealIfOpened: true, ignoreError: true, selection: itemMatch.range() } - }); - this._historyService.shouldIgnoreActiveEditorChange = false; + const disposable = this._historyService.suspendTracking(); + try { + await this._editorService.openEditor({ + resource: itemMatch.parent().resource, + options: { preserveFocus: true, revealIfOpened: true, ignoreError: true, selection: itemMatch.range() } + }); + } finally { + disposable.dispose(); + } }); } })); diff --git a/src/vs/workbench/services/history/browser/historyService.ts b/src/vs/workbench/services/history/browser/historyService.ts index 3c580614b86..108685831ad 100644 --- a/src/vs/workbench/services/history/browser/historyService.ts +++ b/src/vs/workbench/services/history/browser/historyService.ts @@ -12,7 +12,7 @@ import { IEditorService } from 'vs/workbench/services/editor/common/editorServic import { GoFilter, GoScope, IHistoryService } from 'vs/workbench/services/history/common/history'; import { FileChangesEvent, IFileService, FileChangeType, FILES_EXCLUDE_CONFIG, FileOperationEvent, FileOperation } from 'vs/platform/files/common/files'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; -import { dispose, Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; +import { dispose, Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import { Emitter, Event } from 'vs/base/common/event'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; @@ -47,9 +47,7 @@ export class HistoryService extends Disposable implements IHistoryService { private readonly editorHelper = this.instantiationService.createInstance(EditorHelper); - // Can be set to temporarily ignore messages from the editor service that indicate a new active editor. - // Used for ignoring some editors for history. - public shouldIgnoreActiveEditorChange: boolean = false; + constructor( @IEditorService private readonly editorService: EditorServiceImpl, @@ -75,6 +73,13 @@ export class HistoryService extends Disposable implements IHistoryService { } } + private trackingSuspended = false; + suspendTracking(): IDisposable { + this.trackingSuspended = true; + + return toDisposable(() => this.trackingSuspended = false); + } + private registerListeners(): void { // Mouse back/forward support @@ -82,14 +87,14 @@ export class HistoryService extends Disposable implements IHistoryService { // Editor changes this._register(this.editorService.onDidActiveEditorChange((e) => { - if (!this.shouldIgnoreActiveEditorChange) { + if (!this.trackingSuspended) { this.onDidActiveEditorChange(); } })); this._register(this.editorService.onDidOpenEditorFail(event => this.remove(event.editor))); this._register(this.editorService.onDidCloseEditor(event => this.onDidCloseEditor(event))); this._register(this.editorService.onDidMostRecentlyActiveEditorsChange(() => { - if (!this.shouldIgnoreActiveEditorChange) { + if (!this.trackingSuspended) { this.handleEditorEventInRecentEditorsStack(); } })); @@ -190,11 +195,10 @@ export class HistoryService extends Disposable implements IHistoryService { // is having a selection concept. if (isEditorPaneWithSelection(activeEditorPane)) { this.activeEditorListeners.add(activeEditorPane.onDidChangeSelection(e => { - if (!this.shouldIgnoreActiveEditorChange) { + if (!this.trackingSuspended) { this.handleActiveEditorSelectionChangeEvent(activeEditorGroup, activeEditorPane, e); } - } - )); + })); } // Context keys diff --git a/src/vs/workbench/services/history/common/history.ts b/src/vs/workbench/services/history/common/history.ts index 86d21f49dad..3d135ec2a70 100644 --- a/src/vs/workbench/services/history/common/history.ts +++ b/src/vs/workbench/services/history/common/history.ts @@ -8,6 +8,7 @@ import { IResourceEditorInput } from 'vs/platform/editor/common/editor'; import { GroupIdentifier } from 'vs/workbench/common/editor'; import { EditorInput } from 'vs/workbench/common/editor/editorInput'; import { URI } from 'vs/base/common/uri'; +import { IDisposable } from 'vs/base/common/lifecycle'; export const IHistoryService = createDecorator('historyService'); @@ -60,7 +61,6 @@ export const enum GoScope { export interface IHistoryService { readonly _serviceBrand: undefined; - shouldIgnoreActiveEditorChange: boolean; /** * Navigate forwards in editor navigation history. @@ -138,4 +138,9 @@ export interface IHistoryService { * Clear list of recently opened editors. */ clearRecentlyOpened(): void; + + /** + * Temporarily suspend tracking of editor events for the history. + */ + suspendTracking(): IDisposable; } diff --git a/src/vs/workbench/services/history/test/browser/historyService.test.ts b/src/vs/workbench/services/history/test/browser/historyService.test.ts index 124a79c0203..43838aad189 100644 --- a/src/vs/workbench/services/history/test/browser/historyService.test.ts +++ b/src/vs/workbench/services/history/test/browser/historyService.test.ts @@ -807,5 +807,143 @@ suite('HistoryService', function () { return workbenchTeardown(instantiationService); }); + test('suspend should suspend editor changes- skip two editors and continue (single group)', async () => { + const [part, historyService, editorService, , instantiationService] = await createServices(); + + const input1 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar1'), TEST_EDITOR_INPUT_ID)); + const input2 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar2'), TEST_EDITOR_INPUT_ID)); + const input3 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar3'), TEST_EDITOR_INPUT_ID)); + const input4 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar4'), TEST_EDITOR_INPUT_ID)); + const input5 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar5'), TEST_EDITOR_INPUT_ID)); + + let editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange); + await part.activeGroup.openEditor(input1, { pinned: true }); + assert.strictEqual(part.activeGroup.activeEditor, input1); + await editorChangePromise; + + const disposable = historyService.suspendTracking(); + + // wait on two editor changes before disposing + editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange) + .then(() => Event.toPromise(editorService.onDidActiveEditorChange)); + + await part.activeGroup.openEditor(input2, { pinned: true }); + assert.strictEqual(part.activeGroup.activeEditor, input2); + await part.activeGroup.openEditor(input3, { pinned: true }); + assert.strictEqual(part.activeGroup.activeEditor, input3); + + await editorChangePromise; + disposable.dispose(); + + await part.activeGroup.openEditor(input4, { pinned: true }); + assert.strictEqual(part.activeGroup.activeEditor, input4); + await part.activeGroup.openEditor(input5, { pinned: true }); + assert.strictEqual(part.activeGroup.activeEditor, input5); + + // stack should be [input1, input4, input5] + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input4); + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + + await historyService.goForward(); + assert.strictEqual(part.activeGroup.activeEditor, input4); + await historyService.goForward(); + assert.strictEqual(part.activeGroup.activeEditor, input5); + + return workbenchTeardown(instantiationService); + }); + + test('suspend should suspend editor changes- skip two editors and continue (multi group)', async () => { + const [part, historyService, editorService, , instantiationService] = await createServices(); + const rootGroup = part.activeGroup; + + const input1 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar1'), TEST_EDITOR_INPUT_ID)); + const input2 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar2'), TEST_EDITOR_INPUT_ID)); + const input3 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar3'), TEST_EDITOR_INPUT_ID)); + const input4 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar4'), TEST_EDITOR_INPUT_ID)); + const input5 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar5'), TEST_EDITOR_INPUT_ID)); + + const sideGroup = part.addGroup(rootGroup, GroupDirection.RIGHT); + + let editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange); + await rootGroup.openEditor(input1, { pinned: true }); + await editorChangePromise; + + const disposable = historyService.suspendTracking(); + editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange) + .then(() => Event.toPromise(editorService.onDidActiveEditorChange)); + await sideGroup.openEditor(input2, { pinned: true }); + await rootGroup.openEditor(input3, { pinned: true }); + await editorChangePromise; + disposable.dispose(); + + await sideGroup.openEditor(input4, { pinned: true }); + await rootGroup.openEditor(input5, { pinned: true }); + + // stack should be [input1, input4, input5] + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input4); + assert.strictEqual(part.activeGroup, sideGroup); + assert.strictEqual(rootGroup.activeEditor, input5); + + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + assert.strictEqual(part.activeGroup, rootGroup); + assert.strictEqual(sideGroup.activeEditor, input4); + + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + + await historyService.goForward(); + assert.strictEqual(part.activeGroup.activeEditor, input4); + await historyService.goForward(); + assert.strictEqual(part.activeGroup.activeEditor, input5); + + return workbenchTeardown(instantiationService); + }); + + test('suspend should suspend editor changes - interleaved skips', async () => { + const [part, historyService, editorService, , instantiationService] = await createServices(); + + const input1 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar1'), TEST_EDITOR_INPUT_ID)); + const input2 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar2'), TEST_EDITOR_INPUT_ID)); + const input3 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar3'), TEST_EDITOR_INPUT_ID)); + const input4 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar4'), TEST_EDITOR_INPUT_ID)); + const input5 = disposables.add(new TestFileEditorInput(URI.parse('foo://bar5'), TEST_EDITOR_INPUT_ID)); + + let editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange); + await part.activeGroup.openEditor(input1, { pinned: true }); + await editorChangePromise; + + let disposable = historyService.suspendTracking(); + editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange); + await part.activeGroup.openEditor(input2, { pinned: true }); + await editorChangePromise; + disposable.dispose(); + + await part.activeGroup.openEditor(input3, { pinned: true }); + + disposable = historyService.suspendTracking(); + editorChangePromise = Event.toPromise(editorService.onDidActiveEditorChange); + await part.activeGroup.openEditor(input4, { pinned: true }); + await editorChangePromise; + disposable.dispose(); + + await part.activeGroup.openEditor(input5, { pinned: true }); + + // stack should be [input1, input3, input5] + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input3); + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + await historyService.goBack(); + assert.strictEqual(part.activeGroup.activeEditor, input1); + + return workbenchTeardown(instantiationService); + }); + ensureNoDisposablesAreLeakedInTestSuite(); }); diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index 23355faf76d..6db63c1cc2e 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -98,7 +98,7 @@ import { IInputBox, IInputOptions, IPickOptions, IQuickInputButton, IQuickInputS import { QuickInputService } from 'vs/workbench/services/quickinput/browser/quickInputService'; import { IListService } from 'vs/platform/list/browser/listService'; import { win32, posix } from 'vs/base/common/path'; -import { TestContextService, TestStorageService, TestTextResourcePropertiesService, TestExtensionService, TestProductService, createFileStat, TestLoggerService, TestWorkspaceTrustManagementService, TestWorkspaceTrustRequestService, TestMarkerService } from 'vs/workbench/test/common/workbenchTestServices'; +import { TestContextService, TestStorageService, TestTextResourcePropertiesService, TestExtensionService, TestProductService, createFileStat, TestLoggerService, TestWorkspaceTrustManagementService, TestWorkspaceTrustRequestService, TestMarkerService, TestHistoryService } from 'vs/workbench/test/common/workbenchTestServices'; import { IView, ViewContainer, ViewContainerLocation } from 'vs/workbench/common/views'; import { IViewsService } from 'vs/workbench/services/views/common/viewsService'; import { IPaneComposite } from 'vs/workbench/common/panecomposite'; @@ -542,28 +542,6 @@ export class TestMenuService implements IMenuService { } } -export class TestHistoryService implements IHistoryService { - - declare readonly _serviceBrand: undefined; - declare shouldIgnoreActiveEditorChange: boolean; - - constructor(private root?: URI) { } - - async reopenLastClosedEditor(): Promise { } - async goForward(): Promise { } - async goBack(): Promise { } - async goPrevious(): Promise { } - async goLast(): Promise { } - removeFromHistory(_input: EditorInput | IResourceEditorInput): void { } - clear(): void { } - clearRecentlyOpened(): void { } - getHistory(): readonly (EditorInput | IResourceEditorInput)[] { return []; } - async openNextRecentlyUsedEditor(group?: GroupIdentifier): Promise { } - async openPreviouslyUsedEditor(group?: GroupIdentifier): Promise { } - getLastActiveWorkspaceRoot(_schemeFilter: string): URI | undefined { return this.root; } - getLastActiveFile(_schemeFilter: string): URI | undefined { return undefined; } -} - export class TestFileDialogService implements IFileDialogService { declare readonly _serviceBrand: undefined; diff --git a/src/vs/workbench/test/common/workbenchTestServices.ts b/src/vs/workbench/test/common/workbenchTestServices.ts index d1483ff15be..1d09528dcf6 100644 --- a/src/vs/workbench/test/common/workbenchTestServices.ts +++ b/src/vs/workbench/test/common/workbenchTestServices.ts @@ -149,7 +149,6 @@ export class TestStorageService extends InMemoryStorageService { export class TestHistoryService implements IHistoryService { declare readonly _serviceBrand: undefined; - declare shouldIgnoreActiveEditorChange: boolean; constructor(private root?: URI) { } @@ -166,6 +165,7 @@ export class TestHistoryService implements IHistoryService { async openPreviouslyUsedEditor(group?: GroupIdentifier): Promise { } getLastActiveWorkspaceRoot(_schemeFilter: string): URI | undefined { return this.root; } getLastActiveFile(_schemeFilter: string): URI | undefined { return undefined; } + suspendTracking() { return Disposable.None; } } export class TestWorkingCopy extends Disposable implements IWorkingCopy {