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
eclipse-cdt-cloud · Mar 12, 2024 · 4b0c003 · 4b0c003
1 parent 53248da
commit 4b0c003
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 54 deletions.
diff --git 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
@@ -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<COMMAND, ARGS> = Omit<DebugProtocol.Request, 'command' | 'arguments'> & { command: COMMAND, arguments: ARGS };
+export type DebugResponse<COMMAND, BODY> = Omit<DebugProtocol.Response, 'command' | 'body'> & { command: COMMAND, body: BODY };
+export type DebugEvent<T> = DebugProtocol.Event & { body: T };
+
 export async function sendRequest<K extends keyof DebugRequestTypes>(session: DebugSession,
     command: K, args: DebugRequestTypes[K][0]): Promise<DebugRequestTypes[K][1]> {
     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<K extends keyof DebugRequestTypes>(command: K, message: unknown): message is DebugRequest<K, DebugRequestTypes[K][0]> {
+    const assumed = message ? message as DebugProtocol.Request : undefined;
+    return !!assumed && assumed.type === 'request' && assumed.command === command;
+}
+
+export function isDebugResponse<K extends keyof DebugRequestTypes>(command: K, message: unknown): message is DebugResponse<K, DebugRequestTypes[K][1]> {
+    const assumed = message ? message as DebugProtocol.Response : undefined;
+    return !!assumed && assumed.type === 'response' && assumed.command === command;
+}
+
+export function isDebugEvent<K extends keyof DebugEvents>(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
@@ -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<DebugProtocol.ReadMemoryArguments>;
@@ -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
@@ -16,13 +16,15 @@
 
 import { WebviewIdMessageParticipant } from 'vscode-messenger-common';
 import { VariableMetadata } from './memory-range';
+import { ReadMemoryArguments } from './messaging';
 
 export interface WebviewContext {
     messageParticipant: WebviewIdMessageParticipant,
     webviewSection: string,
     showAsciiColumn: boolean
     showVariablesColumn: boolean,
     showRadixPrefix: boolean,
+    activeReadArguments: Required<ReadMemoryArguments>
 }
 
 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
@@ -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
@@ -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<SessionContext>();
     public readonly onDidChangeSessionContext = this._onDidChangeSessionContext.event;
 
-    protected readonly sessions = new Map<string, DebugProtocol.Capabilities | undefined>();
+    protected readonly sessionDebugCapabilities = new Map<string, DebugProtocol.Capabilities | undefined>();
+    protected readonly sessionClientCapabilities = new Map<string, DebugProtocol.InitializeRequestArguments | undefined>();
 
     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<WriteMemoryResult> {
-        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
@@ -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<StoreMemoryOptions>): Promise<StoreMemoryOptions | undefined> {
         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
@@ -442,7 +442,7 @@ export class MemoryTable extends React.PureComponent<MemoryTableProps, MemoryTab
         }
 
         /*
-         * Before opening a context menu for a table cell target we dynamically add the `value` property to the <vscode-data-context.
+         * Before opening a context menu for a table cell target we dynamically add the value property to the vscode-data-context.
          * Using this dynamic approach ensures the the cell value is also set correctly when the menu was opened on empty cell space.
          */
         const value = cell.textContent;

diff --git a/src/webview/components/memory-widget.tsx b/src/webview/components/memory-widget.tsx
@@ -71,6 +71,7 @@ export class MemoryWidget extends React.Component<MemoryWidgetProps, MemoryWidge
             showRadixPrefix: this.props.showRadixPrefix,
             showAsciiColumn: visibleColumns.includes('ascii'),
             showVariablesColumn: visibleColumns.includes('variables'),
+            activeReadArguments: this.props.activeReadArguments
         });
 
     }

diff --git a/src/webview/components/options-widget.tsx b/src/webview/components/options-widget.tsx
@@ -417,7 +417,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
 
     protected handleKeyDown: (event: KeyboardEvent<HTMLInputElement>) => void = e => this.doHandleKeyDown(e);
     protected doHandleKeyDown(event: KeyboardEvent<HTMLInputElement>): void {
-        if (event.code === 'Enter') {
+        if (event.key === 'Enter') {
             const id = event.currentTarget.id as InputId;
             const value = event.currentTarget.value;