From daaecd40f1e5bceb9fa588aed51dcc445b35d20e Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 16 Nov 2022 16:49:12 +0100 Subject: [PATCH 01/16] 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 (#11871), - feat: can customize debug session timeout, and error handling (#11879), - fix: the `debugType` context that is not updated, - fix: `configure` must happen after receiving capabilities (#11886), - fix: added missing conext menu in the _Variables_ view, - fix: handle `setFunctionBreakboints` response with no `body` (#11885), - fix: `DebugExt` fires `didStart` event on `didCreate` (#11916), - fix: validate editor selection based on the text model (#11880) Closes #11871 Closes #11879 Closes #11885 Closes #11886 Closes #11916 Closes #11880 Signed-off-by: Akos Kitta --- CHANGELOG.md | 4 + package.json | 3 +- .../src/browser/debug-session-connection.ts | 2 +- .../src/browser/debug-session-manager.ts | 6 +- packages/debug/src/browser/debug-session.tsx | 103 +++++++++++++----- packages/debug/src/browser/style/index.css | 11 ++ .../debug/src/browser/view/debug-action.tsx | 9 +- .../src/browser/view/debug-toolbar-widget.tsx | 33 +++++- packages/editor/src/browser/editor-manager.ts | 6 + packages/editor/src/browser/editor.ts | 8 ++ .../monaco/src/browser/monaco-editor-model.ts | 9 ++ .../plugin-ext/src/common/plugin-api-rpc.ts | 2 +- .../src/main/browser/debug/debug-main.ts | 2 +- .../menus/plugin-menu-command-adapter.ts | 1 + .../menus/vscode-theia-menu-mappings.ts | 6 + .../plugin-ext/src/plugin/debug/debug-ext.ts | 2 +- 16 files changed, 170 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f10331a02eba..15aedef92a304 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 +[Breaking Changes:](#breaking_changes_1.34.0) + +- [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) diff --git a/package.json b/package.json index 3fcfc46ca2866..5120a0569c70e 100644 --- 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 index e8ca6c9ec7123..983d560ed4734 100644 --- 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 index 1b0bd4b9f474a..3254219f15339 100644 --- 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(); readonly onDidChange: Event = 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; protected inDebugModeKey: ContextKey; + protected debugStateKey: ContextKey; @postConstruct() protected init(): void { this.debugTypeKey = this.contextKeyService.createKey('debugType', undefined); this.inDebugModeKey = this.contextKeyService.createKey('inDebugMode', this.inDebugMode); + this.debugStateKey = this.contextKeyService.createKey('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 index 130885be8d19b..9577798f4f2c2 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -43,6 +43,7 @@ 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, @@ -50,6 +51,14 @@ export enum DebugState { 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(); 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 { - 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 { @@ -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(); protected initialized = false; protected async configure(): Promise { + 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 { 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 index 68a694ee7ed0e..df1f8be4c3fef 100644 --- 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 index 4be5ec9d93956..f7ef0a3c19b5b 100644 --- a/packages/debug/src/browser/view/debug-action.tsx +++ b/packages/debug/src/browser/view/debug-action.tsx @@ -21,7 +21,10 @@ export class DebugAction extends React.Component { 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); } @@ -29,7 +32,9 @@ export class DebugAction extends React.Component { className={classNames.join(' ')} title={label} onClick={this.props.run} - ref={this.setRef} />; + ref={this.setRef} > + {!iconClass &&
{label}
} + ; } focus(): void { diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx index 705c29f6c19bc..25986ea9978df 100644 --- 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 + {this.renderContributedCommands()} {this.renderContinue()} @@ -77,6 +81,29 @@ export class DebugToolBar extends ReactWidget { {this.renderStart()} ; } + + 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 && ; + } + 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 index b9243d3256a6b..9db902ee13754 100644 --- 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 { protected getSelection(widget: EditorWidget, selection: RecursivePartial): 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 index 2326e52b583b3..7b726345875f2 100644 --- 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 index 105c1fc2d473f..efe86466a3b17 100644 --- 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 index d803933c75dc5..eb16a190d1f2c 100644 --- 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; diff --git a/packages/plugin-ext/src/main/browser/debug/debug-main.ts b/packages/plugin-ext/src/main/browser/debug/debug-main.ts index cbeb416440475..cd063cab2226e 100644 --- a/packages/plugin-ext/src/main/browser/debug/debug-main.ts +++ b/packages/plugin-ext/src/main/browser/debug/debug-main.ts @@ -113,7 +113,7 @@ export class DebugMainImpl implements DebugMain, Disposable { this.toDispose.pushAll([ this.breakpointsManager.onDidChangeBreakpoints(fireDidChangeBreakpoints), this.breakpointsManager.onDidChangeFunctionBreakpoints(fireDidChangeBreakpoints), - this.sessionManager.onDidCreateDebugSession(debugSession => this.debugExt.$sessionDidCreate(debugSession.id)), + this.sessionManager.onDidStartDebugSession(debugSession => this.debugExt.$sessionDidStart(debugSession.id)), this.sessionManager.onDidDestroyDebugSession(debugSession => this.debugExt.$sessionDidDestroy(debugSession.id)), this.sessionManager.onDidChangeActiveDebugSession(event => this.debugExt.$sessionDidChange(event.current && event.current.id)), this.sessionManager.onDidReceiveDebugSessionCustomEvent(event => this.debugExt.$onSessionCustomEvent(event.session.id, event.event, event.body)) diff --git a/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts b/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts index a76881060108c..bd82d054dbb38 100644 --- a/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts +++ b/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts @@ -89,6 +89,7 @@ export class PluginMenuCommandAdapter implements MenuCommandAdapter { ['comments/comment/title', toCommentArgs], ['comments/commentThread/context', toCommentArgs], ['debug/callstack/context', firstArgOnly], + ['debug/variables/context', firstArgOnly], ['debug/toolBar', noArgs], ['editor/context', selectedResource], ['editor/title', widgetURI], diff --git a/packages/plugin-ext/src/main/browser/menus/vscode-theia-menu-mappings.ts b/packages/plugin-ext/src/main/browser/menus/vscode-theia-menu-mappings.ts index 4b7c47f157871..97c04d2bb387c 100644 --- a/packages/plugin-ext/src/main/browser/menus/vscode-theia-menu-mappings.ts +++ b/packages/plugin-ext/src/main/browser/menus/vscode-theia-menu-mappings.ts @@ -21,6 +21,8 @@ import { injectable } from '@theia/core/shared/inversify'; import { URI as CodeUri } from '@theia/core/shared/vscode-uri'; import { DebugStackFramesWidget } from '@theia/debug/lib/browser/view/debug-stack-frames-widget'; import { DebugThreadsWidget } from '@theia/debug/lib/browser/view/debug-threads-widget'; +import { DebugToolBar } from '@theia/debug/lib/browser/view/debug-toolbar-widget'; +import { DebugVariablesWidget } from '@theia/debug/lib/browser/view/debug-variables-widget'; import { EditorWidget, EDITOR_CONTEXT_MENU } from '@theia/editor/lib/browser'; import { NAVIGATOR_CONTEXT_MENU } from '@theia/navigator/lib/browser/navigator-contribution'; import { ScmTreeWidget } from '@theia/scm/lib/browser/scm-tree-widget'; @@ -38,6 +40,8 @@ export const implementedVSCodeContributionPoints = [ 'comments/comment/title', 'comments/commentThread/context', 'debug/callstack/context', + 'debug/variables/context', + 'debug/toolBar', 'editor/context', 'editor/title', 'editor/title/context', @@ -59,6 +63,8 @@ export const codeToTheiaMappings = new Map([ ['comments/comment/title', [COMMENT_TITLE]], ['comments/commentThread/context', [COMMENT_THREAD_CONTEXT]], ['debug/callstack/context', [DebugStackFramesWidget.CONTEXT_MENU, DebugThreadsWidget.CONTEXT_MENU]], + ['debug/variables/context', [DebugVariablesWidget.CONTEXT_MENU]], + ['debug/toolBar', [DebugToolBar.MENU]], ['editor/context', [EDITOR_CONTEXT_MENU]], ['editor/title', [PLUGIN_EDITOR_TITLE_MENU]], ['editor/title/context', [SHELL_TABBAR_CONTEXT_MENU]], diff --git a/packages/plugin-ext/src/plugin/debug/debug-ext.ts b/packages/plugin-ext/src/plugin/debug/debug-ext.ts index 146d4d3b65d71..22e9d3d081a4e 100644 --- a/packages/plugin-ext/src/plugin/debug/debug-ext.ts +++ b/packages/plugin-ext/src/plugin/debug/debug-ext.ts @@ -252,7 +252,7 @@ export class DebugExtImpl implements DebugExt { } } - async $sessionDidCreate(sessionId: string): Promise { + async $sessionDidStart(sessionId: string): Promise { const session = this.sessions.get(sessionId); if (session) { this.onDidStartDebugSessionEmitter.fire(session); From e15e5cd8ac0bcf8b97dc64e4d9c345ad66f276ee Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 14 Dec 2022 09:45:37 +0100 Subject: [PATCH 02/16] fix: use when context for command node filtering Signed-off-by: Akos Kitta --- .../src/browser/view/debug-toolbar-widget.tsx | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx index 25986ea9978df..7d8033d878a3e 100644 --- a/packages/debug/src/browser/view/debug-toolbar-widget.tsx +++ b/packages/debug/src/browser/view/debug-toolbar-widget.tsx @@ -16,7 +16,7 @@ import * as React from '@theia/core/shared/react'; import { inject, postConstruct, injectable } from '@theia/core/shared/inversify'; -import { ActionMenuNode, CommandRegistry, CompositeMenuNode, Disposable, DisposableCollection, MenuModelRegistry, MenuPath } from '@theia/core'; +import { CommandMenuNode, CommandRegistry, CompoundMenuNode, Disposable, DisposableCollection, MenuModelRegistry, MenuNodeMetadata, 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'; @@ -83,24 +83,31 @@ export class DebugToolBar extends ReactWidget { } protected renderContributedCommands(): React.ReactNode { + const match = ({ when }: MenuNodeMetadata) => !when || this.contextKeyService.match(when); return this.menuModelRegistry - .getMenu(DebugToolBar.MENU) - .children.filter(node => node instanceof CompositeMenuNode) - .map(node => (node as CompositeMenuNode).children) + .getMenu(DebugToolBar.MENU).children + .filter(CompoundMenuNode.is) + .filter(compoundNode => match(compoundNode)) + .map(compoundNode => compoundNode.children) .reduce((acc, curr) => acc.concat(curr), []) - .filter(node => node instanceof ActionMenuNode) - .map(node => this.debugAction(node as ActionMenuNode)); + .filter(CommandMenuNode.is) + .filter(commandNode => match(commandNode)) + .map(commandNode => this.debugAction(commandNode)); } - protected debugAction(node: ActionMenuNode): React.ReactNode { - const { label, command, when, icon: iconClass = '' } = node; + protected debugAction(node: CommandMenuNode): React.ReactNode { + const { label = '', command, icon = '' } = node; + if (!label && !icon) { + const { when } = node; + console.warn(`Neither 'label' nor 'icon' properties were defined for the command menu node. ('command': ${command}${when ? `, 'when': ${when}` : ''}. Skipping.`); + return undefined; + } const run = () => this.commandRegistry.executeCommand(command); - const enabled = when ? this.contextKeyService.match(when) : true; - return enabled && ; } From 76f93cb0d9c2e7e79d5dc943094bc2c8a0eff027 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 14 Dec 2022 10:14:15 +0100 Subject: [PATCH 03/16] chore: removed test debug VSIX Signed-off-by: Akos Kitta --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 5120a0569c70e..3fcfc46ca2866 100644 --- a/package.json +++ b/package.json @@ -113,8 +113,7 @@ "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", - "vscode.mock-debug": "https://github.com/kittaakos/vscode-mock-debug/raw/theia/mock-debug-0.51.0.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" }, "theiaPluginsExcludeIds": [ "ms-vscode.js-debug-companion", From e10d75eeeb447e99c785144b644f418d8578eeba Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 11:33:35 +0100 Subject: [PATCH 04/16] fix: revert `timeout` check Signed-off-by: Akos Kitta --- packages/debug/src/browser/debug-session-connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/debug/src/browser/debug-session-connection.ts b/packages/debug/src/browser/debug-session-connection.ts index 983d560ed4734..e8ca6c9ec7123 100644 --- 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 (typeof timeout === 'number') { + if (timeout) { const handle = setTimeout(() => { const pendingRequest = this.pendingRequests.get(request.seq); if (pendingRequest) { From 963cc3d6105367609d9a85f589232a72766c2da5 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 11:41:04 +0100 Subject: [PATCH 05/16] fix: clarification on the possible `'debugState'` Signed-off-by: Akos Kitta --- packages/debug/src/browser/debug-session-manager.ts | 6 +++--- packages/debug/src/browser/debug-session.tsx | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/debug/src/browser/debug-session-manager.ts b/packages/debug/src/browser/debug-session-manager.ts index 3254219f15339..d5decd2125dc5 100644 --- 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, debugStateLabel } 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'; @@ -108,7 +108,7 @@ export class DebugSessionManager { protected fireDidChange(current: DebugSession | undefined): void { this.debugTypeKey.set(current?.configuration.type); this.inDebugModeKey.set(this.inDebugMode); - this.debugStateKey.set(debugStateLabel(this.state)); + this.debugStateKey.set(debugStateContextValue(this.state)); this.onDidChangeEmitter.fire(current); } @@ -162,7 +162,7 @@ export class DebugSessionManager { protected init(): void { this.debugTypeKey = this.contextKeyService.createKey('debugType', undefined); this.inDebugModeKey = this.contextKeyService.createKey('inDebugMode', this.inDebugMode); - this.debugStateKey = this.contextKeyService.createKey('debugState', debugStateLabel(this.state)); + this.debugStateKey = this.contextKeyService.createKey('debugState', debugStateContextValue(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 index 9577798f4f2c2..dea10018864fa 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -51,7 +51,11 @@ export enum DebugState { Running, Stopped } -export function debugStateLabel(state: DebugState): string { +/** + * 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'; case DebugState.Stopped: return 'stopped'; From 57e7f39edcb2f5f3236d26552d2c303594b819e4 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 11:43:01 +0100 Subject: [PATCH 06/16] fix: use hard-coded debugger `clientID` and `clientName` Signed-off-by: Akos Kitta --- packages/debug/src/browser/debug-session.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx index dea10018864fa..e3ea516c88f9c 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -43,7 +43,6 @@ 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, @@ -291,11 +290,10 @@ export class DebugSession implements CompositeTreeElement { } protected async initialize(): Promise { - const clientName = FrontendApplicationConfigProvider.get().applicationName; try { const response = await this.connection.sendRequest('initialize', { - clientID: clientName.toLocaleLowerCase().replace(/\s+/g, '_'), - clientName, + clientID: 'Theia', + clientName: 'Theia IDE', adapterID: this.configuration.type, locale: 'en-US', linesStartAt1: true, From 2b6e114695481f0b723230fbb93f03eba3381ecb Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 11:44:44 +0100 Subject: [PATCH 07/16] fix: use review-requested method name Signed-off-by: Akos Kitta --- packages/debug/src/browser/view/debug-toolbar-widget.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx index 7d8033d878a3e..1a6a376c4c777 100644 --- a/packages/debug/src/browser/view/debug-toolbar-widget.tsx +++ b/packages/debug/src/browser/view/debug-toolbar-widget.tsx @@ -92,10 +92,10 @@ export class DebugToolBar extends ReactWidget { .reduce((acc, curr) => acc.concat(curr), []) .filter(CommandMenuNode.is) .filter(commandNode => match(commandNode)) - .map(commandNode => this.debugAction(commandNode)); + .map(commandNode => this.createDebugAction(commandNode)); } - protected debugAction(node: CommandMenuNode): React.ReactNode { + protected createDebugAction(node: CommandMenuNode): React.ReactNode { const { label = '', command, icon = '' } = node; if (!label && !icon) { const { when } = node; From 96954d28fc72319d5e6b179b80b3789adc311899 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:03:27 +0100 Subject: [PATCH 08/16] fix: changed method name + added doc Signed-off-by: Akos Kitta --- packages/editor/src/browser/editor.ts | 11 +++++++---- packages/monaco/src/browser/monaco-editor-model.ts | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/editor/src/browser/editor.ts b/packages/editor/src/browser/editor.ts index 7b726345875f2..97367f57f6935 100644 --- a/packages/editor/src/browser/editor.ts +++ b/packages/editor/src/browser/editor.ts @@ -34,13 +34,16 @@ export interface TextEditorDocument extends lsp.TextDocument, Saveable, Disposab */ findMatches?(options: FindMatchesOptions): FindMatch[]; /** - * Create a valid position. + * Creates a valid position. If the position is outside of the backing document, this method will return a position that is ensured to be inside the document and valid. + * For example, when the `position` is `{ line: 1, character: 0 }` and the document is empty, this method will return with `{ line: 0, character: 0 }`. */ - validatePosition(position: Position): Position; + toValidPosition(position: Position): Position; /** - * Create a valid range. + * Creates a valid range. If the `range` argument is outside of the document, this method will return with a new range that does not exceed the boundaries of the document. + * For example, if the argument is `{ start: { line: 1, character: 0 }, end: { line: 1, character: 0 } }` and the document is empty, the return value is + * `{ start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }`. */ - validateRange(range: Range): Range; + toValidRange(range: Range): Range; } // Refactoring diff --git a/packages/monaco/src/browser/monaco-editor-model.ts b/packages/monaco/src/browser/monaco-editor-model.ts index efe86466a3b17..cd0f4b36e0e48 100644 --- a/packages/monaco/src/browser/monaco-editor-model.ts +++ b/packages/monaco/src/browser/monaco-editor-model.ts @@ -293,12 +293,12 @@ export class MonacoEditorModel implements IResolvedTextEditorModel, TextEditorDo return this.model.getLineMaxColumn(lineNumber); } - validatePosition(position: Position): Position { + toValidPosition(position: Position): Position { const { lineNumber, column } = this.model.validatePosition(this.p2m.asPosition(position)); return this.m2p.asPosition(lineNumber, column); } - validateRange(range: Range): Range { + toValidRange(range: Range): Range { return this.m2p.asRange(this.model.validateRange(this.p2m.asRange(range))); } From 7d995566abc9c5165bc22a15955fb9854b2c04e8 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:07:11 +0100 Subject: [PATCH 09/16] fix: `stopTimeout` is a default `ctor` argument Signed-off-by: Akos Kitta --- packages/debug/src/browser/debug-session.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx index e3ea516c88f9c..c8825227121e2 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -87,10 +87,6 @@ export class DebugSession implements CompositeTreeElement { protected readonly toDispose = new DisposableCollection(); protected isStopping: boolean = false; - /** - * Number of millis after a `stop` request times out. - */ - protected stopTimeout = 5_000; constructor( readonly id: string, @@ -105,6 +101,10 @@ export class DebugSession implements CompositeTreeElement { protected readonly fileService: FileService, protected readonly debugContributionProvider: ContributionProvider, 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(() => { From 84b752d5a20b428bbadf8cef6ff6e3c73ad3b782 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:14:28 +0100 Subject: [PATCH 10/16] fix: incorrect method name Signed-off-by: Akos Kitta --- packages/editor/src/browser/editor-manager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/browser/editor-manager.ts b/packages/editor/src/browser/editor-manager.ts index 9db902ee13754..a30e6ea4f5ca3 100644 --- a/packages/editor/src/browser/editor-manager.ts +++ b/packages/editor/src/browser/editor-manager.ts @@ -255,9 +255,9 @@ export class EditorManager extends NavigatableWidgetOpenHandler { const { start, end } = selection; if (Position.is(start)) { if (Position.is(end)) { - return widget.editor.document.validateRange({ start, end }); + return widget.editor.document.toValidRange({ start, end }); } - return widget.editor.document.validatePosition(start); + return widget.editor.document.toValidPosition(start); } const line = start && start.line !== undefined && start.line >= 0 ? start.line : undefined; if (line === undefined) { From 2b79c77b314dc500d541b83eafa192557d55845a Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:16:29 +0100 Subject: [PATCH 11/16] fix: both `didCreate` and `didStart` must be API Signed-off-by: Akos Kitta --- CHANGELOG.md | 4 ---- packages/plugin-ext/src/common/plugin-api-rpc.ts | 1 + packages/plugin-ext/src/plugin/debug/debug-ext.ts | 12 ++++++++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15aedef92a304..6f10331a02eba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,6 @@ - [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 -[Breaking Changes:](#breaking_changes_1.34.0) - -- [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) diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts index eb16a190d1f2c..e31f1085bffa4 100644 --- a/packages/plugin-ext/src/common/plugin-api-rpc.ts +++ b/packages/plugin-ext/src/common/plugin-api-rpc.ts @@ -1799,6 +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; diff --git a/packages/plugin-ext/src/plugin/debug/debug-ext.ts b/packages/plugin-ext/src/plugin/debug/debug-ext.ts index 22e9d3d081a4e..4bf42e7129a50 100644 --- a/packages/plugin-ext/src/plugin/debug/debug-ext.ts +++ b/packages/plugin-ext/src/plugin/debug/debug-ext.ts @@ -67,6 +67,7 @@ export class DebugExtImpl implements DebugExt { private readonly onDidChangeBreakpointsEmitter = new Emitter(); private readonly onDidChangeActiveDebugSessionEmitter = new Emitter(); private readonly onDidTerminateDebugSessionEmitter = new Emitter(); + private readonly onDidCreateDebugSessionEmitter = new Emitter(); private readonly onDidStartDebugSessionEmitter = new Emitter(); private readonly onDidReceiveDebugSessionCustomEmitter = new Emitter(); @@ -131,6 +132,10 @@ export class DebugExtImpl implements DebugExt { return this.onDidTerminateDebugSessionEmitter.event; } + get onDidCreateDebugSession(): theia.Event { + return this.onDidCreateDebugSessionEmitter.event; + } + get onDidStartDebugSession(): theia.Event { return this.onDidStartDebugSessionEmitter.event; } @@ -252,6 +257,13 @@ export class DebugExtImpl implements DebugExt { } } + async $sessionDidCreate(sessionId: string): Promise { + const session = this.sessions.get(sessionId); + if (session) { + this.onDidCreateDebugSessionEmitter.fire(session); + } + } + async $sessionDidStart(sessionId: string): Promise { const session = this.sessions.get(sessionId); if (session) { From 4e5057c5607686aa146c27d4d9c7bdec4609abd6 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:18:37 +0100 Subject: [PATCH 12/16] fix: call both on create and start Signed-off-by: Akos Kitta --- packages/plugin-ext/src/main/browser/debug/debug-main.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/plugin-ext/src/main/browser/debug/debug-main.ts b/packages/plugin-ext/src/main/browser/debug/debug-main.ts index cd063cab2226e..4221473312236 100644 --- a/packages/plugin-ext/src/main/browser/debug/debug-main.ts +++ b/packages/plugin-ext/src/main/browser/debug/debug-main.ts @@ -113,6 +113,7 @@ export class DebugMainImpl implements DebugMain, Disposable { this.toDispose.pushAll([ this.breakpointsManager.onDidChangeBreakpoints(fireDidChangeBreakpoints), this.breakpointsManager.onDidChangeFunctionBreakpoints(fireDidChangeBreakpoints), + this.sessionManager.onDidCreateDebugSession(debugSession => this.debugExt.$sessionDidCreate(debugSession.id)), this.sessionManager.onDidStartDebugSession(debugSession => this.debugExt.$sessionDidStart(debugSession.id)), this.sessionManager.onDidDestroyDebugSession(debugSession => this.debugExt.$sessionDidDestroy(debugSession.id)), this.sessionManager.onDidChangeActiveDebugSession(event => this.debugExt.$sessionDidChange(event.current && event.current.id)), From 804c93f3b277bdcb9c772bebde6919c91b991e44 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Wed, 18 Jan 2023 12:34:18 +0100 Subject: [PATCH 13/16] fix: workaround for microsoft/vscode-mock-debug#85 Signed-off-by: Akos Kitta --- .../debug/src/browser/model/debug-stack-frame.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/debug/src/browser/model/debug-stack-frame.tsx b/packages/debug/src/browser/model/debug-stack-frame.tsx index 9e70411c4337b..cca9de7940098 100644 --- a/packages/debug/src/browser/model/debug-stack-frame.tsx +++ b/packages/debug/src/browser/model/debug-stack-frame.tsx @@ -72,12 +72,12 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement } const { line, column, endLine, endColumn } = this.raw; const selection: RecursivePartial = { - start: Position.create(line - 1, column - 1) + start: Position.create(this.coerceNonNegative(line - 1), this.coerceNonNegative(column - 1)) }; if (typeof endLine === 'number') { selection.end = { - line: endLine - 1, - character: typeof endColumn === 'number' ? endColumn - 1 : undefined + line: this.coerceNonNegative(endLine - 1), + character: typeof endColumn === 'number' ? this.coerceNonNegative(endColumn - 1) : undefined }; } this.source.open({ @@ -86,6 +86,15 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement }); } + /** + * 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 coerceNonNegative(value: number): number { + return value < 0 ? 0 : value; + } + protected scopes: Promise | undefined; getScopes(): Promise { return this.scopes || (this.scopes = this.doGetScopes()); From 3a8e329e4df418f98060c34bbd4d55f452b7c099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Thu, 19 Jan 2023 05:01:30 -0500 Subject: [PATCH 14/16] simplify writing The collection of contributed commands was written in a convoluted way, this commit makes it more straightforward with just 2 loops. --- .../src/browser/view/debug-toolbar-widget.tsx | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx index 1a6a376c4c777..66c3d0c08d8db 100644 --- a/packages/debug/src/browser/view/debug-toolbar-widget.tsx +++ b/packages/debug/src/browser/view/debug-toolbar-widget.tsx @@ -16,7 +16,7 @@ import * as React from '@theia/core/shared/react'; import { inject, postConstruct, injectable } from '@theia/core/shared/inversify'; -import { CommandMenuNode, CommandRegistry, CompoundMenuNode, Disposable, DisposableCollection, MenuModelRegistry, MenuNodeMetadata, 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'; @@ -83,24 +83,31 @@ export class DebugToolBar extends ReactWidget { } protected renderContributedCommands(): React.ReactNode { - const match = ({ when }: MenuNodeMetadata) => !when || this.contextKeyService.match(when); - return this.menuModelRegistry - .getMenu(DebugToolBar.MENU).children - .filter(CompoundMenuNode.is) - .filter(compoundNode => match(compoundNode)) - .map(compoundNode => compoundNode.children) - .reduce((acc, curr) => acc.concat(curr), []) - .filter(CommandMenuNode.is) - .filter(commandNode => match(commandNode)) - .map(commandNode => this.createDebugAction(commandNode)); + 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 createDebugAction(node: CommandMenuNode): React.ReactNode { - const { label = '', command, icon = '' } = node; + 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 } = node; - console.warn(`Neither 'label' nor 'icon' properties were defined for the command menu node. ('command': ${command}${when ? `, 'when': ${when}` : ''}. Skipping.`); - return undefined; + 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 Date: Thu, 19 Jan 2023 06:16:14 -0500 Subject: [PATCH 15/16] use `Math.max` to clamp values into positives The ternary implementation is stricly equivalent to the `Math.max` function, so use that instead. --- packages/debug/src/browser/model/debug-stack-frame.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/debug/src/browser/model/debug-stack-frame.tsx b/packages/debug/src/browser/model/debug-stack-frame.tsx index cca9de7940098..2ca20268aa9e2 100644 --- a/packages/debug/src/browser/model/debug-stack-frame.tsx +++ b/packages/debug/src/browser/model/debug-stack-frame.tsx @@ -72,12 +72,12 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement } const { line, column, endLine, endColumn } = this.raw; const selection: RecursivePartial = { - start: Position.create(this.coerceNonNegative(line - 1), this.coerceNonNegative(column - 1)) + start: Position.create(this.clampPositive(line - 1), this.clampPositive(column - 1)) }; if (typeof endLine === 'number') { selection.end = { - line: this.coerceNonNegative(endLine - 1), - character: typeof endColumn === 'number' ? this.coerceNonNegative(endColumn - 1) : undefined + line: this.clampPositive(endLine - 1), + character: typeof endColumn === 'number' ? this.clampPositive(endColumn - 1) : undefined }; } this.source.open({ @@ -91,8 +91,8 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement * 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 coerceNonNegative(value: number): number { - return value < 0 ? 0 : value; + protected clampPositive(value: number): number { + return Math.max(value, 0); } protected scopes: Promise | undefined; From d6e57abd198a6bf2247383923e161d25bb3c3eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Thu, 19 Jan 2023 06:18:40 -0500 Subject: [PATCH 16/16] fix default option value handling Assigning a default option object to set default values is problematic as said default values will be discarded if any option object is passed. Instead default values must be handled in conjuction of whatever options are passed to functions. --- packages/debug/src/browser/model/debug-stack-frame.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/debug/src/browser/model/debug-stack-frame.tsx b/packages/debug/src/browser/model/debug-stack-frame.tsx index 2ca20268aa9e2..67b3c14ced513 100644 --- a/packages/debug/src/browser/model/debug-stack-frame.tsx +++ b/packages/debug/src/browser/model/debug-stack-frame.tsx @@ -64,9 +64,7 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement })); } - async open(options: WidgetOpenerOptions = { - mode: 'reveal' - }): Promise { + async open(options?: WidgetOpenerOptions): Promise { if (!this.source) { return undefined; } @@ -81,6 +79,7 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement }; } this.source.open({ + mode: 'reveal', ...options, selection });