fix: various debug fixes and VS Code compatibility enhancements

- 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 <[email protected]>
kittaakos · Dec 13, 2022 · 90f99b0 · 90f99b0
1 parent 1446bca
commit 90f99b0
Show file tree

Hide file tree

Showing 16 changed files with 167 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 
 - [core] returns of many methods of `MenuModelRegistry` changed from `CompositeMenuNode` to `MutableCompoundMenuNode`. To mutate a menu, use the `updateOptions` method or add a check for `instanceof CompositeMenuNode`, which will be true in most cases.
 - [plugin-ext] refactored the plugin RPC API - now also reuses the msgpackR based RPC protocol that is better suited for handling binary data and enables message tunneling [#11228](https://github.com/eclipse-theia/theia/pull/11261). All plugin protocol types now use `UInt8Array` as type for message parameters instead of `string`  - Contributed on behalf of STMicroelectronics.
+- [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.32.0 - 11/24/2022
 

diff --git a/package.json b/package.json
@@ -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",

diff --git a/packages/debug/src/browser/debug-session-connection.ts b/packages/debug/src/browser/debug-session-connection.ts
@@ -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) {

diff --git a/packages/debug/src/browser/debug-session-manager.ts b/packages/debug/src/browser/debug-session-manager.ts
@@ -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';
@@ -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);
     }
 
@@ -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()) {

diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx
@@ -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 {
@@ -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,
@@ -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> {
@@ -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) {
@@ -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;
@@ -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) {
@@ -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

diff --git a/packages/debug/src/browser/style/index.css b/packages/debug/src/browser/style/index.css
@@ -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 {

diff --git a/packages/debug/src/browser/view/debug-action.tsx b/packages/debug/src/browser/view/debug-action.tsx
@@ -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 {

diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx
@@ -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';
@@ -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();
 
@@ -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} />
@@ -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) {

diff --git a/packages/editor/src/browser/editor-manager.ts b/packages/editor/src/browser/editor-manager.ts
@@ -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;

diff --git a/packages/editor/src/browser/editor.ts b/packages/editor/src/browser/editor.ts
@@ -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

diff --git a/packages/monaco/src/browser/monaco-editor-model.ts b/packages/monaco/src/browser/monaco-editor-model.ts
@@ -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;
     }

diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts
@@ -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[]>;