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 API for call stack selection changes (63943) #179132

Merged
merged 13 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/vs/workbench/api/browser/mainThreadDebugService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { URI as uri, UriComponents } from 'vs/base/common/uri';
import { IDebugService, IConfig, IDebugConfigurationProvider, IBreakpoint, IFunctionBreakpoint, IBreakpointData, IDebugAdapter, IDebugAdapterDescriptorFactory, IDebugSession, IDebugAdapterFactory, IDataBreakpoint, IDebugSessionOptions, IInstructionBreakpoint, DebugConfigurationProviderTriggerKind } from 'vs/workbench/contrib/debug/common/debug';
import {
ExtHostContext, ExtHostDebugServiceShape, MainThreadDebugServiceShape, DebugSessionUUID, MainContext,
IBreakpointsDeltaDto, ISourceMultiBreakpointDto, ISourceBreakpointDto, IFunctionBreakpointDto, IDebugSessionDto, IDataBreakpointDto, IStartDebuggingOptions, IDebugConfiguration
IBreakpointsDeltaDto, ISourceMultiBreakpointDto, ISourceBreakpointDto, IFunctionBreakpointDto, IDebugSessionDto, IDataBreakpointDto, IStartDebuggingOptions, IDebugConfiguration, IThreadFocusDto, IStackFrameFocusDto
} from 'vs/workbench/api/common/extHost.protocol';
import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
import severity from 'vs/base/common/severity';
Expand Down Expand Up @@ -56,6 +56,29 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
this._debugConfigurationProviders = new Map();
this._debugAdapterDescriptorFactories = new Map();
this._sessions = new Set();

this.debugService.getViewModel().onDidFocusThread(({ thread, explicit, session }) => {
roblourens marked this conversation as resolved.
Show resolved Hide resolved
if (session) {
const dto: IThreadFocusDto = {
kind: 'thread',
threadId: thread?.threadId,
sessionId: session!.getId(),
};
this._proxy.$acceptStackFrameFocus(dto);
}
});

this.debugService.getViewModel().onDidFocusStackFrame(({ stackFrame, explicit, session }) => {
roblourens marked this conversation as resolved.
Show resolved Hide resolved
if (session) {
const dto: IStackFrameFocusDto = {
kind: 'stackFrame',
threadId: stackFrame?.thread.threadId,
frameId: stackFrame?.frameId,
sessionId: session.getId(),
};
this._proxy.$acceptStackFrameFocus(dto);
}
});
}

public dispose(): void {
Expand Down
6 changes: 6 additions & 0 deletions src/vs/workbench/api/common/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
get breakpoints() {
return extHostDebugService.breakpoints;
},
get stackFrameFocus() {
return extHostDebugService.stackFrameFocus;
},
onDidStartDebugSession(listener, thisArg?, disposables?) {
return extHostDebugService.onDidStartDebugSession(listener, thisArg, disposables);
},
Expand All @@ -1119,6 +1122,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
onDidChangeBreakpoints(listener, thisArgs?, disposables?) {
return extHostDebugService.onDidChangeBreakpoints(listener, thisArgs, disposables);
},
onDidChangeStackFrameFocus(listener, thisArg?, disposables?) {
return extHostDebugService.onDidChangeStackFrameFocus(listener, thisArg, disposables);
},
registerDebugConfigurationProvider(debugType: string, provider: vscode.DebugConfigurationProvider, triggerKind?: vscode.DebugConfigurationProviderTriggerKind) {
return extHostDebugService.registerDebugConfigurationProvider(debugType, provider, triggerKind || DebugConfigurationProviderTriggerKind.Initial);
},
Expand Down
17 changes: 17 additions & 0 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,22 @@ export interface IDebugSessionFullDto {

export type IDebugSessionDto = IDebugSessionFullDto | DebugSessionUUID;

export type IDebugFocusType = 'thread' | 'stackFrame' | 'empty';
roblourens marked this conversation as resolved.
Show resolved Hide resolved

export interface IThreadFocusDto {
kind: 'thread';
threadId: number | undefined;
sessionId: string | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Can sessionId or threadId actually be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently, no, but i wanted to allow for it for the future? seemed easier to allow it now, than change to allow it later?
but i barely have any feeling about that, it seemed far-fetched that the 'thread' event should be used when there is no session, so i can make whatever change you suggest. thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'd rather just have it reflect whatever is possible now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've changed the sessionId to always be defined.
the threadId may still be undefined, both for thread and stackframe focus events.

}

export interface IStackFrameFocusDto {
kind: 'stackFrame';
threadId: number | undefined;
frameId: number | undefined;
sessionId: string | undefined;
}


export interface ExtHostDebugServiceShape {
$substituteVariables(folder: UriComponents | undefined, config: IConfig): Promise<IConfig>;
$runInTerminal(args: DebugProtocol.RunInTerminalRequestArguments, sessionId: string): Promise<number | undefined>;
Expand All @@ -2034,6 +2050,7 @@ export interface ExtHostDebugServiceShape {
$acceptDebugSessionCustomEvent(session: IDebugSessionDto, event: any): void;
$acceptBreakpointsDelta(delta: IBreakpointsDeltaDto): void;
$acceptDebugSessionNameChanged(session: IDebugSessionDto, name: string): void;
$acceptStackFrameFocus(focus: IThreadFocusDto | IStackFrameFocusDto | undefined): void;
}


Expand Down
45 changes: 44 additions & 1 deletion src/vs/workbench/api/common/extHostDebugService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { ISignService } from 'vs/platform/sign/common/sign';
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
import { DebugSessionUUID, ExtHostDebugServiceShape, IBreakpointsDeltaDto, IDebugSessionDto, IFunctionBreakpointDto, ISourceMultiBreakpointDto, MainContext, MainThreadDebugServiceShape } from 'vs/workbench/api/common/extHost.protocol';
import { DebugSessionUUID, ExtHostDebugServiceShape, IBreakpointsDeltaDto, IThreadFocusDto, IStackFrameFocusDto, IDebugSessionDto, IFunctionBreakpointDto, ISourceMultiBreakpointDto, MainContext, MainThreadDebugServiceShape } from 'vs/workbench/api/common/extHost.protocol';
import { IExtHostEditorTabs } from 'vs/workbench/api/common/extHostEditorTabs';
import { IExtHostExtensionService } from 'vs/workbench/api/common/extHostExtensionService';
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
Expand Down Expand Up @@ -41,6 +41,8 @@ export interface IExtHostDebugService extends ExtHostDebugServiceShape {
onDidReceiveDebugSessionCustomEvent: Event<vscode.DebugSessionCustomEvent>;
onDidChangeBreakpoints: Event<vscode.BreakpointsChangeEvent>;
breakpoints: vscode.Breakpoint[];
onDidChangeStackFrameFocus: Event<vscode.ThreadFocus | vscode.StackFrameFocus | undefined>;
stackFrameFocus: vscode.ThreadFocus | vscode.StackFrameFocus | undefined;

addBreakpoints(breakpoints0: readonly vscode.Breakpoint[]): Promise<void>;
removeBreakpoints(breakpoints0: readonly vscode.Breakpoint[]): Promise<void>;
Expand Down Expand Up @@ -91,6 +93,9 @@ export abstract class ExtHostDebugServiceBase implements IExtHostDebugService, E

private readonly _onDidChangeBreakpoints: Emitter<vscode.BreakpointsChangeEvent>;

private _stackFrameFocus: vscode.ThreadFocus | vscode.StackFrameFocus | undefined;
private readonly _onDidChangeStackFrameFocus: Emitter<vscode.ThreadFocus | vscode.StackFrameFocus | undefined>;

private _debugAdapters: Map<number, IDebugAdapter>;
private _debugAdaptersTrackers: Map<number, vscode.DebugAdapterTracker>;

Expand Down Expand Up @@ -129,6 +134,8 @@ export abstract class ExtHostDebugServiceBase implements IExtHostDebugService, E
}
});

this._onDidChangeStackFrameFocus = new Emitter<vscode.ThreadFocus | vscode.StackFrameFocus | undefined>();

this._activeDebugConsole = new ExtHostDebugConsole(this._debugServiceProxy);

this._breakpoints = new Map<string, vscode.Breakpoint>();
Expand Down Expand Up @@ -190,6 +197,15 @@ export abstract class ExtHostDebugServiceBase implements IExtHostDebugService, E

// extension debug API


get stackFrameFocus(): vscode.ThreadFocus | vscode.StackFrameFocus | undefined {
return this._stackFrameFocus;
}

get onDidChangeStackFrameFocus(): Event<vscode.ThreadFocus | vscode.StackFrameFocus | undefined> {
return this._onDidChangeStackFrameFocus.event;
}

get onDidChangeBreakpoints(): Event<vscode.BreakpointsChangeEvent> {
return this._onDidChangeBreakpoints.event;
}
Expand Down Expand Up @@ -584,6 +600,33 @@ export abstract class ExtHostDebugServiceBase implements IExtHostDebugService, E
this.fireBreakpointChanges(a, r, c);
}

public async $acceptStackFrameFocus(focusDto: IThreadFocusDto | IStackFrameFocusDto): Promise<void> {
//
let focus: vscode.ThreadFocus | vscode.StackFrameFocus;
const session = focusDto.sessionId ? await this.getSession(focusDto.sessionId) : undefined;
if (!session) {
throw new Error('no DebugSession found for debug focus context');
}

if (focusDto.kind === 'thread') {
focus = {
kind: focusDto.kind,
threadId: focusDto.threadId,
session,
};
} else {
focus = {
kind: focusDto.kind,
threadId: focusDto.threadId,
frameId: focusDto.frameId,
session,
};
}

this._stackFrameFocus = focus;
this._onDidChangeStackFrameFocus.fire(focus);
}

public $provideDebugConfigurations(configProviderHandle: number, folderUri: UriComponents | undefined, token: CancellationToken): Promise<vscode.DebugConfiguration[]> {
return asPromise(async () => {
const provider = this.getConfigProviderByHandle(configProviderHandle);
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/debug/common/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,8 @@ export interface IViewModel extends ITreeElement {
isMultiSessionView(): boolean;

onDidFocusSession: Event<IDebugSession | undefined>;
onDidFocusStackFrame: Event<{ stackFrame: IStackFrame | undefined; explicit: boolean }>;
onDidFocusThread: Event<{ thread: IThread | undefined; explicit: boolean; session: IDebugSession | undefined }>;
onDidFocusStackFrame: Event<{ stackFrame: IStackFrame | undefined; explicit: boolean; session: IDebugSession | undefined }>;
onDidSelectExpression: Event<{ expression: IExpression; settingWatch: boolean } | undefined>;
onDidEvaluateLazyExpression: Event<IExpressionContainer>;
onWillUpdateViews: Event<void>;
Expand Down
19 changes: 16 additions & 3 deletions src/vs/workbench/contrib/debug/common/debugViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export class ViewModel implements IViewModel {
private _focusedThread: IThread | undefined;
private selectedExpression: { expression: IExpression; settingWatch: boolean } | undefined;
private readonly _onDidFocusSession = new Emitter<IDebugSession | undefined>();
private readonly _onDidFocusStackFrame = new Emitter<{ stackFrame: IStackFrame | undefined; explicit: boolean }>();
private readonly _onDidFocusThread = new Emitter<{ thread: IThread | undefined; explicit: boolean; session: IDebugSession | undefined }>();
private readonly _onDidFocusStackFrame = new Emitter<{ stackFrame: IStackFrame | undefined; explicit: boolean; session: IDebugSession | undefined }>();
private readonly _onDidSelectExpression = new Emitter<{ expression: IExpression; settingWatch: boolean } | undefined>();
private readonly _onDidEvaluateLazyExpression = new Emitter<IExpressionContainer>();
private readonly _onWillUpdateViews = new Emitter<void>();
Expand Down Expand Up @@ -74,6 +75,10 @@ export class ViewModel implements IViewModel {
setFocus(stackFrame: IStackFrame | undefined, thread: IThread | undefined, session: IDebugSession | undefined, explicit: boolean): void {
const shouldEmitForStackFrame = this._focusedStackFrame !== stackFrame;
const shouldEmitForSession = this._focusedSession !== session;
// currently, it does not happen that shouldEmitForThread === true, but shouldEmitForStackFrame === false.
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I don't follow this comment. If I select a different thread under the same frame, I think I would expect it to fire for thread but not for stackframe, is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't understand; isn't it flipped, we select stack frames under threads?
there's two things: when you select a thread, the model always then sets the focus to a stack frame; you can't get a 'thread focused' event currently, but i wanted to allow for it. (I didn't want to introduce that yet, because its not required for my use case, and can be added later without changing this API).

Also, during my debugging, the stack frame comparison never was 'they do equal each other' because its using exact comparison, and they were always different objects. Didn't want to change due to risk, and also, the change was not needed to simply allow extensions to know about selection state.
Thanks for the comment - did this make sense? Wanted to make sure i understood your question before changing anything here.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I don't know what I was thinking, I looked at this for too long and got it mixed up in my head.

And re: the object comparison, I can look into it, but I would assume that object comparison should be ok, and we will for example create new stack frame objects on every step, and then it's a different frame and should fire the event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks. i've removed this comment. the comment later about firing only stackframe or thread focus should be enough

// the stack frame object comparison above is always false, so selecting a thread results in a stack frame focus.
const shouldEmitForThread = this._focusedThread !== thread;


this._focusedStackFrame = stackFrame;
this._focusedThread = thread;
Expand All @@ -98,16 +103,24 @@ export class ViewModel implements IViewModel {
if (shouldEmitForSession) {
this._onDidFocusSession.fire(session);
}

// should not fire this if a stack frame focus is fired.
Copy link
Member

Choose a reason for hiding this comment

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

Does "this" mean the thread event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes,made the comment more clear, thanks

if (shouldEmitForStackFrame) {
this._onDidFocusStackFrame.fire({ stackFrame, explicit });
this._onDidFocusStackFrame.fire({ stackFrame, explicit, session });
} else if (shouldEmitForThread) {
this._onDidFocusThread.fire({ thread, explicit, session });
}
}

get onDidFocusSession(): Event<IDebugSession | undefined> {
return this._onDidFocusSession.event;
}

get onDidFocusStackFrame(): Event<{ stackFrame: IStackFrame | undefined; explicit: boolean }> {
get onDidFocusThread(): Event<{ thread: IThread | undefined; explicit: boolean; session: IDebugSession | undefined }> {
return this._onDidFocusThread.event;
}

get onDidFocusStackFrame(): Event<{ stackFrame: IStackFrame | undefined; explicit: boolean; session: IDebugSession | undefined }> {
return this._onDidFocusStackFrame.event;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const allApiProposals = Object.freeze({
contribViewsRemote: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribViewsRemote.d.ts',
contribViewsWelcome: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribViewsWelcome.d.ts',
customEditorMove: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.customEditorMove.d.ts',
debugFocus: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.debugFocus.d.ts',
diffCommand: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.diffCommand.d.ts',
diffContentOptions: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.diffContentOptions.d.ts',
documentFiltersExclusive: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.documentFiltersExclusive.d.ts',
Expand Down
57 changes: 57 additions & 0 deletions src/vscode-dts/vscode.proposed.debugFocus.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

declare module 'vscode' {

// See https://github.com/microsoft/vscode/issues/63943

export interface ThreadFocus {
// discriminator
kind: 'thread';

/**
* Id of the debug session (DAP id).
Copy link
Member

Choose a reason for hiding this comment

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

Not an id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, ooops, fixed comment, thanks!

*/
readonly session: DebugSession;

/**
* Id of the associated thread (DAP id). May be undefined if thread has become unselected.
*/
readonly threadId: number | undefined;
}

export interface StackFrameFocus {
// discriminator
kind: 'stackFrame';

/**
* Id of the debug session (DAP id).
*/
readonly session: DebugSession;

/**
* Id of the associated thread (DAP id). May be undefined if a frame is unselected.
*/
readonly threadId: number | undefined;
/**
* Id of the stack frame (DAP id). May be undefined if a frame is unselected.
*/
readonly frameId: number | undefined;
}


export namespace debug {
/**
* The currently focused thread or stack frame id, or `undefined` if this has not been set. (e.g. not in debug mode).
*/
export let stackFrameFocus: ThreadFocus | StackFrameFocus | undefined;

/**
* An {@link Event} which fires when the {@link debug.stackFrameFocus} changes. Provides a sessionId. threadId is not undefined
* when a thread of frame has gained focus. frameId is defined when a stackFrame has gained focus.
*/
export const onDidChangeStackFrameFocus: Event<ThreadFocus | StackFrameFocus | undefined>;
}
}