Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug window context menu contribution points #212501

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/base/browser/contextmenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export interface IContextMenuDelegate {
getActions(): readonly IAction[];
getCheckedActionsRepresentation?(action: IAction): 'radio' | 'checkbox';
getActionViewItem?(action: IAction, options: IActionViewItemOptions): IActionViewItem | undefined;
getActionsContext?(event?: IContextMenuEvent): unknown;
getActionsContext?(event?: IContextMenuEvent, contributedAction?: boolean): unknown;
getKeyBinding?(action: IAction): ResolvedKeybinding | undefined;
getMenuClassName?(): string;
onHide?(didCancel: boolean): void;
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/actions/common/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class MenuId {
static readonly DebugWatchContext = new MenuId('DebugWatchContext');
static readonly DebugToolBar = new MenuId('DebugToolBar');
static readonly DebugToolBarStop = new MenuId('DebugToolBarStop');
static readonly DebugDisassemblyContext = new MenuId('DebugDisassemblyContext');
static readonly DebugCallStackToolbar = new MenuId('DebugCallStackToolbar');
static readonly DebugCreateConfiguration = new MenuId('DebugCreateConfiguration');
static readonly EditorContext = new MenuId('EditorContext');
Expand Down
3 changes: 3 additions & 0 deletions src/vs/platform/extensions/common/extensionsApiProposals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ const _allApiProposals = {
customEditorMove: {
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.customEditorMove.d.ts',
},
debugContext: {
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.debugContext.d.ts',
},
debugVisualization: {
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.debugVisualization.d.ts',
},
Expand Down
46 changes: 42 additions & 4 deletions src/vs/workbench/contrib/debug/browser/breakpointsView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ interface InputBoxData {
type: 'condition' | 'hitCount' | 'name';
}

interface IBreakpointContext {
sessionId: string | undefined;
breakpoint: DebugProtocol.SourceBreakpoint | DebugProtocol.FunctionBreakpoint | DebugProtocol.DataBreakpoint | DebugProtocol.InstructionBreakpoint | undefined;
path?: string;
}

export class BreakpointsView extends ViewPane {

private list!: WorkbenchList<BreakpointItem>;
Expand Down Expand Up @@ -258,7 +264,7 @@ export class BreakpointsView extends ViewPane {
}
}

private onListContextMenu(e: IListContextMenuEvent<IEnablement>): void {
private async onListContextMenu(e: IListContextMenuEvent<IBaseBreakpoint>): Promise<void> {
const element = e.element;
const type = element instanceof Breakpoint ? 'breakpoint' : element instanceof ExceptionBreakpoint ? 'exceptionBreakpoint' :
element instanceof FunctionBreakpoint ? 'functionBreakpoint' : element instanceof DataBreakpoint ? 'dataBreakpoint' :
Expand All @@ -269,15 +275,47 @@ export class BreakpointsView extends ViewPane {
this.breakpointSupportsCondition.set(conditionSupported);
this.breakpointIsDataBytes.set(element instanceof DataBreakpoint && element.src.type === DataBreakpointSetType.Address);

const { secondary } = getContextMenuActions(this.menu.getActions({ arg: e.element, shouldForwardArgs: false }), 'inline');

const { secondary } = getContextMenuActions(this.menu.getActions({ shouldForwardArgs: true }), 'inline');
const context = await this.getBreakpointContext(element);
this.contextMenuService.showContextMenu({
getAnchor: () => e.anchor,
getActions: () => secondary,
getActionsContext: () => element
getActionsContext: (_e, contributedAction) => contributedAction ? context : element
});
}

private async getBreakpointContext(breakpoint?: IBaseBreakpoint): Promise<IBreakpointContext | undefined> {
const debugSession = this.debugService.getViewModel()?.focusedSession;
if (!debugSession) {
return;
}

if (breakpoint instanceof Breakpoint) {
return {
sessionId: debugSession.getId(),
path: breakpoint.uri.toString(),
breakpoint: breakpoint.toDAP()
};
} else if (breakpoint instanceof FunctionBreakpoint) {
return {
sessionId: debugSession.getId(),
breakpoint: breakpoint.toDAP()
};
} else if (breakpoint instanceof DataBreakpoint) {
return {
sessionId: debugSession.getId(),
breakpoint: await breakpoint.toDAP(debugSession)
};
} else if (breakpoint instanceof InstructionBreakpoint) {
return {
sessionId: debugSession.getId(),
breakpoint: breakpoint.toDAP()
};
}

return;
}

private updateSize(): void {
const containerModel = this.viewDescriptorService.getViewContainerModel(this.viewDescriptorService.getViewContainerByViewId(this.id)!)!;

Expand Down
40 changes: 38 additions & 2 deletions src/vs/workbench/contrib/debug/browser/disassemblyView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { PixelRatio } from '../../../../base/browser/pixelRatio.js';
import { $, Dimension, addStandardDisposableListener, append } from '../../../../base/browser/dom.js';
import { IListAccessibilityProvider } from '../../../../base/browser/ui/list/listWidget.js';
import { ITableRenderer, ITableVirtualDelegate } from '../../../../base/browser/ui/table/table.js';
import { ITableRenderer, ITableVirtualDelegate, ITableContextMenuEvent } from '../../../../base/browser/ui/table/table.js';
import { binarySearch2 } from '../../../../base/common/arrays.js';
import { Color } from '../../../../base/common/color.js';
import { Emitter } from '../../../../base/common/event.js';
Expand Down Expand Up @@ -43,8 +43,11 @@ import { getUriFromSource } from '../common/debugSource.js';
import { isUri, sourcesEqual } from '../common/debugUtils.js';
import { IEditorService } from '../../../services/editor/common/editorService.js';
import { IEditorGroup } from '../../../services/editor/common/editorGroupsService.js';
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
import { IMenu, IMenuService, MenuId } from '../../../../platform/actions/common/actions.js';
import { getFlatContextMenuActions } from '../../../../platform/actions/browser/menuEntryActionViewItem.js';

interface IDisassembledInstructionEntry {
export interface IDisassembledInstructionEntry {
allowBreakpoint: boolean;
isBreakpointSet: boolean;
isBreakpointEnabled: boolean;
Expand All @@ -62,6 +65,10 @@ interface IDisassembledInstructionEntry {
address: bigint;
}

interface IDisassemblyContext {
sessionId: string | undefined;
instruction: DebugProtocol.DisassembledInstruction;
}

// Special entry as a placeholer when disassembly is not available
const disassemblyNotAvailable: IDisassembledInstructionEntry = {
Expand Down Expand Up @@ -91,6 +98,7 @@ export class DisassemblyView extends EditorPane {
private _enableSourceCodeRender: boolean = true;
private _loadingLock: boolean = false;
private readonly _referenceToMemoryAddress = new Map<string, bigint>();
private menu: IMenu;

constructor(
group: IEditorGroup,
Expand All @@ -100,9 +108,14 @@ export class DisassemblyView extends EditorPane {
@IConfigurationService private readonly _configurationService: IConfigurationService,
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IDebugService private readonly _debugService: IDebugService,
@IContextMenuService private readonly _contextMenuService: IContextMenuService,
@IMenuService menuService: IMenuService,
@IContextKeyService contextKeyService: IContextKeyService,
) {
super(DISASSEMBLY_VIEW_ID, group, telemetryService, themeService, storageService);

this.menu = menuService.createMenu(MenuId.DebugDisassemblyContext, contextKeyService);
this._register(this.menu);
this._disassembledInstructions = undefined;
this._onDidChangeStackFrame = this._register(new Emitter<void>({ leakWarningThreshold: 1000 }));
this._previousDebuggingState = _debugService.state;
Expand Down Expand Up @@ -273,6 +286,8 @@ export class DisassemblyView extends EditorPane {
}
}));

this._register(this._disassembledInstructions.onContextMenu(e => this.onContextMenu(e)));

this._register(this._debugService.getViewModel().onDidFocusStackFrame(({ stackFrame }) => {
if (this._disassembledInstructions && stackFrame?.instructionPointerReference) {
this.goToInstructionAndOffset(stackFrame.instructionPointerReference, 0);
Expand Down Expand Up @@ -633,6 +648,27 @@ export class DisassemblyView extends EditorPane {
this._referenceToMemoryAddress.clear();
this._disassembledInstructions?.splice(0, this._disassembledInstructions.length, [disassemblyNotAvailable]);
}

private onContextMenu(e: ITableContextMenuEvent<IDisassembledInstructionEntry>): void {
const context = this.getDisassemblyContext(e.element);
const actions = getFlatContextMenuActions(this.menu.getActions({ shouldForwardArgs: true }));
this._contextMenuService.showContextMenu({
getAnchor: () => e.anchor,
getActions: () => actions,
getActionsContext: (_e, contributedAction) => contributedAction ? context : e.element
});
}

private getDisassemblyContext(instruction?: IDisassembledInstructionEntry): IDisassemblyContext | undefined {
if (!instruction) {
return undefined;
}

return {
sessionId: this.debugSession?.getId(),
instruction: instruction.instruction
};
}
}

interface IBreakpointColumnTemplateData {
Expand Down
27 changes: 25 additions & 2 deletions src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ const MAX_VALUE_RENDER_LENGTH_IN_VIEWLET = 1024;
let ignoreViewUpdates = false;
let useCachedEvaluation = false;

interface IWatchContext {
sessionId: string | undefined;
type: string | undefined;
expressionId: string;
name: string;
value: string;
}

export class WatchExpressionsView extends ViewPane implements IDebugViewWithVariables {

private watchExpressionsUpdatedScheduler: RunOnceScheduler;
Expand Down Expand Up @@ -219,18 +227,33 @@ export class WatchExpressionsView extends ViewPane implements IDebugViewWithVari

private onContextMenu(e: ITreeContextMenuEvent<IExpression>): void {
const element = e.element;
const context = this.getWatchContext(element);
const selection = this.tree.getSelection();

this.watchItemType.set(element instanceof Expression ? 'expression' : element instanceof Variable ? 'variable' : undefined);
const attributes = element instanceof Variable ? element.presentationHint?.attributes : undefined;
this.variableReadonly.set(!!attributes && attributes.indexOf('readOnly') >= 0 || !!element?.presentationHint?.lazy);
const actions = getFlatContextMenuActions(this.menu.getActions({ arg: element, shouldForwardArgs: true }));
const actions = getFlatContextMenuActions(this.menu.getActions({ shouldForwardArgs: true }));
this.contextMenuService.showContextMenu({
getAnchor: () => e.anchor,
getActions: () => actions,
getActionsContext: () => element && selection.includes(element) ? selection : element ? [element] : [],
getActionsContext: (_e, contributedAction) => contributedAction ? context : element && selection.includes(element) ? selection : element ? [element] : [],
});
}

private getWatchContext(expression: IExpression | null): IWatchContext | undefined {
if (!expression) {
return undefined;
}

return {
sessionId: this.debugService.getViewModel().focusedSession?.getId(),
expressionId: expression.getId(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this ID is meaningful or exposed to extensions elsewhere, or is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could be right, but it may be useful as a unique ID?

name: expression.name,
type: expression.type,
value: expression.value
};
}
}

class WatchExpressionsDelegate implements IListVirtualDelegate<IExpression> {
Expand Down
18 changes: 18 additions & 0 deletions src/vs/workbench/services/actions/common/menusExtensionPoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,24 @@ const apiMenus: IAPIMenu[] = [
proposed: 'contribDebugCreateConfiguration',
description: localize('menus.debugCreateConfiguation', "The debug create configuration menu")
},
{
key: 'debug/disassembly/context',
id: MenuId.DebugDisassemblyContext,
description: localize('menus.debugDisassemblyContext', "The debug disassembly context menu"),
proposed: 'debugContext'
},
{
key: 'debug/watch/context',
id: MenuId.DebugWatchContext,
description: localize('menus.debugWatchContext', "The debug watch context menu"),
proposed: 'debugContext'
},
{
key: 'debug/breakpoints/context',
id: MenuId.DebugBreakpointsContext,
description: localize('menus.debugBreakpointsContext', "The debug breakpoints context menu"),
proposed: 'debugContext'
},
{
key: 'notebook/variables/context',
id: MenuId.NotebookVariablesContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { stripIcons } from '../../../../base/common/iconLabels.js';
import { coalesce } from '../../../../base/common/arrays.js';
import { Event, Emitter } from '../../../../base/common/event.js';
import { AnchorAlignment, AnchorAxisAlignment, isAnchor } from '../../../../base/browser/ui/contextview/contextview.js';
import { IMenuService } from '../../../../platform/actions/common/actions.js';
import { IMenuService, MenuItemAction } from '../../../../platform/actions/common/actions.js';
import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
import { Disposable } from '../../../../base/common/lifecycle.js';

Expand Down Expand Up @@ -252,7 +252,10 @@ class NativeContextMenuService extends Disposable implements IContextMenuService
this.telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: actionToRun.id, from: 'contextMenu' });
}

const context = delegate.getActionsContext ? delegate.getActionsContext(event) : undefined;
// Determine whether the action source is contributed via a proxy (e.g. from an external extension)
// Context will need to be serializable (e.g. no recursive structures)
const contributedAction = (actionToRun instanceof MenuItemAction) && actionToRun?.item?.source;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the existing context menus already utilising referenced objects in their commands which cannot be serialised (e.g. due to circular references or bigint types), a mechanism is required to determine if the context menu item has been contributed by an external extension.

The method chosen here relies on a source existing for external contributions. I'm not a bug fan of this, is there a better way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't try to use two different contexts for one toolbar. That is one thing that has always been an issue with exposing menus to extensions, and I'm not sure whether there's a better way to handle it, I will try to discuss that this week.

const context = delegate.getActionsContext ? delegate.getActionsContext(event, !!contributedAction) : undefined;

const runnable = actionRunner.run(actionToRun, context);
try {
Expand Down
8 changes: 8 additions & 0 deletions src/vscode-dts/vscode.proposed.debugContext.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

// empty placeholder for debug context menus

// https://github.com/microsoft/vscode/issues/200880 @thegecko