From 40e586c8a61a4087a7a1bf1ded6c113bd43f0fcb Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 28 Aug 2024 09:28:34 -0700 Subject: [PATCH] testing: call stack tpi refinements Fixes #226855 - ctrl+click on words in files takes you to the source Fixes #226863 - clicking file titles should toggle collapse Fixes #226857 - add 'collapse all' button in peek Fixes #226852 - name 'go to source' actions better --- .../contrib/debug/browser/callStackWidget.ts | 124 ++++++++++++++---- .../debug/browser/media/callStackWidget.css | 6 + .../testResultsView/testMessageStack.ts | 4 + .../testResultsView/testResultsSubject.ts | 3 + .../testResultsView/testResultsTree.ts | 13 +- .../testResultsView/testResultsViewContent.ts | 7 + .../testing/browser/testing.contribution.ts | 3 +- .../testing/browser/testingOutputPeek.ts | 64 ++++++++- .../testing/common/testingContextKeys.ts | 1 + 9 files changed, 188 insertions(+), 37 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/callStackWidget.ts b/src/vs/workbench/contrib/debug/browser/callStackWidget.ts index 50cfbf0f3a53a..0b6b0e6bc286e 100644 --- a/src/vs/workbench/contrib/debug/browser/callStackWidget.ts +++ b/src/vs/workbench/contrib/debug/browser/callStackWidget.ts @@ -12,20 +12,24 @@ import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cance import { Codicon } from 'vs/base/common/codicons'; import { Emitter, Event } from 'vs/base/common/event'; import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; -import { autorun, autorunWithStore, derived, IObservable, ISettableObservable, observableValue } from 'vs/base/common/observable'; +import { autorun, autorunWithStore, derived, IObservable, ISettableObservable, observableValue, transaction } from 'vs/base/common/observable'; import { ThemeIcon } from 'vs/base/common/themables'; import { Constants } from 'vs/base/common/uint'; import { URI } from 'vs/base/common/uri'; import { generateUuid } from 'vs/base/common/uuid'; import 'vs/css!./media/callStackWidget'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; -import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; +import { EditorContributionCtor, EditorContributionInstantiation, IEditorContributionDescription } from 'vs/editor/browser/editorExtensions'; import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditor/codeEditorWidget'; import { EmbeddedCodeEditorWidget } from 'vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget'; import { IEditorOptions } from 'vs/editor/common/config/editorOptions'; +import { Position } from 'vs/editor/common/core/position'; import { Range } from 'vs/editor/common/core/range'; +import { IWordAtPosition } from 'vs/editor/common/core/wordHelper'; +import { IEditorContribution, IEditorDecorationsCollection } from 'vs/editor/common/editorCommon'; import { Location } from 'vs/editor/common/languages'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; +import { ClickLinkGesture, ClickLinkMouseEvent } from 'vs/editor/contrib/gotoSymbol/browser/link/clickLinkGesture'; import { localize, localize2 } from 'vs/nls'; import { createActionViewItem } from 'vs/platform/actions/browser/menuEntryActionViewItem'; import { MenuWorkbenchToolBar } from 'vs/platform/actions/browser/toolbar'; @@ -38,7 +42,7 @@ import { INotificationService } from 'vs/platform/notification/common/notificati import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { ResourceLabel } from 'vs/workbench/browser/labels'; import { makeStackFrameColumnDecoration, TOP_STACK_FRAME_DECORATION } from 'vs/workbench/contrib/debug/browser/callStackEditorContribution'; -import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; export class CallStackFrame { @@ -97,6 +101,9 @@ class WrappedCustomStackFrame implements IFrameLikeItem { constructor(public readonly original: CustomStackFrame) { } } +const isFrameLike = (item: unknown): item is IFrameLikeItem => + item instanceof WrappedCallStackFrame || item instanceof WrappedCustomStackFrame; + type ListItem = WrappedCallStackFrame | SkippedCallFrames | WrappedCustomStackFrame; const WIDGET_CLASS_NAME = 'multiCallStackWidget'; @@ -157,6 +164,17 @@ export class CallStackWidget extends Disposable { this.layoutEmitter.fire(); } + public collapseAll() { + transaction(tx => { + for (let i = 0; i < this.list.length; i++) { + const frame = this.list.element(i); + if (isFrameLike(frame)) { + frame.collapsed.set(true, tx); + } + } + }); + } + private async loadFrame(replacing: SkippedCallFrames): Promise { if (!this.cts) { return; @@ -356,9 +374,9 @@ abstract class AbstractFrameRenderer { - item.collapsed.set(!item.collapsed.get(), undefined); - })); + const toggleCollapse = () => item.collapsed.set(!item.collapsed.get(), undefined); + elementStore.add(collapse.onDidClick(toggleCollapse)); + elementStore.add(dom.addDisposableListener(elements.title, 'click', toggleCollapse)); } disposeElement(element: ListItem, index: number, templateData: T, height: number | undefined): void { @@ -382,26 +400,33 @@ class FrameCodeRenderer extends AbstractFrameRenderer { private readonly containingEditor: ICodeEditor | undefined, private readonly onLayout: Event, @ITextModelService private readonly modelService: ITextModelService, - @ICodeEditorService private readonly editorService: ICodeEditorService, @IInstantiationService instantiationService: IInstantiationService, ) { super(instantiationService); } protected override finishRenderTemplate(data: IAbstractFrameRendererTemplateData): IStackTemplateData { + // override default e.g. language contributions, only allow users to click + // on code in the call stack to go to its source location + const contributions: IEditorContributionDescription[] = [{ + id: ClickToLocationContribution.ID, + instantiation: EditorContributionInstantiation.BeforeFirstInteraction, + ctor: ClickToLocationContribution as EditorContributionCtor, + }]; + const editor = this.containingEditor ? this.instantiationService.createInstance( EmbeddedCodeEditorWidget, data.elements.editor, editorOptions, - { isSimpleWidget: true }, + { isSimpleWidget: true, contributions }, this.containingEditor, ) : this.instantiationService.createInstance( CodeEditorWidget, data.elements.editor, editorOptions, - { isSimpleWidget: true }, + { isSimpleWidget: true, contributions }, ); data.templateStore.add(editor); @@ -423,20 +448,6 @@ class FrameCodeRenderer extends AbstractFrameRenderer { const uri = item.source!; template.label.element.setFile(uri); - template.elements.title.role = 'link'; - elementStore.add(dom.addDisposableListener(template.elements.title, 'click', e => { - this.editorService.openCodeEditor({ - resource: uri, - options: { - selection: Range.fromPositions({ - column: item.column ?? 1, - lineNumber: item.line ?? 1, - }), - selectionRevealType: TextEditorSelectionRevealType.CenterIfOutsideViewport, - }, - }, this.containingEditor || null, e.ctrlKey || e.metaKey); - })); - const cts = new CancellationTokenSource(); elementStore.add(toDisposable(() => cts.dispose(true))); this.modelService.createModelReference(uri).then(reference => { @@ -632,6 +643,73 @@ class SkippedRenderer implements IListRenderer { } } +/** A simple contribution that makes all data in the editor clickable to go to the location */ +class ClickToLocationContribution extends Disposable implements IEditorContribution { + public static readonly ID = 'clickToLocation'; + private readonly linkDecorations: IEditorDecorationsCollection; + private current: { line: number; word: IWordAtPosition } | undefined; + + constructor( + private readonly editor: ICodeEditor, + @IEditorService editorService: IEditorService, + ) { + super(); + this.linkDecorations = editor.createDecorationsCollection(); + this._register(toDisposable(() => this.linkDecorations.clear())); + + const clickLinkGesture = this._register(new ClickLinkGesture(editor)); + + this._register(clickLinkGesture.onMouseMoveOrRelevantKeyDown(([mouseEvent, keyboardEvent]) => { + this.onMove(mouseEvent); + })); + this._register(clickLinkGesture.onExecute((e) => { + const model = this.editor.getModel(); + if (!this.current || !model) { + return; + } + + editorService.openEditor({ + resource: model.uri, + options: { + selection: Range.fromPositions(new Position(this.current.line, this.current.word.startColumn)), + selectionRevealType: TextEditorSelectionRevealType.CenterIfOutsideViewport, + }, + }, e.hasSideBySideModifier ? SIDE_GROUP : undefined); + })); + } + + private onMove(mouseEvent: ClickLinkMouseEvent) { + if (!mouseEvent.hasTriggerModifier) { + return this.clear(); + } + + const position = mouseEvent.target.position; + const word = position && this.editor.getModel()?.getWordAtPosition(position); + if (!word) { + return this.clear(); + } + + const prev = this.current?.word; + if (prev && prev.startColumn === word.startColumn && prev.endColumn === word.endColumn && prev.word === word.word) { + return; + } + + this.current = { word, line: position.lineNumber }; + this.linkDecorations.set([{ + range: new Range(position.lineNumber, word.startColumn, position.lineNumber, word.endColumn), + options: { + description: 'call-stack-go-to-file-link', + inlineClassName: 'call-stack-go-to-file-link', + }, + }]); + } + + private clear() { + this.linkDecorations.clear(); + this.current = undefined; + } +} + registerAction2(class extends Action2 { constructor() { super({ diff --git a/src/vs/workbench/contrib/debug/browser/media/callStackWidget.css b/src/vs/workbench/contrib/debug/browser/media/callStackWidget.css index 60f5493087f8f..e4c0a1a61b134 100644 --- a/src/vs/workbench/contrib/debug/browser/media/callStackWidget.css +++ b/src/vs/workbench/contrib/debug/browser/media/callStackWidget.css @@ -59,3 +59,9 @@ line-height: inherit !important; } } + +.monaco-editor .call-stack-go-to-file-link { + text-decoration: underline; + cursor: pointer; + color: var(--vscode-editorLink-activeForeground) !important; +} diff --git a/src/vs/workbench/contrib/testing/browser/testResultsView/testMessageStack.ts b/src/vs/workbench/contrib/testing/browser/testResultsView/testMessageStack.ts index cb133b4b20d00..c6c396e584023 100644 --- a/src/vs/workbench/contrib/testing/browser/testResultsView/testMessageStack.ts +++ b/src/vs/workbench/contrib/testing/browser/testResultsView/testMessageStack.ts @@ -30,6 +30,10 @@ export class TestResultStackWidget extends Disposable { )); } + public collapseAll() { + this.widget.collapseAll(); + } + public update(messageFrame: AnyStackFrame, stack: ITestMessageStackFrame[]) { this.widget.setFrames([messageFrame, ...stack.map(frame => new CallStackFrame( frame.label, diff --git a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject.ts b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject.ts index 9fa863304ff7f..e61463bcdc00e 100644 --- a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject.ts +++ b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject.ts @@ -22,6 +22,9 @@ interface ISubjectCommon { controllerId: string; } +export const inspectSubjectHasStack = (subject: InspectSubject | undefined) => + subject instanceof MessageSubject && !!subject.stack?.length; + export class MessageSubject implements ISubjectCommon { public readonly test: ITestItem; public readonly message: ITestMessage; diff --git a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsTree.ts b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsTree.ts index a65a1057df569..6be1cd3c43f18 100644 --- a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsTree.ts +++ b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsTree.ts @@ -838,22 +838,21 @@ class TreeActionsProvider { } if (element instanceof TestMessageElement) { + id = MenuId.TestMessageContext; + contextKeys.push([TestingContextKeys.testMessageContext.key, element.contextValue]); + primary.push(new Action( - 'testing.outputPeek.goToFile', - localize('testing.goToFile', "Go to Source"), + 'testing.outputPeek.goToTest', + localize('testing.goToTest', "Go to Test"), ThemeIcon.asClassName(Codicon.goToFile), undefined, () => this.commandService.executeCommand('vscode.revealTest', element.test.item.extId), )); - } - if (element instanceof TestMessageElement) { - id = MenuId.TestMessageContext; - contextKeys.push([TestingContextKeys.testMessageContext.key, element.contextValue]); if (this.showRevealLocationOnMessages && element.location) { primary.push(new Action( 'testing.outputPeek.goToError', - localize('testing.goToError', "Go to Source"), + localize('testing.goToError', "Go to Error"), ThemeIcon.asClassName(Codicon.goToFile), undefined, () => this.editorService.openEditor({ diff --git a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts index db80e2c6ed8a3..67f9f05bfc72a 100644 --- a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts +++ b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts @@ -322,6 +322,13 @@ export class TestResultsViewContent extends Disposable { }); } + /** + * Collapses all displayed stack frames. + */ + public collapseStack() { + this.callStackWidget.collapseAll(); + } + private getCallFrames(subject: InspectSubject) { if (!(subject instanceof MessageSubject)) { return undefined; diff --git a/src/vs/workbench/contrib/testing/browser/testing.contribution.ts b/src/vs/workbench/contrib/testing/browser/testing.contribution.ts index e422256e5197a..8caac4a252753 100644 --- a/src/vs/workbench/contrib/testing/browser/testing.contribution.ts +++ b/src/vs/workbench/contrib/testing/browser/testing.contribution.ts @@ -25,7 +25,7 @@ import { testingResultsIcon, testingViewIcon } from 'vs/workbench/contrib/testin import { TestCoverageView } from 'vs/workbench/contrib/testing/browser/testCoverageView'; import { TestingDecorationService, TestingDecorations } from 'vs/workbench/contrib/testing/browser/testingDecorations'; import { TestingExplorerView } from 'vs/workbench/contrib/testing/browser/testingExplorerView'; -import { CloseTestPeek, GoToNextMessageAction, GoToPreviousMessageAction, OpenMessageInEditorAction, TestResultsView, TestingOutputPeekController, TestingPeekOpener, ToggleTestingPeekHistory } from 'vs/workbench/contrib/testing/browser/testingOutputPeek'; +import { CloseTestPeek, CollapsePeekStack, GoToNextMessageAction, GoToPreviousMessageAction, OpenMessageInEditorAction, TestResultsView, TestingOutputPeekController, TestingPeekOpener, ToggleTestingPeekHistory } from 'vs/workbench/contrib/testing/browser/testingOutputPeek'; import { TestingProgressTrigger } from 'vs/workbench/contrib/testing/browser/testingProgressUiService'; import { TestingViewPaneContainer } from 'vs/workbench/contrib/testing/browser/testingViewPaneContainer'; import { testingConfiguration } from 'vs/workbench/contrib/testing/common/configuration'; @@ -136,6 +136,7 @@ registerAction2(GoToPreviousMessageAction); registerAction2(GoToNextMessageAction); registerAction2(CloseTestPeek); registerAction2(ToggleTestingPeekHistory); +registerAction2(CollapsePeekStack); Registry.as(WorkbenchExtensions.Workbench).registerWorkbenchContribution(TestingContentProvider, LifecyclePhase.Restored); Registry.as(WorkbenchExtensions.Workbench).registerWorkbenchContribution(TestingPeekOpener, LifecyclePhase.Eventually); diff --git a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts index 2bac0cf08eed4..3acce2ece8128 100644 --- a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts +++ b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts @@ -14,6 +14,7 @@ import { Iterable } from 'vs/base/common/iterator'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { Lazy } from 'vs/base/common/lazy'; import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle'; +import { observableValue } from 'vs/base/common/observable'; import { count } from 'vs/base/common/strings'; import { URI } from 'vs/base/common/uri'; import { ICodeEditor, isCodeEditor } from 'vs/editor/browser/editorBrowser'; @@ -43,6 +44,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { INotificationService } from 'vs/platform/notification/common/notification'; +import { bindContextKey } from 'vs/platform/observable/common/platformObservableUtils'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; @@ -52,7 +54,7 @@ import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity' import { IViewPaneOptions, ViewPane } from 'vs/workbench/browser/parts/views/viewPane'; import { IViewDescriptorService } from 'vs/workbench/common/views'; import { renderTestMessageAsText } from 'vs/workbench/contrib/testing/browser/testMessageColorizer'; -import { InspectSubject, MessageSubject, TaskSubject, TestOutputSubject, mapFindTestMessage } from 'vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject'; +import { InspectSubject, MessageSubject, TaskSubject, TestOutputSubject, inspectSubjectHasStack, mapFindTestMessage } from 'vs/workbench/contrib/testing/browser/testResultsView/testResultsSubject'; import { TestResultsViewContent } from 'vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent'; import { testingMessagePeekBorder, testingPeekBorder, testingPeekHeaderBackground, testingPeekMessageHeaderBackground } from 'vs/workbench/contrib/testing/browser/theme'; import { AutoOpenPeekViewWhen, TestingConfigKeys, getTestingConfiguration } from 'vs/workbench/contrib/testing/common/configuration'; @@ -498,6 +500,13 @@ export class TestingOutputPeekController extends Disposable implements IEditorCo this.peek.clear(); } + /** + * Collapses all displayed stack frames. + */ + public collapseStack() { + this.peek.value?.collapseStack(); + } + /** * Shows the next message in the peek, if possible. */ @@ -645,10 +654,14 @@ class TestResultsPeek extends PeekViewWidget { private static lastHeightInLines?: number; private readonly visibilityChange = this._disposables.add(new Emitter()); + private readonly _current = observableValue('testPeekCurrent', undefined); private content!: TestResultsViewContent; private scopedContextKeyService!: IContextKeyService; private dimension?: dom.Dimension; - public current?: InspectSubject; + + public get current() { + return this._current.get(); + } constructor( editor: ICodeEditor, @@ -702,7 +715,14 @@ class TestResultsPeek extends PeekViewWidget { protected override _fillHead(container: HTMLElement): void { super._fillHead(container); - const menu = this.menuService.createMenu(MenuId.TestPeekTitle, this.contextKeyService); + const menuContextKeyService = this._disposables.add(this.contextKeyService.createScoped(container)); + this._disposables.add(bindContextKey( + TestingContextKeys.peekHasStack, + menuContextKeyService, + reader => inspectSubjectHasStack(this._current.read(reader)), + )); + + const menu = this.menuService.createMenu(MenuId.TestPeekTitle, menuContextKeyService); const actionBar = this._actionbarWidget!; this._disposables.add(menu.onDidChange(() => { actions.length = 0; @@ -732,7 +752,7 @@ class TestResultsPeek extends PeekViewWidget { */ public setModel(subject: InspectSubject): Promise { if (subject instanceof TaskSubject || subject instanceof TestOutputSubject) { - this.current = subject; + this._current.set(subject, undefined); return this.showInPlace(subject); } @@ -743,14 +763,14 @@ class TestResultsPeek extends PeekViewWidget { return Promise.resolve(); } - this.current = subject; + this._current.set(subject, undefined); if (!revealLocation) { return this.showInPlace(subject); } // If there is a stack we want to display, ensure the default size is large-ish const peekLines = TestResultsPeek.lastHeightInLines || Math.max( - subject instanceof MessageSubject && subject.stack?.length ? Math.ceil(this.getVisibleEditorLines() / 2) : 0, + inspectSubjectHasStack(subject) ? Math.ceil(this.getVisibleEditorLines() / 2) : 0, hintMessagePeekHeight(message) ); @@ -760,6 +780,13 @@ class TestResultsPeek extends PeekViewWidget { return this.showInPlace(subject); } + /** + * Collapses all displayed stack frames. + */ + public collapseStack() { + this.content.collapseStack(); + } + private getVisibleEditorLines() { // note that we don't use the view ranges because we don't want to get // thrown off by large wrapping lines. Being approximate here is okay. @@ -1025,6 +1052,31 @@ export class GoToPreviousMessageAction extends Action2 { } } +export class CollapsePeekStack extends Action2 { + public static readonly ID = 'testing.collapsePeekStack'; + constructor() { + super({ + id: CollapsePeekStack.ID, + title: localize2('testing.collapsePeekStack', 'Collapse Stack Frames'), + icon: Codicon.collapseAll, + category: Categories.Test, + menu: [{ + id: MenuId.TestPeekTitle, + when: TestingContextKeys.peekHasStack, + group: 'navigation', + order: 4, + }], + }); + } + + public override run(accessor: ServicesAccessor) { + const editor = getPeekedEditorFromFocus(accessor.get(ICodeEditorService)); + if (editor) { + TestingOutputPeekController.get(editor)?.collapseStack(); + } + } +} + export class OpenMessageInEditorAction extends Action2 { public static readonly ID = 'testing.openMessageInEditor'; constructor() { diff --git a/src/vs/workbench/contrib/testing/common/testingContextKeys.ts b/src/vs/workbench/contrib/testing/common/testingContextKeys.ts index ac303b4886f9e..1cfd764dc234f 100644 --- a/src/vs/workbench/contrib/testing/common/testingContextKeys.ts +++ b/src/vs/workbench/contrib/testing/common/testingContextKeys.ts @@ -29,6 +29,7 @@ export namespace TestingContextKeys { export const inlineCoverageEnabled = new RawContextKey('testing.inlineCoverageEnabled', false, { type: 'boolean', description: localize('testing.inlineCoverageEnabled', 'Indicates whether inline coverage is shown') }); export const canGoToRelatedCode = new RawContextKey('testing.canGoToRelatedCode', false, { type: 'boolean', description: localize('testing.canGoToRelatedCode', 'Whether a controller implements a capability to find code related to a test') }); export const canGoToRelatedTest = new RawContextKey('testing.canGoToRelatedTest', false, { type: 'boolean', description: localize('testing.canGoToRelatedTest', 'Whether a controller implements a capability to find tests related to code') }); + export const peekHasStack = new RawContextKey('testing.peekHasStack', false, { type: 'boolean', description: localize('testing.peekHasStack', 'Whether the message shown in a peek view has a stack trace') }); export const capabilityToContextKey: { [K in TestRunProfileBitset]: RawContextKey } = { [TestRunProfileBitset.Run]: hasRunnableTests,