Consider PR feedback

- Move evaluation expression for addressOf and sizeOf to adapter
- Make 'toHexStringWithRadixMarker' more flexible
- Shorten validation messages for memory
- Align props and method names with existing method names, no prefixes
- Replace 'MemoryVariableNode' with 'IVariablesContext'
- Add comment on overlap calculation

src/common/debug-requests.ts

@@ -29,11 +29,17 @@ export async function sendRequest<K extends keyof DebugRequestTypes>(session: De
     return session.customRequest(command, args);
 }
 
-export namespace EvaluateExpression {
-    export function sizeOf(expression: string): string {
-        return `sizeof(${expression})`;
-    }
-    export function addressOf(expression: string): string {
-        return `&(${expression})`;
-    }
-};
+export function isDebugVariable(variable: DebugProtocol.Variable | unknown): variable is DebugProtocol.Variable {
+    const assumed = variable ? variable as DebugProtocol.Variable : undefined;
+    return typeof assumed?.name === 'string' && typeof assumed?.value === 'string';
+}
+
+export function isDebugScope(scope: DebugProtocol.Scope | unknown): scope is DebugProtocol.Scope {
+    const assumed = scope ? scope as DebugProtocol.Scope : undefined;
+    return typeof assumed?.name === 'string' && typeof assumed?.variablesReference === 'number';
+}
+
+export function isDebugEvaluateArguments(args: DebugProtocol.EvaluateArguments | unknown): args is DebugProtocol.EvaluateArguments {
+    const assumed = args ? args as DebugProtocol.EvaluateArguments : undefined;
+    return typeof assumed?.expression === 'string';
+}

src/common/memory-range.ts

@@ -91,15 +91,15 @@ export function getRadixMarker(radix: Radix): string {
     return radixPrefixMap[radix];
 }
 
-export function getAddressString(address: bigint, radix: Radix, paddedLength: number = 0): string {
+export function getAddressString(address: bigint | number, radix: Radix, paddedLength: number = 0): string {
     return address.toString(radix).padStart(paddedLength, '0');
 }
 
 export function getAddressLength(padding: number, radix: Radix): number {
     return Math.ceil(padding / Math.log2(radix));
 }
 
-export function toHexStringWithRadixMarker(target: bigint, paddedLength: number = 0): string {
+export function toHexStringWithRadixMarker(target: bigint | number, paddedLength: number = 0): string {
     return `${getRadixMarker(Radix.Hexadecimal)}${getAddressString(target, Radix.Hexadecimal, paddedLength)}`;
 }
 

src/common/memory.ts

@@ -14,16 +14,19 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import type { DebugProtocol } from '@vscode/debugprotocol';
-import { ReadMemoryResult } from './messaging';
+import { ReadMemoryArguments, ReadMemoryResult } from './messaging';
 
 export interface Memory {
     address: bigint;
     bytes: Uint8Array;
 }
 
-export function createMemoryFromRead(result: ReadMemoryResult): Memory {
-    if (!result?.data) { throw new Error('No memory provided!'); }
+export function createMemoryFromRead(result: ReadMemoryResult, request?: ReadMemoryArguments): Memory {
+    if (!result?.data) {
+        const message = request ? `No memory provided for address ${request.memoryReference}`
+            + `, offset ${request.offset} and count ${request.count}!` : 'No memory provided.';
+        throw new Error(message);
+    }
     const address = BigInt(result.address);
     const bytes = stringToBytesMemory(result.data);
     return { bytes, address };
@@ -40,35 +43,19 @@ export function bytesToStringMemory(data: Uint8Array): string {
 export function validateMemoryReference(reference: string): string | undefined {
     const asNumber = Number(reference);
     // we allow an address that is not a number, e.g., an expression, but if it is a number it must be >= 0
-    return !isNaN(asNumber) && asNumber < 0 ? 'Value needs to be >= 0' : undefined;
+    return !isNaN(asNumber) && asNumber < 0 ? 'Value must be >= 0' : undefined;
 }
 
 export function validateOffset(offset: string): string | undefined {
     const asNumber = Number(offset);
-    return isNaN(asNumber) ? 'No number provided' : undefined;
+    return isNaN(asNumber) ? 'Must be number' : undefined;
 }
 
 export function validateCount(count: string): string | undefined {
     const asNumber = Number(count);
     if (isNaN(asNumber)) {
-        return 'No number provided';
+        return 'Must be number';
     } else if (asNumber <= 0) {
-        return 'Value needs to be > 0';
+        return 'Value must be > 0';
     }
 }
-
-export interface MemoryVariable extends DebugProtocol.Variable {
-    memoryReference: string;
-}
-
-export const isMemoryVariable = (variable: unknown): variable is MemoryVariable => !!variable && !!(variable as MemoryVariable).memoryReference;
-
-export interface MemoryVariableNode {
-    variable: MemoryVariable;
-    sessionId: string;
-}
-
-export const isMemoryVariableNode = (node: unknown): node is MemoryVariableNode =>
-    !!node
-    && isMemoryVariable((node as MemoryVariableNode).variable)
-    && typeof (node as MemoryVariableNode).sessionId === 'string';

src/common/messaging.ts

@@ -19,8 +19,8 @@ import type { NotificationType, RequestType } from 'vscode-messenger-common';
 import { MemoryViewSettings } from '../webview/utils/view-types';
 import type { VariableRange, WrittenMemory } from './memory-range';
 import { DebugRequestTypes } from './debug-requests';
-import { MemoryVariableNode } from './memory';
 import { URI } from 'vscode-uri';
+import { VariablesView } from '../plugin/external-views';
 
 // convenience types for easier readability and better semantics
 export type MemoryOptions = Partial<DebugProtocol.ReadMemoryArguments>;
@@ -31,7 +31,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 | MemoryVariableNode;
+export type StoreMemoryArguments = MemoryOptions & { proposedOutputName?: string } | VariablesView.IVariablesContext;
 export type StoreMemoryResult = void;
 
 export type ApplyMemoryArguments = URI | undefined;

src/plugin/adapter-registry/adapter-capabilities.ts

@@ -25,6 +25,10 @@ export interface AdapterCapabilities {
     getVariables?(session: vscode.DebugSession): Promise<VariableRange[]>;
     /** Resolve symbols resident in the memory at the specified range. Will be preferred to {@link getVariables} if present. */
     getResidents?(session: vscode.DebugSession, params: DebugProtocol.ReadMemoryArguments): Promise<VariableRange[]>;
+    /** Resolves the address of a given variable in bytes withthe current context. */
+    getAddressOfVariable?(session: vscode.DebugSession, variableName: string): Promise<string | undefined>;
+    /** Resolves the size of a given variable in bytes within the current context. */
+    getSizeOfVariable?(session: vscode.DebugSession, variableName: string): Promise<bigint | undefined>;
     initializeAdapterTracker?(session: vscode.DebugSession): vscode.DebugAdapterTracker | undefined;
 }
 
@@ -106,9 +110,15 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker {
     protected variableToVariableRange(_variable: DebugProtocol.Variable, _session: vscode.DebugSession): Promise<VariableRange | undefined> {
         throw new Error('To be implemented by derived classes!');
     }
+
+    /** Resolves the address of a given variable in bytes withthe current context. */
+    getAddressOfVariable?(variableName: string, session: vscode.DebugSession): Promise<string | undefined>;
+
+    /** Resolves the size of a given variable in bytes within the current context. */
+    getSizeOfVariable?(variableName: string, session: vscode.DebugSession): Promise<bigint | undefined>;
 }
 
-export class VariableTracker {
+export class VariableTracker implements AdapterCapabilities {
     protected sessions = new Map<string, AdapterVariableTracker>();
     protected types: string[];
 
@@ -127,8 +137,16 @@ export class VariableTracker {
         }
     }
 
-    getVariables(session: vscode.DebugSession): Promise<VariableRange[]> {
-        return Promise.resolve(this.sessions.get(session.id)?.getLocals(session) ?? []);
+    async getVariables(session: vscode.DebugSession): Promise<VariableRange[]> {
+        return this.sessions.get(session.id)?.getLocals(session) ?? [];
+    }
+
+    async getAddressOfVariable(session: vscode.DebugSession, variableName: string): Promise<string | undefined> {
+        return this.sessions.get(session.id)?.getAddressOfVariable?.(variableName, session);
+    }
+
+    async getSizeOfVariable(session: vscode.DebugSession, variableName: string): Promise<bigint | undefined> {
+        return this.sessions.get(session.id)?.getSizeOfVariable?.(variableName, session);
     }
 }
 

src/plugin/adapter-registry/c-tracker.ts

@@ -18,9 +18,19 @@ import * as vscode from 'vscode';
 import { DebugProtocol } from '@vscode/debugprotocol';
 import { AdapterVariableTracker, hexAddress, notADigit } from './adapter-capabilities';
 import { toHexStringWithRadixMarker, VariableRange } from '../../common/memory-range';
-import { sendRequest, EvaluateExpression } from '../../common/debug-requests';
+import { sendRequest } from '../../common/debug-requests';
+
+export namespace CEvaluateExpression {
+    export function sizeOf(expression: string): string {
+        return `sizeof(${expression})`;
+    }
+    export function addressOf(expression: string): string {
+        return `&(${expression})`;
+    }
+};
 
 export class CTracker extends AdapterVariableTracker {
+
     /**
      * Resolves memory location and size using evaluate requests for `$(variable.name)` and `sizeof(variable.name)`
      * Ignores the presence or absence of variable.memoryReference.
@@ -32,15 +42,14 @@ export class CTracker extends AdapterVariableTracker {
             return undefined;
         }
         try {
-            const [addressResponse, sizeResponse] = await Promise.all([
-                sendRequest(session, 'evaluate', { expression: EvaluateExpression.addressOf(variable.name), context: 'watch', frameId: this.currentFrame }),
-                sendRequest(session, 'evaluate', { expression: EvaluateExpression.sizeOf(variable.name), context: 'watch', frameId: this.currentFrame })
-            ]);
-            const addressPart = hexAddress.exec(addressResponse.result);
-            if (!addressPart) { return undefined; }
-            const startAddress = BigInt(addressPart[0]);
-            const endAddress = notADigit.test(sizeResponse.result) ? undefined : startAddress + BigInt(sizeResponse.result);
-            this.logger.debug('Resolved', variable.name, { start: addressPart[0], size: sizeResponse.result });
+            const variableAddress = await this.getAddressOfVariable(variable.name, session);
+            if (!variableAddress) { return undefined; }
+            const variableSize = await this.getSizeOfVariable(variable.name, session);
+            if (!variableSize) { return undefined; }
+
+            const startAddress = BigInt(variableAddress);
+            const endAddress = BigInt(variableAddress) + variableSize;
+            this.logger.debug('Resolved', variable.name, { start: variableAddress, size: variableSize });
             return {
                 name: variable.name,
                 startAddress: toHexStringWithRadixMarker(startAddress),
@@ -52,4 +61,15 @@ export class CTracker extends AdapterVariableTracker {
             return undefined;
         }
     }
+
+    async getAddressOfVariable(variableName: string, session: vscode.DebugSession): Promise<string | undefined> {
+        const response = await sendRequest(session, 'evaluate', { expression: CEvaluateExpression.addressOf(variableName), context: 'watch', frameId: this.currentFrame });
+        const addressPart = hexAddress.exec(response.result);
+        return addressPart ? addressPart[0] : undefined;
+    }
+
+    async getSizeOfVariable(variableName: string, session: vscode.DebugSession): Promise<bigint | undefined> {
+        const response = await sendRequest(session, 'evaluate', { expression: CEvaluateExpression.sizeOf(variableName), context: 'watch', frameId: this.currentFrame });
+        return notADigit.test(response.result) ? undefined : BigInt(response.result);
+    }
 }

src/plugin/external-views.ts

@@ -0,0 +1,32 @@
+/********************************************************************************
+ * Copyright (C) 2024 EclipseSource.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { DebugProtocol } from '@vscode/debugprotocol';
+import { isDebugVariable, isDebugScope, isDebugEvaluateArguments } from '../common/debug-requests';
+
+export namespace VariablesView {
+    // from https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/browser/variablesView.ts
+    export interface IVariablesContext {
+        sessionId: string | undefined;
+        container: DebugProtocol.Variable | DebugProtocol.Scope | DebugProtocol.EvaluateArguments;
+        variable: DebugProtocol.Variable;
+    }
+}
+
+export function isVariablesContext(context: VariablesView.IVariablesContext | unknown): context is VariablesView.IVariablesContext {
+    const assumed = context ? context as VariablesView.IVariablesContext : undefined;
+    return isDebugVariable(assumed?.variable) && (isDebugVariable(assumed?.container) || isDebugScope(assumed?.container) || isDebugEvaluateArguments(assumed?.container));
+}

src/plugin/memory-provider.ts

@@ -132,14 +132,22 @@ export class MemoryProvider {
         });
     }
 
-    public async evaluate(args: DebugProtocol.EvaluateArguments): Promise<DebugProtocol.EvaluateResponse['body']> {
-        return sendRequest(this.assertActiveSession('evaluate'), 'evaluate', args);
-    }
-
     public async getVariables(variableArguments: DebugProtocol.ReadMemoryArguments): Promise<VariableRange[]> {
         const session = this.assertActiveSession('get variables');
         const handler = this.adapterRegistry?.getHandlerForSession(session.type);
         if (handler?.getResidents) { return handler.getResidents(session, variableArguments); }
         return handler?.getVariables?.(session) ?? [];
     }
+
+    public async getAddressOfVariable(variableName: string): Promise<string | undefined> {
+        const session = this.assertActiveSession('get address of variable');
+        const handler = this.adapterRegistry?.getHandlerForSession(session.type);
+        return handler?.getAddressOfVariable?.(session, variableName);
+    }
+
+    public async getSizeOfVariable(variableName: string): Promise<bigint | undefined> {
+        const session = this.assertActiveSession('get address of variable');
+        const handler = this.adapterRegistry?.getHandlerForSession(session.type);
+        return handler?.getSizeOfVariable?.(session, variableName);
+    }
 }

src/plugin/memory-storage.ts

@@ -20,24 +20,25 @@ import { URI, Utils } from 'vscode-uri';
 import { IntelHEX } from '../common/intel-hex';
 import {
     bytesToStringMemory, createMemoryFromRead,
-    isMemoryVariableNode, validateCount, validateMemoryReference, validateOffset
+    validateCount, validateMemoryReference, validateOffset
 } from '../common/memory';
 import { toHexStringWithRadixMarker } from '../common/memory-range';
 import * as manifest from './manifest';
 import { MemoryProvider } from './memory-provider';
-import { EvaluateExpression } from '../common/debug-requests';
 import { ApplyMemoryArguments, ApplyMemoryResult, MemoryOptions, StoreMemoryArguments } from '../common/messaging';
+import { isVariablesContext } from './external-views';
 
 export const StoreCommandType = `${manifest.PACKAGE_NAME}.store-file`;
 export const ApplyCommandType = `${manifest.PACKAGE_NAME}.apply-file`;
 
 const VALID_FILE_NAME_CHARS = /[^a-zA-Z0-9 _-]/g;
 
 type StoreMemoryOptions = Required<MemoryOptions> & {
+    proposedOutputName?: string,
     outputFile: vscode.Uri;
 };
 
-const DEFAULT_STORE_OPTIONS: Omit<StoreMemoryOptions, 'outputFile'> = {
+const DEFAULT_STORE_OPTIONS: Omit<StoreMemoryOptions, 'outputFile' | 'proposedOutputName'> = {
     memoryReference: toHexStringWithRadixMarker(0n, 8),
     offset: 0,
     count: 256
@@ -91,12 +92,12 @@ export class MemoryStorage {
         if (!args) {
             return {};
         }
-        if (isMemoryVariableNode(args)) {
+        if (isVariablesContext(args)) {
             try {
                 const variableName = args.variable.evaluateName ?? args.variable.name;
-                const { result } = await this.memoryProvider.evaluate({ expression: EvaluateExpression.sizeOf(variableName), context: 'watch' });
-                const count = validateCount(result) === undefined ? Number(result) : undefined;
-                return { count, memoryReference: EvaluateExpression.addressOf(variableName), offset: 0 };
+                const count = await this.memoryProvider.getSizeOfVariable(variableName);
+                const memoryReference = args.variable.memoryReference ?? await this.memoryProvider.getAddressOfVariable(variableName);
+                return { count: Number(count), memoryReference, offset: 0, proposedOutputName: variableName };
             } catch (error) {
                 // ignore, we are just using them as default values
                 return { memoryReference: args.variable.memoryReference, offset: 0 };
@@ -137,7 +138,9 @@ export class MemoryStorage {
             return;
         }
         const workspaceUri = vscode.workspace.workspaceFolders?.[0]?.uri;
-        const defaultUri = workspaceUri ? Utils.joinPath(workspaceUri, memoryReference.replace(VALID_FILE_NAME_CHARS, '') + '_' + count) : workspaceUri;
+        const proposedName = providedDefault?.proposedOutputName ?? memoryReference + '_' + count;
+        const validName = proposedName.replace(VALID_FILE_NAME_CHARS, '');
+        const defaultUri = workspaceUri ? Utils.joinPath(workspaceUri, validName) : workspaceUri;
         const saveFile = await vscode.window.showSaveDialog({ title: 'Store Memory', defaultUri, filters: IntelHEX.DialogFilters });
         if (!saveFile) {
             return;
@@ -159,7 +162,7 @@ export class MemoryStorage {
             let memoryReference: string | undefined;
             let count: number | undefined;
             for (const [address, memory] of memoryMap) {
-                memoryReference = toHexStringWithRadixMarker(BigInt(address));
+                memoryReference = toHexStringWithRadixMarker(address);
                 count = memory.length;
                 const data = bytesToStringMemory(memory);
                 await this.memoryProvider.writeMemory({ memoryReference, data, count });