From e98deca874989ccbfc1a95df4ec12b8ffb5fc7ea Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 10 Feb 2022 10:21:34 +0100 Subject: [PATCH] history - introduce and use new `JUMP` source --- .../gotoSymbol/browser/goToCommands.ts | 5 ++-- .../browser/peek/referencesController.ts | 5 ++-- src/vs/platform/editor/common/editor.ts | 24 +++++++++++++---- .../browser/parts/editor/textEditor.ts | 1 + src/vs/workbench/common/editor.ts | 15 ++++++++--- .../workbench/common/editor/editorOptions.ts | 9 ++++--- .../history/browser/historyService.ts | 24 ++++++++--------- .../test/browser/historyService.test.ts | 27 +++++++++++++++++++ .../test/browser/workbenchTestServices.ts | 10 +------ 9 files changed, 83 insertions(+), 37 deletions(-) diff --git a/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts b/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts index db0d755e5df..1e63a6a1f97 100644 --- a/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts +++ b/src/vs/editor/contrib/gotoSymbol/browser/goToCommands.ts @@ -32,7 +32,7 @@ import * as nls from 'vs/nls'; import { ISubmenuItem, MenuId, MenuRegistry } from 'vs/platform/actions/common/actions'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; -import { TextEditorSelectionRevealType } from 'vs/platform/editor/common/editor'; +import { TextEditorSelectionRevealType, TextEditorSelectionSource } from 'vs/platform/editor/common/editor'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { INotificationService } from 'vs/platform/notification/common/notification'; @@ -202,7 +202,8 @@ export abstract class SymbolNavigationAction extends EditorAction { resource: reference.uri, options: { selection: Range.collapseToStart(range), - selectionRevealType: TextEditorSelectionRevealType.NearTopIfOutsideViewport + selectionRevealType: TextEditorSelectionRevealType.NearTopIfOutsideViewport, + selectionSource: TextEditorSelectionSource.JUMP } }, editor, sideBySide); diff --git a/src/vs/editor/contrib/gotoSymbol/browser/peek/referencesController.ts b/src/vs/editor/contrib/gotoSymbol/browser/peek/referencesController.ts index 4ad25f48171..273867826c8 100644 --- a/src/vs/editor/contrib/gotoSymbol/browser/peek/referencesController.ts +++ b/src/vs/editor/contrib/gotoSymbol/browser/peek/referencesController.ts @@ -19,6 +19,7 @@ import * as nls from 'vs/nls'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { ContextKeyExpr, IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey'; +import { TextEditorSelectionSource } from 'vs/platform/editor/common/editor'; import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { IListService, WorkbenchListFocusContextKey, WorkbenchTreeElementCanCollapse, WorkbenchTreeElementCanExpand } from 'vs/platform/list/browser/listService'; @@ -246,7 +247,7 @@ export abstract class ReferencesController implements IEditorContribution { return this._editorService.openCodeEditor({ resource: ref.uri, - options: { selection: range } + options: { selection: range, selectionSource: TextEditorSelectionSource.JUMP } }, this._editor).then(openedEditor => { this._ignoreModelChangeEvent = false; @@ -292,7 +293,7 @@ export abstract class ReferencesController implements IEditorContribution { const { uri, range } = ref; this._editorService.openCodeEditor({ resource: uri, - options: { selection: range, pinned } + options: { selection: range, selectionSource: TextEditorSelectionSource.JUMP, pinned } }, this._editor, sideBySide); } } diff --git a/src/vs/platform/editor/common/editor.ts b/src/vs/platform/editor/common/editor.ts index 686fd93b8a7..e13f691c9a6 100644 --- a/src/vs/platform/editor/common/editor.ts +++ b/src/vs/platform/editor/common/editor.ts @@ -328,16 +328,25 @@ export const enum TextEditorSelectionSource { /** * Programmatic source indicates a selection change that - * was not triggered by the user via keyboard or mouse. + * was not triggered by the user via keyboard or mouse + * but through text editor APIs. */ PROGRAMMATIC = 'api', /** - * Navigation source indicates a change that was caused - * by navigating in the text editor from commands such - * as "Go to definition" + * Navigation source indicates a selection change that + * was caused via some command or UI component such as + * an outline tree. */ - NAVIGATION = 'code.navigation' + NAVIGATION = 'code.navigation', + + /** + * Jump source indicates a selection change that + * was caused from within the text editor to another + * location in the same or different text editor such + * as "Go to definition". + */ + JUMP = 'code.jump' } export interface ITextEditorOptions extends IEditorOptions { @@ -352,4 +361,9 @@ export interface ITextEditorOptions extends IEditorOptions { * Defaults to TextEditorSelectionRevealType.Center */ selectionRevealType?: TextEditorSelectionRevealType; + + /** + * Source of the call that caused the selection. + */ + selectionSource?: TextEditorSelectionSource | string; } diff --git a/src/vs/workbench/browser/parts/editor/textEditor.ts b/src/vs/workbench/browser/parts/editor/textEditor.ts index e5edf3fb7b2..406b5de5192 100644 --- a/src/vs/workbench/browser/parts/editor/textEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textEditor.ts @@ -153,6 +153,7 @@ export abstract class BaseTextEditor extends Abstrac switch (e.source) { case TextEditorSelectionSource.PROGRAMMATIC: return EditorPaneSelectionChangeReason.PROGRAMMATIC; case TextEditorSelectionSource.NAVIGATION: return EditorPaneSelectionChangeReason.NAVIGATION; + case TextEditorSelectionSource.JUMP: return EditorPaneSelectionChangeReason.JUMP; default: return EditorPaneSelectionChangeReason.USER; } } diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index adc0cfe3313..22afa09337d 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -223,10 +223,19 @@ export const enum EditorPaneSelectionChangeReason { * The selection was changed as a result of a navigation * action. * - * For a text editor pane, this for example can be invoking - * "Go to definition" on a symbol. + * For a text editor pane, this for example can be a result + * of selecting an entry from a text outline view. */ - NAVIGATION + NAVIGATION, + + /** + * The selection was changed as a result of a jump action + * from within the editor pane. + * + * For a text editor pane, this for example can be a result + * of invoking "Go to definition" from a symbol. + */ + JUMP } export interface IEditorPaneSelection { diff --git a/src/vs/workbench/common/editor/editorOptions.ts b/src/vs/workbench/common/editor/editorOptions.ts index 95d1d01425f..ee5de0c98f4 100644 --- a/src/vs/workbench/common/editor/editorOptions.ts +++ b/src/vs/workbench/common/editor/editorOptions.ts @@ -25,10 +25,11 @@ export function applyTextEditorOptions(options: ITextEditorOptions, editor: IEdi endColumn: options.selection.endColumn ?? options.selection.startColumn }; - // Apply selection and give it a `TextEditorSelectionSource` - // so that listeners can distinguish this selection change - // from others. - editor.setSelection(range, TextEditorSelectionSource.NAVIGATION); + // Apply selection with a source so that listeners can + // distinguish this selection change from others. + // If no source is provided, set a default source to + // signal this navigation. + editor.setSelection(range, options.selectionSource ?? TextEditorSelectionSource.NAVIGATION); // Reveal selection if (options.selectionRevealType === TextEditorSelectionRevealType.NearTop) { diff --git a/src/vs/workbench/services/history/browser/historyService.ts b/src/vs/workbench/services/history/browser/historyService.ts index 50e4d650839..987046c6a74 100644 --- a/src/vs/workbench/services/history/browser/historyService.ts +++ b/src/vs/workbench/services/history/browser/historyService.ts @@ -310,6 +310,7 @@ export class HistoryService extends Disposable implements IHistoryService { } private handleActiveEditorSelectionChangeEventInNavigationStacks(editorPane: IEditorPaneWithSelection, event: IEditorPaneSelectionChangeEvent): void { + const previous = this.globalDefaultEditorNavigationStack.current; // Always send to global navigation stack this.globalDefaultEditorNavigationStack.notifyNavigation(editorPane, event); @@ -324,15 +325,17 @@ export class HistoryService extends Disposable implements IHistoryService { // Note: ignore if global navigation stack is navigating because // in that case we do not want to receive repeated entries in // the navigation stack. - else if (event.reason === EditorPaneSelectionChangeReason.NAVIGATION && !this.globalDefaultEditorNavigationStack.isNavigating()) { + else if ( + (event.reason === EditorPaneSelectionChangeReason.NAVIGATION || event.reason === EditorPaneSelectionChangeReason.JUMP) && + !this.globalDefaultEditorNavigationStack.isNavigating() + ) { - // A navigation selection change always has a source and target - // As such, we add the previous entry of the global navigation - // stack so that our navigation stack receives both entries - // unless the user is currently navigating. + // A "JUMP" navigation selection change always has a source and + // target. As such, we add the previous entry of the global + // navigation stack so that our navigation stack receives both + // entries unless the user is currently navigating. - if (!this.globalNavigationsEditorNavigationStack.isNavigating()) { - const previous = this.globalDefaultEditorNavigationStack.previous; + if (event.reason === EditorPaneSelectionChangeReason.JUMP && !this.globalNavigationsEditorNavigationStack.isNavigating()) { if (previous) { this.globalNavigationsEditorNavigationStack.addOrReplace(previous.groupId, previous.editor, previous.selection); } @@ -1035,7 +1038,7 @@ export class EditorNavigationStack extends Disposable { private currentSelectionState: EditorSelectionState | undefined = undefined; - private get current(): IEditorNavigationStackEntry | undefined { + get current(): IEditorNavigationStackEntry | undefined { return this.stack[this.index]; } @@ -1045,10 +1048,6 @@ export class EditorNavigationStack extends Disposable { } } - get previous(): IEditorNavigationStackEntry | undefined { - return this.stack[this.index - 1]; - } - constructor( private readonly kind: GoFilter, @IInstantiationService private readonly instantiationService: IInstantiationService, @@ -1120,6 +1119,7 @@ ${entryLabels.join('\n')} switch (event.reason) { case EditorPaneSelectionChangeReason.EDIT: return 'edit'; case EditorPaneSelectionChangeReason.NAVIGATION: return 'navigation'; + case EditorPaneSelectionChangeReason.JUMP: return 'jump'; case EditorPaneSelectionChangeReason.PROGRAMMATIC: return 'programmatic'; case EditorPaneSelectionChangeReason.USER: return 'user'; } 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 27ba9e679b0..34977f23893 100644 --- a/src/vs/workbench/services/history/test/browser/historyService.test.ts +++ b/src/vs/workbench/services/history/test/browser/historyService.test.ts @@ -171,6 +171,33 @@ suite('HistoryService', function () { await historyService.goBack(GoFilter.NAVIGATION); assertTextSelection(new Selection(5, 3, 5, 20), pane); + await historyService.goBack(GoFilter.NAVIGATION); + assertTextSelection(new Selection(5, 3, 5, 20), pane); + + await historyService.goForward(GoFilter.NAVIGATION); + assertTextSelection(new Selection(120, 8, 120, 18), pane); + + await historyService.goPrevious(GoFilter.NAVIGATION); + assertTextSelection(new Selection(5, 3, 5, 20), pane); + + await historyService.goPrevious(GoFilter.NAVIGATION); + assertTextSelection(new Selection(120, 8, 120, 18), pane); + }); + + test('back / forward: in-editor text selection changes (jump)', async function () { + const [, historyService, editorService] = await createServices(); + + const resource = toResource.call(this, '/path/index.txt'); + + const pane = await editorService.openEditor({ resource, options: { pinned: true } }) as TestTextFileEditor; + + await setTextSelection(historyService, pane, new Selection(2, 2, 2, 10), EditorPaneSelectionChangeReason.USER); + await setTextSelection(historyService, pane, new Selection(5, 3, 5, 20), EditorPaneSelectionChangeReason.JUMP); + await setTextSelection(historyService, pane, new Selection(120, 8, 120, 18), EditorPaneSelectionChangeReason.JUMP); + + await historyService.goBack(GoFilter.NAVIGATION); + assertTextSelection(new Selection(5, 3, 5, 20), pane); + await historyService.goBack(GoFilter.NAVIGATION); assertTextSelection(new Selection(2, 2, 2, 10), pane); diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index e0ea4a9da9c..8b0556bb40d 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -186,16 +186,8 @@ export class TestTextFileEditor extends TextFileEditor { return this.instantiationService.createInstance(TestCodeEditor, parent, configuration, {}); } - fireSelectionChangeEvent(reason: EditorPaneSelectionChangeReason) { - this._onDidChangeSelection.fire({ reason }); - } - setSelection(selection: Selection | undefined, reason: EditorPaneSelectionChangeReason): void { - if (selection) { - this.setOptions({ selection }); - } else { - this.setOptions(undefined); - } + this._options = selection ? { selection } as IEditorOptions : undefined; this._onDidChangeSelection.fire({ reason }); }