From 83c51b5fc421391a1a50d6da1abcc67882cbf1f9 Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Wed, 13 Mar 2024 09:48:34 +0100 Subject: [PATCH] Add further PR feedback - Use short (PascalCase) names for generic arguments - Adapt copyright header in debug-requests - Remove optional 'count' parameter in messaging and memory-provider - Remove caching of session context and simply re-create if necessary - Execute variable queries in parallel instead of sequentially - Fix typo in comment --- src/common/debug-requests.ts | 6 ++-- src/common/messaging.ts | 2 +- .../adapter-registry/adapter-capabilities.ts | 2 +- src/plugin/adapter-registry/c-tracker.ts | 9 +++--- src/plugin/memory-provider.ts | 31 +++++++++---------- src/plugin/memory-storage.ts | 2 +- src/plugin/memory-webview-main.ts | 6 ++-- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/common/debug-requests.ts b/src/common/debug-requests.ts index 1e2f54f..dd3d78a 100644 --- a/src/common/debug-requests.ts +++ b/src/common/debug-requests.ts @@ -1,5 +1,5 @@ /******************************************************************************** - * Copyright (C) 2018 Red Hat, Inc. and others. + * 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 @@ -32,8 +32,8 @@ export interface DebugEvents { 'stopped': DebugProtocol.StoppedEvent } -export type DebugRequest = Omit & { command: COMMAND, arguments: ARGS }; -export type DebugResponse = Omit & { command: COMMAND, body: BODY }; +export type DebugRequest = Omit & { command: C, arguments: A }; +export type DebugResponse = Omit & { command: C, body: B }; export type DebugEvent = DebugProtocol.Event & { body: T }; export async function sendRequest(session: DebugSession, diff --git a/src/common/messaging.ts b/src/common/messaging.ts index 2b71c6f..0fe2e43 100644 --- a/src/common/messaging.ts +++ b/src/common/messaging.ts @@ -29,7 +29,7 @@ export type MemoryOptions = Partial; export type ReadMemoryArguments = DebugRequestTypes['readMemory'][0]; export type ReadMemoryResult = DebugRequestTypes['readMemory'][1]; -export type WriteMemoryArguments = DebugRequestTypes['writeMemory'][0] & { count?: number }; +export type WriteMemoryArguments = DebugRequestTypes['writeMemory'][0]; export type WriteMemoryResult = DebugRequestTypes['writeMemory'][1]; export type StoreMemoryArguments = MemoryOptions & { proposedOutputName?: string } | VariablesView.IVariablesContext | WebviewContext; diff --git a/src/plugin/adapter-registry/adapter-capabilities.ts b/src/plugin/adapter-registry/adapter-capabilities.ts index 6d029fe..b8e1940 100644 --- a/src/plugin/adapter-registry/adapter-capabilities.ts +++ b/src/plugin/adapter-registry/adapter-capabilities.ts @@ -112,7 +112,7 @@ export class AdapterVariableTracker implements vscode.DebugAdapterTracker { throw new Error('To be implemented by derived classes!'); } - /** Resolves the address of a given variable in bytes withthe current context. */ + /** Resolves the address of a given variable in bytes within the current context. */ getAddressOfVariable?(variableName: string, session: vscode.DebugSession): Promise; /** Resolves the size of a given variable in bytes within the current context. */ diff --git a/src/plugin/adapter-registry/c-tracker.ts b/src/plugin/adapter-registry/c-tracker.ts index adc2817..8c3caa8 100644 --- a/src/plugin/adapter-registry/c-tracker.ts +++ b/src/plugin/adapter-registry/c-tracker.ts @@ -42,13 +42,14 @@ export class CTracker extends AdapterVariableTracker { return undefined; } try { - const variableAddress = await this.getAddressOfVariable(variable.name, session); + const [variableAddress, variableSize] = await Promise.all([ + variable.memoryReference && hexAddress.test(variable.memoryReference) ? variable.memoryReference : this.getAddressOfVariable(variable.name, session), + this.getSizeOfVariable(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; + const endAddress = variableSize !== undefined ? startAddress + variableSize : undefined; this.logger.debug('Resolved', variable.name, { start: variableAddress, size: variableSize }); return { name: variable.name, diff --git a/src/plugin/memory-provider.ts b/src/plugin/memory-provider.ts index 7bacae7..14640e6 100644 --- a/src/plugin/memory-provider.ts +++ b/src/plugin/memory-provider.ts @@ -37,7 +37,6 @@ export class MemoryProvider { private _onDidWriteMemory = new vscode.EventEmitter(); public readonly onDidWriteMemory = this._onDidWriteMemory.event; - private _sessionContext: SessionContext = { canRead: false, canWrite: false }; private _onDidChangeSessionContext = new vscode.EventEmitter(); public readonly onDidChangeSessionContext = this._onDidChangeSessionContext.event; @@ -47,10 +46,6 @@ export class MemoryProvider { constructor(protected adapterRegistry: AdapterRegistry) { } - get sessionContext(): SessionContext { - return this._sessionContext; - } - public activate(context: vscode.ExtensionContext): void { const createDebugAdapterTracker = (session: vscode.DebugSession): Required => { const handlerForSession = this.adapterRegistry.getHandlerForSession(session.type); @@ -105,16 +100,21 @@ export class MemoryProvider { this.sessionClientCapabilities.delete(session.id); } - protected setContext(session?: vscode.DebugSession): void { - const capabilities = session && this.sessionDebugCapabilities.get(session.id); - this._sessionContext = { - sessionId: session?.id, + createContext(session = vscode.debug.activeDebugSession): SessionContext { + const sessionId = session?.id; + const capabilities = sessionId ? this.sessionDebugCapabilities.get(sessionId) : undefined; + return { + sessionId, canRead: !!capabilities?.supportsReadMemoryRequest, canWrite: !!capabilities?.supportsWriteMemoryRequest }; - vscode.commands.executeCommand('setContext', MemoryProvider.ReadKey, this.sessionContext.canRead); - vscode.commands.executeCommand('setContext', MemoryProvider.WriteKey, this.sessionContext.canWrite); - this._onDidChangeSessionContext.fire(this.sessionContext); + } + + protected setContext(session?: vscode.DebugSession): void { + const newContext = this.createContext(session); + vscode.commands.executeCommand('setContext', MemoryProvider.ReadKey, newContext.canRead); + vscode.commands.executeCommand('setContext', MemoryProvider.WriteKey, newContext.canWrite); + this._onDidChangeSessionContext.fire(newContext); } /** Returns the session if the capability is present, otherwise throws. */ @@ -145,15 +145,14 @@ export class MemoryProvider { return sendRequest(this.assertCapability('supportsReadMemoryRequest', 'read memory'), 'readMemory', args); } - public async writeMemory(args: DebugProtocol.WriteMemoryArguments & { count?: number }): Promise { + public async writeMemory(args: DebugProtocol.WriteMemoryArguments): Promise { 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; 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 + const offset = response?.offset ? (args.offset ?? 0) + response.offset : args.offset; + const count = response?.bytesWritten ?? stringToBytesMemory(args.data).length; 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 84e55a9..65915bb 100644 --- a/src/plugin/memory-storage.ts +++ b/src/plugin/memory-storage.ts @@ -169,7 +169,7 @@ export class MemoryStorage { memoryReference = toHexStringWithRadixMarker(address); count = memory.length; const data = bytesToStringMemory(memory); - await this.memoryProvider.writeMemory({ memoryReference, data, count }); + await this.memoryProvider.writeMemory({ memoryReference, data }); } await vscode.window.showInformationMessage(`Memory from '${vscode.workspace.asRelativePath(options.uri)}' applied.`); return { memoryReference, count, offset: 0 }; diff --git a/src/plugin/memory-webview-main.ts b/src/plugin/memory-webview-main.ts index 3f4d8cf..6462148 100644 --- a/src/plugin/memory-webview-main.ts +++ b/src/plugin/memory-webview-main.ts @@ -206,7 +206,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider { const disposables = [ this.messenger.onNotification(readyType, () => { this.setInitialSettings(participant, panel.title); - this.setSessionContext(participant, this.memoryProvider.sessionContext); + this.setSessionContext(participant, this.memoryProvider.createContext()); this.refresh(participant, options); }, { sender: participant }), this.messenger.onRequest(setOptionsType, o => { @@ -316,7 +316,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider { protected async storeMemory(storeArguments: StoreMemoryArguments): Promise { // Even if we disable the command in VS Code through enablement or when condition, programmatic execution is still possible. // However, we want to fail early in case the user tries to execute a disabled command - if (!this.memoryProvider.sessionContext.canRead) { + if (!this.memoryProvider.createContext().canRead) { throw new Error('Cannot read memory, no valid debug session.'); } return vscode.commands.executeCommand(StoreCommandType, storeArguments); @@ -325,7 +325,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider { protected async applyMemory(): Promise { // Even if we disable the command in VS Code through enablement or when condition, programmatic execution is still possible. // However, we want to fail early in case the user tries to execute a disabled command - if (!this.memoryProvider.sessionContext.canWrite) { + if (!this.memoryProvider.createContext().canWrite) { throw new Error('Cannot write memory, no valid debug session.'); } return vscode.commands.executeCommand(ApplyCommandType);