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

fix: various debug fixes and VS Code compatibility enhancements #11984

Merged
merged 16 commits into from
Jan 24, 2023
Merged
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
6 changes: 5 additions & 1 deletion packages/debug/src/browser/debug-session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { DebugConfiguration } from '../common/debug-common';
import { DebugError, DebugService } from '../common/debug-service';
import { BreakpointManager } from './breakpoint/breakpoint-manager';
import { DebugConfigurationManager } from './debug-configuration-manager';
import { DebugSession, DebugState } from './debug-session';
import { DebugSession, DebugState, debugStateContextValue } from './debug-session';
import { DebugSessionContributionRegistry, DebugSessionFactory } from './debug-session-contribution';
import { DebugCompoundRoot, DebugCompoundSessionOptions, DebugConfigurationSessionOptions, DebugSessionOptions, InternalDebugSessionOptions } from './debug-session-options';
import { DebugStackFrame } from './model/debug-stack-frame';
Expand Down Expand Up @@ -106,7 +106,9 @@ export class DebugSessionManager {
protected readonly onDidChangeEmitter = new Emitter<DebugSession | undefined>();
readonly onDidChange: Event<DebugSession | undefined> = this.onDidChangeEmitter.event;
protected fireDidChange(current: DebugSession | undefined): void {
this.debugTypeKey.set(current?.configuration.type);
this.inDebugModeKey.set(this.inDebugMode);
this.debugStateKey.set(debugStateContextValue(this.state));
this.onDidChangeEmitter.fire(current);
}

Expand Down Expand Up @@ -154,11 +156,13 @@ export class DebugSessionManager {

protected debugTypeKey: ContextKey<string>;
protected inDebugModeKey: ContextKey<boolean>;
protected debugStateKey: ContextKey<string>;

@postConstruct()
protected init(): void {
this.debugTypeKey = this.contextKeyService.createKey<string>('debugType', undefined);
this.inDebugModeKey = this.contextKeyService.createKey<boolean>('inDebugMode', this.inDebugMode);
this.debugStateKey = this.contextKeyService.createKey<string>('debugState', debugStateContextValue(this.state));
this.breakpoints.onDidChangeMarkers(uri => this.fireDidChangeBreakpoints({ uri }));
this.labelProvider.onDidChange(event => {
for (const uriString of this.breakpoints.getUris()) {
Expand Down
105 changes: 79 additions & 26 deletions packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ export enum DebugState {
Running,
Stopped
}
/**
* The mapped string values must not change as they are used for the `debugState` when context closure.
* For more details see the `Debugger contexts` section of the [official doc](https://code.visualstudio.com/api/references/when-clause-contexts#available-contexts).
*/
export function debugStateContextValue(state: DebugState): string {
switch (state) {
case DebugState.Initializing: return 'initializing';
kittaakos marked this conversation as resolved.
Show resolved Hide resolved
case DebugState.Stopped: return 'stopped';
case DebugState.Running: return 'running';
default: return 'inactive';
}
}

// FIXME: make injectable to allow easily inject services
export class DebugSession implements CompositeTreeElement {
Expand All @@ -74,7 +86,7 @@ export class DebugSession implements CompositeTreeElement {
protected readonly childSessions = new Map<string, DebugSession>();
protected readonly toDispose = new DisposableCollection();

private isStopping: boolean = false;
protected isStopping: boolean = false;

constructor(
readonly id: string,
Expand All @@ -89,6 +101,10 @@ export class DebugSession implements CompositeTreeElement {
protected readonly fileService: FileService,
protected readonly debugContributionProvider: ContributionProvider<DebugContribution>,
protected readonly workspaceService: WorkspaceService,
/**
* Number of millis after a `stop` request times out. It's 5 seconds by default.
*/
protected readonly stopTimeout = 5_000,
) {
this.connection.onRequest('runInTerminal', (request: DebugProtocol.RunInTerminalRequest) => this.runInTerminal(request));
this.connection.onDidClose(() => {
Expand Down Expand Up @@ -274,19 +290,25 @@ export class DebugSession implements CompositeTreeElement {
}

protected async initialize(): Promise<void> {
const response = await this.connection.sendRequest('initialize', {
clientID: 'Theia',
clientName: 'Theia IDE',
adapterID: this.configuration.type,
locale: 'en-US',
linesStartAt1: true,
columnsStartAt1: true,
pathFormat: 'path',
supportsVariableType: false,
supportsVariablePaging: false,
supportsRunInTerminalRequest: true
});
this.updateCapabilities(response?.body || {});
try {
const response = await this.connection.sendRequest('initialize', {
clientID: 'Theia',
clientName: 'Theia IDE',
adapterID: this.configuration.type,
locale: 'en-US',
linesStartAt1: true,
columnsStartAt1: true,
pathFormat: 'path',
supportsVariableType: false,
supportsVariablePaging: false,
supportsRunInTerminalRequest: true
});
this.updateCapabilities(response?.body || {});
this.didReceiveCapabilities.resolve();
} catch (err) {
this.didReceiveCapabilities.reject(err);
throw err;
}
}

protected async launchOrAttach(): Promise<void> {
Expand All @@ -304,8 +326,17 @@ export class DebugSession implements CompositeTreeElement {
}
}

/**
* The `send('initialize')` request could resolve later than `on('initialized')` emits the event.
tsmaeder marked this conversation as resolved.
Show resolved Hide resolved
* Hence, the `configure` would use the empty object `capabilities`.
* Using the empty `capabilities` could result in missing exception breakpoint filters, as
* always `capabilities.exceptionBreakpointFilters` is falsy. This deferred promise works
* around this timing issue. https://github.com/eclipse-theia/theia/issues/11886
*/
protected didReceiveCapabilities = new Deferred<void>();
protected initialized = false;
protected async configure(): Promise<void> {
await this.didReceiveCapabilities.promise;
if (this.capabilities.exceptionBreakpointFilters) {
const exceptionBreakpoints = [];
for (const filter of this.capabilities.exceptionBreakpointFilters) {
Expand Down Expand Up @@ -340,24 +371,39 @@ export class DebugSession implements CompositeTreeElement {
if (!this.isStopping) {
this.isStopping = true;
if (this.canTerminate()) {
const terminated = this.waitFor('terminated', 5000);
const terminated = this.waitFor('terminated', this.stopTimeout);
try {
await this.connection.sendRequest('terminate', { restart: isRestart }, 5000);
await this.connection.sendRequest('terminate', { restart: isRestart }, this.stopTimeout);
await terminated;
} catch (e) {
console.error('Did not receive terminated event in time', e);
this.handleTerminateError(e);
}
} else {
const terminateDebuggee = this.initialized && this.capabilities.supportTerminateDebuggee;
try {
await this.sendRequest('disconnect', { restart: isRestart }, 5000);
await this.sendRequest('disconnect', { restart: isRestart, terminateDebuggee }, this.stopTimeout);
} catch (e) {
console.error('Error on disconnect', e);
this.handleDisconnectError(e);
}
}
callback();
}
}

/**
* Invoked when sending the `terminate` request to the debugger is rejected or timed out.
*/
protected handleTerminateError(err: unknown): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

This introduces new API, and I don't really see why. What's the use case?

Copy link
Contributor Author

@kittaakos kittaakos Jan 18, 2023

Choose a reason for hiding this comment

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

It will help downstream to customize the error handling without overriding the entire stop method and copy-pasting code from Theia to downstream. This generally simplifies the Theia version bumps for extenders.

What's the use case?

I can add dummy customization to the api-samples extension.

console.error('Did not receive terminated event in time', err);
}

/**
* Invoked when sending the `disconnect` request to the debugger is rejected or timed out.
*/
protected handleDisconnectError(err: unknown): void {
console.error('Error on disconnect', err);
}

async disconnect(isRestart: boolean, callback: () => void): Promise<void> {
if (!this.isStopping) {
this.isStopping = true;
Expand Down Expand Up @@ -665,12 +711,17 @@ export class DebugSession implements CompositeTreeElement {
const response = await this.sendRequest('setFunctionBreakpoints', {
breakpoints: enabled.map(b => b.origin.raw)
});
response.body.breakpoints.forEach((raw, index) => {
// node debug adapter returns more breakpoints sometimes
if (enabled[index]) {
enabled[index].update({ raw });
}
});
// Apparently, `body` and `breakpoints` can be missing.
// https://github.com/eclipse-theia/theia/issues/11885
// https://github.com/microsoft/vscode/blob/80004351ccf0884b58359f7c8c801c91bb827d83/src/vs/workbench/contrib/debug/browser/debugSession.ts#L448-L449
if (response && response.body) {
response.body.breakpoints.forEach((raw, index) => {
// node debug adapter returns more breakpoints sometimes
if (enabled[index]) {
enabled[index].update({ raw });
}
});
}
} catch (error) {
// could be error or promise rejection of DebugProtocol.SetFunctionBreakpoints
if (error instanceof Error) {
Expand Down Expand Up @@ -699,10 +750,12 @@ export class DebugSession implements CompositeTreeElement {
);
const enabled = all.filter(b => b.enabled);
try {
const breakpoints = enabled.map(({ origin }) => origin.raw);
const response = await this.sendRequest('setBreakpoints', {
source: source.raw,
sourceModified,
breakpoints: enabled.map(({ origin }) => origin.raw)
breakpoints,
lines: breakpoints.map(({ line }) => line)
});
response.body.breakpoints.forEach((raw, index) => {
// node debug adapter returns more breakpoints sometimes
Expand Down
20 changes: 14 additions & 6 deletions packages/debug/src/browser/model/debug-stack-frame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,36 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement
}));
}

async open(options: WidgetOpenerOptions = {
mode: 'reveal'
}): Promise<EditorWidget | undefined> {
async open(options?: WidgetOpenerOptions): Promise<EditorWidget | undefined> {
if (!this.source) {
return undefined;
}
const { line, column, endLine, endColumn } = this.raw;
const selection: RecursivePartial<Range> = {
start: Position.create(line - 1, column - 1)
start: Position.create(this.clampPositive(line - 1), this.clampPositive(column - 1))
};
if (typeof endLine === 'number') {
selection.end = {
line: endLine - 1,
character: typeof endColumn === 'number' ? endColumn - 1 : undefined
line: this.clampPositive(endLine - 1),
character: typeof endColumn === 'number' ? this.clampPositive(endColumn - 1) : undefined
};
}
this.source.open({
mode: 'reveal',
...options,
selection
});
}

/**
* Debugger can send `column: 0` value despite of initializing the debug session with `columnsStartAt1: true`.
* This method can be used to ensure that neither `column` nor `column` are negative numbers.
* See https://github.com/microsoft/vscode-mock-debug/issues/85.
*/
protected clampPositive(value: number): number {
return Math.max(value, 0);
}

protected scopes: Promise<DebugScope[]> | undefined;
getScopes(): Promise<DebugScope[]> {
return this.scopes || (this.scopes = this.doGetScopes());
Expand Down
11 changes: 11 additions & 0 deletions packages/debug/src/browser/style/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,17 @@
opacity: 1;
}

.debug-toolbar .debug-action>div {
font-family: var(--theia-ui-font-family);
font-size: var(--theia-ui-font-size0);
display: flex;
align-items: center;
align-self: center;
justify-content: center;
text-align: center;
min-height: inherit;
}

/** Console */

#debug-console .theia-console-info {
Expand Down
9 changes: 7 additions & 2 deletions packages/debug/src/browser/view/debug-action.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,20 @@ export class DebugAction extends React.Component<DebugAction.Props> {

override render(): React.ReactNode {
const { enabled, label, iconClass } = this.props;
const classNames = ['debug-action', ...codiconArray(iconClass, true)];
const classNames = ['debug-action'];
if (iconClass) {
classNames.push(...codiconArray(iconClass, true));
}
if (enabled === false) {
classNames.push(DISABLED_CLASS);
}
return <span tabIndex={0}
className={classNames.join(' ')}
title={label}
onClick={this.props.run}
ref={this.setRef} />;
ref={this.setRef} >
{!iconClass && <div>{label}</div>}
</span>;
}

focus(): void {
Expand Down
47 changes: 44 additions & 3 deletions packages/debug/src/browser/view/debug-toolbar-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

import * as React from '@theia/core/shared/react';
import { inject, postConstruct, injectable } from '@theia/core/shared/inversify';
import { Disposable, DisposableCollection, MenuPath } from '@theia/core';
import { CommandMenuNode, CommandRegistry, CompoundMenuNode, Disposable, DisposableCollection, MenuModelRegistry, MenuPath } from '@theia/core';
import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
import { ReactWidget } from '@theia/core/lib/browser/widgets';
import { DebugViewModel } from './debug-view-model';
import { DebugState } from '../debug-session';
Expand All @@ -28,8 +29,10 @@ export class DebugToolBar extends ReactWidget {

static readonly MENU: MenuPath = ['debug-toolbar-menu'];

@inject(DebugViewModel)
protected readonly model: DebugViewModel;
@inject(CommandRegistry) protected readonly commandRegistry: CommandRegistry;
@inject(MenuModelRegistry) protected readonly menuModelRegistry: MenuModelRegistry;
@inject(ContextKeyService) protected readonly contextKeyService: ContextKeyService;
@inject(DebugViewModel) protected readonly model: DebugViewModel;

protected readonly onRender = new DisposableCollection();

Expand Down Expand Up @@ -65,6 +68,7 @@ export class DebugToolBar extends ReactWidget {
protected render(): React.ReactNode {
const { state } = this.model;
return <React.Fragment>
{this.renderContributedCommands()}
{this.renderContinue()}
<DebugAction enabled={state === DebugState.Stopped} run={this.stepOver} label={nls.localizeByDefault('Step Over')}
iconClass='debug-step-over' ref={this.setStepRef} />
Expand All @@ -77,6 +81,43 @@ export class DebugToolBar extends ReactWidget {
{this.renderStart()}
</React.Fragment>;
}

protected renderContributedCommands(): React.ReactNode {
const debugActions: React.ReactNode[] = [];
// first, search for CompoundMenuNodes:
this.menuModelRegistry.getMenu(DebugToolBar.MENU).children.forEach(compoundMenuNode => {
if (CompoundMenuNode.is(compoundMenuNode) && this.matchContext(compoundMenuNode.when)) {
// second, search for nested CommandMenuNodes:
compoundMenuNode.children.forEach(commandMenuNode => {
if (CommandMenuNode.is(commandMenuNode) && this.matchContext(commandMenuNode.when)) {
debugActions.push(this.debugAction(commandMenuNode));
}
});
}
});
return debugActions;
}

protected matchContext(when?: string): boolean {
return !when || this.contextKeyService.match(when);
}

protected debugAction(commandMenuNode: CommandMenuNode): React.ReactNode {
const { command, icon = '', label = '' } = commandMenuNode;
if (!label && !icon) {
const { when } = commandMenuNode;
console.warn(`Neither 'label' nor 'icon' properties were defined for the command menu node. (${JSON.stringify({ command, when })}}. Skipping.`);
return;
}
const run = () => this.commandRegistry.executeCommand(command);
return <DebugAction
key={command}
enabled={true}
label={label}
iconClass={icon}
run={run} />;
}

protected renderStart(): React.ReactNode {
const { state } = this.model;
if (state === DebugState.Inactive && this.model.sessionCount === 1) {
Expand Down
6 changes: 6 additions & 0 deletions packages/editor/src/browser/editor-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,12 @@ export class EditorManager extends NavigatableWidgetOpenHandler<EditorWidget> {

protected getSelection(widget: EditorWidget, selection: RecursivePartial<Range>): Range | Position | undefined {
const { start, end } = selection;
if (Position.is(start)) {
if (Position.is(end)) {
return widget.editor.document.toValidRange({ start, end });
}
return widget.editor.document.toValidPosition(start);
}
const line = start && start.line !== undefined && start.line >= 0 ? start.line : undefined;
if (line === undefined) {
return undefined;
Expand Down
Loading