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
eclipse-cdt-cloud · Mar 12, 2024 · e945093 · e945093
1 parent 28b4eb8
commit e945093
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 29 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,20 @@ 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']]
     'writeMemory': [DebugProtocol.WriteMemoryArguments, DebugProtocol.WriteMemoryResponse['body']]
 }
 
+export interface DebugEvents {
+    'memory': DebugProtocol.MemoryEvent,
+    'stopped': DebugProtocol.StoppedEvent
+}
+
+export type DebugRequest<T> = DebugProtocol.Request & { body: T };
+export type DebugResponse<T> = DebugProtocol.Response & { body: T };
+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 +53,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<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<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/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.body);
+                    }
+                    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
@@ -428,7 +428,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
@@ -69,6 +69,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/memory-webview-view.tsx b/src/webview/memory-webview-view.tsx
@@ -49,6 +49,7 @@ import { decorationService } from './decorations/decoration-service';
 import { Decoration, MemoryDisplayConfiguration, MemoryState } from './utils/view-types';
 import { variableDecorator } from './variables/variable-decorations';
 import { messenger } from './view-messenger';
+import { debounce } from 'lodash';
 
 export interface MemoryAppState extends MemoryState, MemoryDisplayConfiguration {
     messageParticipant: WebviewIdMessageParticipant;
@@ -139,13 +140,14 @@ class App extends React.Component<{}, MemoryAppState> {
         }
     }
 
-    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 {
@@ -161,16 +163,17 @@ 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
         }
 
         // 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 });