Skip to content

Commit

Permalink
fix: various debug fixes and VS Code compatibility enhancements
Browse files Browse the repository at this point in the history
 - feat: added support for `debug/toolbar` and `debug/variables/context`,
 - feat: added support for `debugState` when context (eclipse-theia#11871),
 - feat: can customize debug session timeout, and error handling (eclipse-theia#11879),
 - fix: the `debugType` context that is not updated,
 - fix: `configure` must happen after receiving capabilities (eclipse-theia#11886),
 - fix: added missing conext menu in the _Variables_ view,
 - fix: handle `setFunctionBreakboints` response with no `body` (eclipse-theia#11885),
 - fix: `DebugExt` fires `didStart` event on `didCreate` (eclipse-theia#11916),
 - fix: validate editor selection based on the text model (eclipse-theia#11880)

Closes eclipse-theia#11871
Closes eclipse-theia#11879
Closes eclipse-theia#11885
Closes eclipse-theia#11886
Closes eclipse-theia#11916
Closes eclipse-theia#11880

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
  • Loading branch information
Akos Kitta committed Jan 17, 2023
1 parent d383fe2 commit daaecd4
Show file tree
Hide file tree
Showing 16 changed files with 170 additions and 37 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
- [plugin] added support for `isTransient` of `TerminalOptions` and `ExternalTerminalOptions` VS Code API [#12055](https://github.com/eclipse-theia/theia/pull/12055) - Contributed on behalf of STMicroelectronics
- [terminal] added support for preference `terminal.integrated.enablePersistentSessions` to allow disabling restoring terminals on reload [#12055](https://github.com/eclipse-theia/theia/pull/12055) - Contributed on behalf of STMicroelectronics

<a name="breaking_changes_1.34.0">[Breaking Changes:](#breaking_changes_1.34.0)</a>

- [plugin-ext] `DebugExtImpl#sessionDidCreate` has been replaced with `DebugExtImpl#sessionDidStart` to avoid prematurely firing a `didStart` event on `didCreate` [#11916](https://github.com/eclipse-theia/theia/issues/11916)

## v1.33.0 - 12/20/2022

- [application-package] added support for declaring extensions as peer dependencies [#11808](https://github.com/eclipse-theia/theia/pull/11808)
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@
"vscode.typescript-language-features": "https://open-vsx.org/api/vscode/typescript-language-features/1.62.3/file/vscode.typescript-language-features-1.62.3.vsix",
"EditorConfig.EditorConfig": "https://open-vsx.org/api/EditorConfig/EditorConfig/0.14.4/file/EditorConfig.EditorConfig-0.14.4.vsix",
"dbaeumer.vscode-eslint": "https://open-vsx.org/api/dbaeumer/vscode-eslint/2.1.1/file/dbaeumer.vscode-eslint-2.1.1.vsix",
"ms-vscode.references-view": "https://open-vsx.org/api/ms-vscode/references-view/0.0.89/file/ms-vscode.references-view-0.0.89.vsix"
"ms-vscode.references-view": "https://open-vsx.org/api/ms-vscode/references-view/0.0.89/file/ms-vscode.references-view-0.0.89.vsix",
"vscode.mock-debug": "https://github.com/kittaakos/vscode-mock-debug/raw/theia/mock-debug-0.51.0.vsix"
},
"theiaPluginsExcludeIds": [
"ms-vscode.js-debug-companion",
Expand Down
2 changes: 1 addition & 1 deletion packages/debug/src/browser/debug-session-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export class DebugSessionConnection implements Disposable {
};

this.pendingRequests.set(request.seq, result);
if (timeout) {
if (typeof timeout === 'number') {
const handle = setTimeout(() => {
const pendingRequest = this.pendingRequests.get(request.seq);
if (pendingRequest) {
Expand Down
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, debugStateLabel } 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(debugStateLabel(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', debugStateLabel(this.state));
this.breakpoints.onDidChangeMarkers(uri => this.fireDidChangeBreakpoints({ uri }));
this.labelProvider.onDidChange(event => {
for (const uriString of this.breakpoints.getUris()) {
Expand Down
103 changes: 77 additions & 26 deletions packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,22 @@ import { DebugContribution } from './debug-contribution';
import { Deferred, waitForEvent } from '@theia/core/lib/common/promise-util';
import { WorkspaceService } from '@theia/workspace/lib/browser';
import { DebugInstructionBreakpoint } from './model/debug-instruction-breakpoint';
import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';

export enum DebugState {
Inactive,
Initializing,
Running,
Stopped
}
export function debugStateLabel(state: DebugState): string {
switch (state) {
case DebugState.Initializing: return 'initializing';
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 +83,11 @@ 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;
/**
* Number of millis after a `stop` request times out.
*/
protected stopTimeout = 5_000;

constructor(
readonly id: string,
Expand Down Expand Up @@ -274,19 +287,26 @@ 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 || {});
const clientName = FrontendApplicationConfigProvider.get().applicationName;
try {
const response = await this.connection.sendRequest('initialize', {
clientID: clientName.toLocaleLowerCase().replace(/\s+/g, '_'),
clientName,
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 +324,17 @@ export class DebugSession implements CompositeTreeElement {
}
}

/**
* The `send('initialize')` request could resolve later than `on('initialized')` emits the event.
* 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 +369,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 {
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 +709,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 +748,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
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
33 changes: 30 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 { ActionMenuNode, CommandRegistry, CompositeMenuNode, 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,29 @@ export class DebugToolBar extends ReactWidget {
{this.renderStart()}
</React.Fragment>;
}

protected renderContributedCommands(): React.ReactNode {
return this.menuModelRegistry
.getMenu(DebugToolBar.MENU)
.children.filter(node => node instanceof CompositeMenuNode)
.map(node => (node as CompositeMenuNode).children)
.reduce((acc, curr) => acc.concat(curr), [])
.filter(node => node instanceof ActionMenuNode)
.map(node => this.debugAction(node as ActionMenuNode));
}

protected debugAction(node: ActionMenuNode): React.ReactNode {
const { label, command, when, icon: iconClass = '' } = node;
const run = () => this.commandRegistry.executeCommand(command);
const enabled = when ? this.contextKeyService.match(when) : true;
return enabled && <DebugAction
key={command}
enabled={enabled}
label={label}
iconClass={iconClass}
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.validateRange({ start, end });
}
return widget.editor.document.validatePosition(start);
}
const line = start && start.line !== undefined && start.line >= 0 ? start.line : undefined;
if (line === undefined) {
return undefined;
Expand Down
8 changes: 8 additions & 0 deletions packages/editor/src/browser/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ export interface TextEditorDocument extends lsp.TextDocument, Saveable, Disposab
* @since 1.8.0
*/
findMatches?(options: FindMatchesOptions): FindMatch[];
/**
* Create a valid position.
*/
validatePosition(position: Position): Position;
/**
* Create a valid range.
*/
validateRange(range: Range): Range;
}

// Refactoring
Expand Down
9 changes: 9 additions & 0 deletions packages/monaco/src/browser/monaco-editor-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,15 @@ export class MonacoEditorModel implements IResolvedTextEditorModel, TextEditorDo
return this.model.getLineMaxColumn(lineNumber);
}

validatePosition(position: Position): Position {
const { lineNumber, column } = this.model.validatePosition(this.p2m.asPosition(position));
return this.m2p.asPosition(lineNumber, column);
}

validateRange(range: Range): Range {
return this.m2p.asRange(this.model.validateRange(this.p2m.asRange(range)));
}

get readOnly(): boolean {
return this.resource.saveContents === undefined;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/common/plugin-api-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1799,7 +1799,7 @@ export interface DebugConfigurationProviderDescriptor {
export interface DebugExt {
$onSessionCustomEvent(sessionId: string, event: string, body?: any): void;
$breakpointsDidChange(added: Breakpoint[], removed: string[], changed: Breakpoint[]): void;
$sessionDidCreate(sessionId: string): void;
$sessionDidStart(sessionId: string): void;
$sessionDidDestroy(sessionId: string): void;
$sessionDidChange(sessionId: string | undefined): void;
$provideDebugConfigurationsByHandle(handle: number, workspaceFolder: string | undefined): Promise<theia.DebugConfiguration[]>;
Expand Down
Loading

0 comments on commit daaecd4

Please sign in to comment.