From 4b0c0033c498c1e0d0a3f6258e72e66ec793b740 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Tue, 12 Mar 2024 10:25:50 +0100 Subject: [PATCH] Further enhancements: Context menu, memory event, PR feedback - Integrate commands with context menu - React to 'memory' event from debug adapter - Use 'Store Memory to File' as proper storage label - Use workspace folder as default save location - Minor fix for numpad key enter on memory options --- package.json | 12 ++++- src/common/debug-requests.ts | 27 ++++++++++ src/common/messaging.ts | 3 +- src/common/webview-context.ts | 10 ++++ .../adapter-registry/adapter-capabilities.ts | 29 ++--------- src/plugin/memory-provider.ts | 50 ++++++++++++------- src/plugin/memory-storage.ts | 16 ++++-- src/webview/components/memory-table.tsx | 2 +- src/webview/components/memory-widget.tsx | 1 + src/webview/components/options-widget.tsx | 2 +- src/webview/memory-webview-view.tsx | 13 +++-- 11 files changed, 111 insertions(+), 54 deletions(-) diff --git a/package.json b/package.json index 887479e..d9d0453 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,7 @@ }, { "command": "memory-inspector.store-file", - "title": "Store Memory as File", + "title": "Store Memory to File", "enablement": "memory-inspector.canRead", "category": "Memory" }, @@ -190,6 +190,16 @@ "command": "memory-inspector.show-advanced-display-options", "group": "display@4", "when": "webviewId === memory-inspector.memory" + }, + { + "command": "memory-inspector.store-file", + "group": "display@5", + "when": "webviewId === memory-inspector.memory" + }, + { + "command": "memory-inspector.apply-file", + "group": "display@6", + "when": "webviewId === memory-inspector.memory" } ] }, diff --git a/src/common/debug-requests.ts b/src/common/debug-requests.ts index 4313a3a..1e2f54f 100644 --- a/src/common/debug-requests.ts +++ b/src/common/debug-requests.ts @@ -20,10 +20,22 @@ import type { DebugSession } from 'vscode'; export interface DebugRequestTypes { 'evaluate': [DebugProtocol.EvaluateArguments, DebugProtocol.EvaluateResponse['body']] + 'initialize': [DebugProtocol.InitializeRequestArguments, DebugProtocol.InitializeResponse['body']] 'readMemory': [DebugProtocol.ReadMemoryArguments, DebugProtocol.ReadMemoryResponse['body']] + 'scopes': [DebugProtocol.ScopesArguments, DebugProtocol.ScopesResponse['body']] + 'variables': [DebugProtocol.VariablesArguments, DebugProtocol.VariablesResponse['body']] 'writeMemory': [DebugProtocol.WriteMemoryArguments, DebugProtocol.WriteMemoryResponse['body']] } +export interface DebugEvents { + 'memory': DebugProtocol.MemoryEvent, + 'stopped': DebugProtocol.StoppedEvent +} + +export type DebugRequest = Omit & { command: COMMAND, arguments: ARGS }; +export type DebugResponse = Omit & { command: COMMAND, body: BODY }; +export type DebugEvent = DebugProtocol.Event & { body: T }; + export async function sendRequest(session: DebugSession, command: K, args: DebugRequestTypes[K][0]): Promise { return session.customRequest(command, args); @@ -43,3 +55,18 @@ export function isDebugEvaluateArguments(args: DebugProtocol.EvaluateArguments | const assumed = args ? args as DebugProtocol.EvaluateArguments : undefined; return typeof assumed?.expression === 'string'; } + +export function isDebugRequest(command: K, message: unknown): message is DebugRequest { + const assumed = message ? message as DebugProtocol.Request : undefined; + return !!assumed && assumed.type === 'request' && assumed.command === command; +} + +export function isDebugResponse(command: K, message: unknown): message is DebugResponse { + const assumed = message ? message as DebugProtocol.Response : undefined; + return !!assumed && assumed.type === 'response' && assumed.command === command; +} + +export function isDebugEvent(event: K, message: unknown): message is DebugEvents[K] { + const assumed = message ? message as DebugProtocol.Event : undefined; + return !!assumed && assumed.type === 'event' && assumed.event === event; +} diff --git a/src/common/messaging.ts b/src/common/messaging.ts index d4836d5..2b71c6f 100644 --- a/src/common/messaging.ts +++ b/src/common/messaging.ts @@ -21,6 +21,7 @@ import type { VariableRange, WrittenMemory } from './memory-range'; import { DebugRequestTypes } from './debug-requests'; import { URI } from 'vscode-uri'; import { VariablesView } from '../plugin/external-views'; +import { WebviewContext } from './webview-context'; // convenience types for easier readability and better semantics export type MemoryOptions = Partial; @@ -31,7 +32,7 @@ export type ReadMemoryResult = DebugRequestTypes['readMemory'][1]; export type WriteMemoryArguments = DebugRequestTypes['writeMemory'][0] & { count?: number }; export type WriteMemoryResult = DebugRequestTypes['writeMemory'][1]; -export type StoreMemoryArguments = MemoryOptions & { proposedOutputName?: string } | VariablesView.IVariablesContext; +export type StoreMemoryArguments = MemoryOptions & { proposedOutputName?: string } | VariablesView.IVariablesContext | WebviewContext; export type StoreMemoryResult = void; export type ApplyMemoryArguments = URI | undefined; diff --git a/src/common/webview-context.ts b/src/common/webview-context.ts index 044c442..264b11a 100644 --- a/src/common/webview-context.ts +++ b/src/common/webview-context.ts @@ -16,6 +16,7 @@ import { WebviewIdMessageParticipant } from 'vscode-messenger-common'; import { VariableMetadata } from './memory-range'; +import { ReadMemoryArguments } from './messaging'; export interface WebviewContext { messageParticipant: WebviewIdMessageParticipant, @@ -23,6 +24,7 @@ export interface WebviewContext { showAsciiColumn: boolean showVariablesColumn: boolean, showRadixPrefix: boolean, + activeReadArguments: Required } export interface WebviewCellContext extends WebviewContext { @@ -48,3 +50,11 @@ export function getVisibleColumns(context: WebviewContext): string[] { } return columns; } + +export function isWebviewContext(args: WebviewContext | unknown): args is WebviewContext { + const assumed = args ? args as WebviewContext : undefined; + return typeof assumed?.messageParticipant?.type === 'string' && assumed.messageParticipant.type === 'webview' && typeof assumed.messageParticipant.webviewId === 'string' + && typeof assumed.webviewSection === 'string' && typeof assumed.showAsciiColumn === 'boolean' && typeof assumed.showVariablesColumn === 'boolean' + && typeof assumed.showRadixPrefix === 'boolean' && typeof assumed.activeReadArguments?.count === 'number' && typeof assumed.activeReadArguments?.offset === 'number' + && typeof assumed.activeReadArguments?.memoryReference === 'string'; +} diff --git a/src/plugin/adapter-registry/adapter-capabilities.ts b/src/plugin/adapter-registry/adapter-capabilities.ts index d0512aa..6d029fe 100644 --- a/src/plugin/adapter-registry/adapter-capabilities.ts +++ b/src/plugin/adapter-registry/adapter-capabilities.ts @@ -18,6 +18,7 @@ import * as vscode from 'vscode'; import { DebugProtocol } from '@vscode/debugprotocol'; import { VariableRange } from '../../common/memory-range'; import { Logger } from '../logger'; +import { isDebugRequest, isDebugResponse } from '../../common/debug-requests'; /** Represents capabilities that may be achieved with particular debug adapters but are not part of the DAP */ export interface AdapterCapabilities { @@ -46,9 +47,9 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker { constructor(protected readonly onEnd: vscode.Disposable, protected logger: Logger) { } onWillReceiveMessage(message: unknown): void { - if (isScopesRequest(message)) { + if (isDebugRequest('scopes', message)) { this.currentFrame = message.arguments.frameId; - } else if (isVariableRequest(message)) { + } else if (isDebugRequest('variables', message)) { if (message.arguments.variablesReference in this.variablesTree) { this.pendingMessages.set(message.seq, message.arguments.variablesReference); } @@ -57,7 +58,7 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker { /** Produces a two-level tree of scopes and their immediate children. Does not handle expansion of complex variables. */ onDidSendMessage(message: unknown): void { - if (isScopesResponse(message)) { + if (isDebugResponse('scopes', message)) { this.variablesTree = {}; // Scopes request implies that all scopes will be queried again. for (const scope of message.body.scopes) { if (this.isDesiredScope(scope)) { @@ -66,7 +67,7 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker { } } } - } else if (isVariableResponse(message)) { + } else if (isDebugResponse('variables', message)) { if (this.pendingMessages.has(message.request_seq)) { const parentReference = this.pendingMessages.get(message.request_seq)!; this.pendingMessages.delete(message.request_seq); @@ -149,23 +150,3 @@ export class VariableTracker implements AdapterCapabilities { return this.sessions.get(session.id)?.getSizeOfVariable?.(variableName, session); } } - -export function isScopesRequest(message: unknown): message is DebugProtocol.ScopesRequest { - const candidate = message as DebugProtocol.ScopesRequest; - return !!candidate && candidate.command === 'scopes'; -} - -export function isVariableRequest(message: unknown): message is DebugProtocol.VariablesRequest { - const candidate = message as DebugProtocol.VariablesRequest; - return !!candidate && candidate.command === 'variables'; -} - -export function isScopesResponse(message: unknown): message is DebugProtocol.ScopesResponse { - const candidate = message as DebugProtocol.ScopesResponse; - return !!candidate && candidate.command === 'scopes' && Array.isArray(candidate.body.scopes); -} - -export function isVariableResponse(message: unknown): message is DebugProtocol.VariablesResponse { - const candidate = message as DebugProtocol.VariablesResponse; - return !!candidate && candidate.command === 'variables' && Array.isArray(candidate.body.variables); -} diff --git a/src/plugin/memory-provider.ts b/src/plugin/memory-provider.ts index 690cbae..7bacae7 100644 --- a/src/plugin/memory-provider.ts +++ b/src/plugin/memory-provider.ts @@ -20,18 +20,13 @@ import { VariableRange, WrittenMemory } from '../common/memory-range'; import { ReadMemoryResult, SessionContext, WriteMemoryResult } from '../common/messaging'; import { AdapterRegistry } from './adapter-registry/adapter-registry'; import * as manifest from './manifest'; -import { sendRequest } from '../common/debug-requests'; +import { isDebugEvent, isDebugRequest, isDebugResponse, sendRequest } from '../common/debug-requests'; import { stringToBytesMemory } from '../common/memory'; export interface LabeledUint8Array extends Uint8Array { label?: string; } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const isInitializeMessage = (message: any): message is DebugProtocol.InitializeResponse => message.command === 'initialize' && message.type === 'response'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const isStoppedEvent = (message: any): boolean => message.type === 'event' && message.event === 'stopped'; - export class MemoryProvider { public static ReadKey = `${manifest.PACKAGE_NAME}.canRead`; public static WriteKey = `${manifest.PACKAGE_NAME}.canWrite`; @@ -46,7 +41,8 @@ export class MemoryProvider { private _onDidChangeSessionContext = new vscode.EventEmitter(); public readonly onDidChangeSessionContext = this._onDidChangeSessionContext.event; - protected readonly sessions = new Map(); + protected readonly sessionDebugCapabilities = new Map(); + protected readonly sessionClientCapabilities = new Map(); constructor(protected adapterRegistry: AdapterRegistry) { } @@ -70,21 +66,27 @@ export class MemoryProvider { contributedTracker?.onWillStopSession?.(); }, onDidSendMessage: message => { - if (isInitializeMessage(message)) { + if (isDebugResponse('initialize', message)) { // Check for right capabilities in the adapter - this.sessions.set(session.id, message.body); + this.sessionDebugCapabilities.set(session.id, message.body); if (vscode.debug.activeDebugSession?.id === session.id) { this.setContext(session); } - } - if (isStoppedEvent(message)) { + } else if (isDebugEvent('stopped', message)) { this._onDidStopDebug.fire(session); + } else if (isDebugEvent('memory', message)) { + this._onDidWriteMemory.fire(message.body); } contributedTracker?.onDidSendMessage?.(message); }, onError: error => { contributedTracker?.onError?.(error); }, onExit: (code, signal) => { contributedTracker?.onExit?.(code, signal); }, - onWillReceiveMessage: message => { contributedTracker?.onWillReceiveMessage?.(message); } + onWillReceiveMessage: message => { + if (isDebugRequest('initialize', message)) { + this.sessionClientCapabilities.set(session.id, message.arguments); + } + contributedTracker?.onWillReceiveMessage?.(message); + } }); }; @@ -99,11 +101,12 @@ export class MemoryProvider { } protected debugSessionTerminated(session: vscode.DebugSession): void { - this.sessions.delete(session.id); + this.sessionDebugCapabilities.delete(session.id); + this.sessionClientCapabilities.delete(session.id); } protected setContext(session?: vscode.DebugSession): void { - const capabilities = session && this.sessions.get(session.id); + const capabilities = session && this.sessionDebugCapabilities.get(session.id); this._sessionContext = { sessionId: session?.id, canRead: !!capabilities?.supportsReadMemoryRequest, @@ -117,12 +120,20 @@ export class MemoryProvider { /** Returns the session if the capability is present, otherwise throws. */ protected assertCapability(capability: keyof DebugProtocol.Capabilities, action: string): vscode.DebugSession { const session = this.assertActiveSession(action); - if (!this.sessions.get(session.id)?.[capability]) { + if (!this.hasDebugCapabilitiy(session, capability)) { throw new Error(`Cannot ${action}. Session does not have capability ${capability}.`); } return session; } + protected hasDebugCapabilitiy(session: vscode.DebugSession, capability: keyof DebugProtocol.Capabilities): boolean { + return !!this.sessionDebugCapabilities.get(session.id)?.[capability]; + } + + protected hasClientCapabilitiy(session: vscode.DebugSession, capability: keyof DebugProtocol.InitializeRequestArguments): boolean { + return !!this.sessionClientCapabilities.get(session.id)?.[capability]; + } + protected assertActiveSession(action: string): vscode.DebugSession { if (!vscode.debug.activeDebugSession) { throw new Error(`Cannot ${action}. No active debug session.`); @@ -135,11 +146,16 @@ export class MemoryProvider { } public async writeMemory(args: DebugProtocol.WriteMemoryArguments & { count?: number }): Promise { - return sendRequest(this.assertCapability('supportsWriteMemoryRequest', 'write memory'), 'writeMemory', args).then(response => { + const session = this.assertCapability('supportsWriteMemoryRequest', 'write memory'); + return sendRequest(session, 'writeMemory', args).then(response => { const offset = response?.offset ? (args.offset ?? 0) + response.offset : args.offset; // we accept count as an additional argument so we can skip the memory length calculation const count = response?.bytesWritten ?? args.count ?? stringToBytesMemory(args.data).length; - this._onDidWriteMemory.fire({ memoryReference: args.memoryReference, offset, count }); + if (!this.hasClientCapabilitiy(session, 'supportsMemoryEvent')) { + // we only send out a custom event if we don't expect the client to handle the memory event + // since our client is VS Code we can assume that they will always support this but better to be safe + this._onDidWriteMemory.fire({ memoryReference: args.memoryReference, offset, count }); + } return response; }); } diff --git a/src/plugin/memory-storage.ts b/src/plugin/memory-storage.ts index 0bb1d33..84e55a9 100644 --- a/src/plugin/memory-storage.ts +++ b/src/plugin/memory-storage.ts @@ -27,6 +27,7 @@ import * as manifest from './manifest'; import { MemoryProvider } from './memory-provider'; import { ApplyMemoryArguments, ApplyMemoryResult, MemoryOptions, StoreMemoryArguments } from '../common/messaging'; import { isVariablesContext } from './external-views'; +import { isWebviewContext } from '../common/webview-context'; export const StoreCommandType = `${manifest.PACKAGE_NAME}.store-file`; export const ApplyCommandType = `${manifest.PACKAGE_NAME}.apply-file`; @@ -92,6 +93,9 @@ export class MemoryStorage { if (!args) { return {}; } + if (isWebviewContext(args)) { + return { ...args.activeReadArguments }; + } if (isVariablesContext(args)) { try { const variableName = args.variable.evaluateName ?? args.variable.name; @@ -108,7 +112,7 @@ export class MemoryStorage { protected async getStoreMemoryOptions(providedDefault?: Partial): Promise { const memoryReference = await vscode.window.showInputBox({ - title: 'Store Memory as File (1/3)', + title: 'Store Memory to File (1/3)', prompt: 'Start Memory Address', placeHolder: 'Hex address or expression', value: providedDefault?.memoryReference ?? DEFAULT_STORE_OPTIONS.memoryReference, @@ -118,7 +122,7 @@ export class MemoryStorage { return; } const offset = await vscode.window.showInputBox({ - title: 'Store Memory as File (2/3)', + title: 'Store Memory to File (2/3)', prompt: 'Memory Address Offset', placeHolder: 'Positive or negative offset in bytes', value: providedDefault?.offset?.toString() ?? DEFAULT_STORE_OPTIONS.offset.toString(), @@ -128,7 +132,7 @@ export class MemoryStorage { return; } const count = await vscode.window.showInputBox({ - title: 'Store Memory as File (3/3)', + title: 'Store Memory to File (3/3)', prompt: 'Length', placeHolder: 'Number of bytes to read', value: providedDefault?.count?.toString() ?? DEFAULT_STORE_OPTIONS.count.toString(), @@ -188,7 +192,11 @@ export class MemoryStorage { // if we are already given a URI, let's not bother the user and simply use it return { uri: providedDefault.uri }; } - const selectedUris = await vscode.window.showOpenDialog({ title: 'Apply Memory', filters: IntelHEX.DialogFilters }); + const selectedUris = await vscode.window.showOpenDialog({ + title: 'Apply Memory', + filters: IntelHEX.DialogFilters, + defaultUri: vscode.workspace.workspaceFolders?.[0]?.uri + }); if (selectedUris && selectedUris?.length > 0) { return { uri: selectedUris[0] }; } diff --git a/src/webview/components/memory-table.tsx b/src/webview/components/memory-table.tsx index 8533ea7..a57f76c 100644 --- a/src/webview/components/memory-table.tsx +++ b/src/webview/components/memory-table.tsx @@ -442,7 +442,7 @@ export class MemoryTable extends React.PureComponent) => void = e => this.doHandleKeyDown(e); protected doHandleKeyDown(event: KeyboardEvent): void { - if (event.code === 'Enter') { + if (event.key === 'Enter') { const id = event.currentTarget.id as InputId; const value = event.currentTarget.value; diff --git a/src/webview/memory-webview-view.tsx b/src/webview/memory-webview-view.tsx index 7277d2e..679c2b2 100644 --- a/src/webview/memory-webview-view.tsx +++ b/src/webview/memory-webview-view.tsx @@ -53,6 +53,7 @@ import { hoverService, HoverService } from './hovers/hover-service'; import { AddressHover } from './hovers/address-hover'; import { DataHover } from './hovers/data-hover'; import { VariableHover } from './hovers/variable-hover'; +import { debounce } from 'lodash'; export interface MemoryAppState extends MemoryState, MemoryDisplayConfiguration { messageParticipant: WebviewIdMessageParticipant; @@ -149,13 +150,14 @@ class App extends React.Component<{}, MemoryAppState> { hoverService.setMemoryState(this.state); } - protected memoryWritten(writtenMemory: WrittenMemory): void { + // use a slight debounce as the same event may come in short succession + protected memoryWritten = debounce((writtenMemory: WrittenMemory): void => { if (!this.state.memory) { return; } if (this.state.activeReadArguments.memoryReference === writtenMemory.memoryReference) { // catch simple case - this.updateMemoryState(); + this.fetchMemory(); return; } try { @@ -171,7 +173,8 @@ class App extends React.Component<{}, MemoryAppState> { endAddress: this.state.memory.address + BigInt(this.state.memory.bytes.length) }; if (doOverlap(written, shown)) { - this.updateMemoryState(); + this.fetchMemory(); + return; } } catch (error) { // ignore and fall through @@ -179,8 +182,8 @@ class App extends React.Component<{}, MemoryAppState> { // we could try to convert any expression we may have to an address by sending an evaluation request to the DA // but for now we just go with a pessimistic approach: if we are unsure, we refresh the memory - this.updateMemoryState(); - } + this.fetchMemory(); + }, 100); protected sessionContextChanged(sessionContext: SessionContext): void { this.setState({ sessionContext });