Skip to content

Commit

Permalink
PR Feedback: Disable store/apply memory buttons if not possible
Browse files Browse the repository at this point in the history 
- VS Code always allows to execute a command programmatically
-- There is no way to determine whether a command is enabled or not

- Forward current session context to memory inspector
  • Loading branch information
@martin-fleck-at
martin-fleck-at committed Mar 8, 2024
1 parent 40522ae commit 733088f
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 21 deletions.
7 changes: 7 additions & 0 deletions src/common/messaging.ts
View file
Expand Up @@ -37,12 +37,19 @@ export type StoreMemoryResult = void;
export type ApplyMemoryArguments = URI | undefined;
export type ApplyMemoryResult = MemoryOptions;

export interface SessionContext {
sessionId?: string;
canRead: boolean;
canWrite: boolean;
}

// Notifications
export const readyType: NotificationType<void> = { method: 'ready' };
export const setMemoryViewSettingsType: NotificationType<Partial<MemoryViewSettings>> = { method: 'setMemoryViewSettings' };
export const resetMemoryViewSettingsType: NotificationType<void> = { method: 'resetMemoryViewSettings' };
export const setTitleType: NotificationType<string> = { method: 'setTitle' };
export const memoryWrittenType: NotificationType<WrittenMemory> = { method: 'memoryWritten' };
export const sessionContextChangedType: NotificationType<SessionContext> = { method: 'sessionContextChanged' };

// Requests
export const setOptionsType: RequestType<MemoryOptions, void> = { method: 'setOptions' };
Expand Down
30 changes: 21 additions & 9 deletions src/plugin/memory-provider.ts
View file
Expand Up @@ -17,7 +17,7 @@
import { DebugProtocol } from '@vscode/debugprotocol';
import * as vscode from 'vscode';
import { VariableRange, WrittenMemory } from '../common/memory-range';
import { ReadMemoryResult, WriteMemoryResult } from '../common/messaging';
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';
Expand All @@ -42,11 +42,19 @@ export class MemoryProvider {
private _onDidWriteMemory = new vscode.EventEmitter<WrittenMemory>();
public readonly onDidWriteMemory = this._onDidWriteMemory.event;

private _sessionContext: SessionContext = { canRead: false, canWrite: false };
private _onDidChangeSessionContext = new vscode.EventEmitter<SessionContext>();
public readonly onDidChangeSessionContext = this._onDidChangeSessionContext.event;

protected readonly sessions = new Map<string, DebugProtocol.Capabilities | undefined>();

constructor(protected adapterRegistry: AdapterRegistry) {
}

get sessionContext(): SessionContext {
return this._sessionContext;
}

public activate(context: vscode.ExtensionContext): void {
const createDebugAdapterTracker = (session: vscode.DebugSession): Required<vscode.DebugAdapterTracker> => {
const handlerForSession = this.adapterRegistry.getHandlerForSession(session.type);
Expand All @@ -66,7 +74,7 @@ export class MemoryProvider {
// Check for right capabilities in the adapter
this.sessions.set(session.id, message.body);
if (vscode.debug.activeDebugSession?.id === session.id) {
this.setContext(message.body);
this.setContext(session);
}
}
if (isStoppedEvent(message)) {
Expand All @@ -82,10 +90,7 @@ export class MemoryProvider {

context.subscriptions.push(
vscode.debug.registerDebugAdapterTrackerFactory('*', { createDebugAdapterTracker }),
vscode.debug.onDidChangeActiveDebugSession(session => {
const capabilities = session && this.sessions.get(session.id);
this.setContext(capabilities);
})
vscode.debug.onDidChangeActiveDebugSession(session => this.setContext(session))
);
}

Expand All @@ -97,9 +102,16 @@ export class MemoryProvider {
this.sessions.delete(session.id);
}

protected setContext(capabilities?: DebugProtocol.Capabilities): void {
vscode.commands.executeCommand('setContext', MemoryProvider.ReadKey, !!capabilities?.supportsReadMemoryRequest);
vscode.commands.executeCommand('setContext', MemoryProvider.WriteKey, !!capabilities?.supportsWriteMemoryRequest);
protected setContext(session?: vscode.DebugSession): void {
const capabilities = session && this.sessions.get(session.id);
this._sessionContext = {
sessionId: session?.id,
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);
}

/** Returns the session if the capability is present, otherwise throws. */
Expand Down
18 changes: 18 additions & 0 deletions src/plugin/memory-webview-main.ts
View file
Expand Up @@ -22,6 +22,7 @@ import {
MemoryOptions,
ReadMemoryArguments,
ReadMemoryResult,
SessionContext,
StoreMemoryArguments,
WriteMemoryArguments,
WriteMemoryResult,
Expand All @@ -32,6 +33,7 @@ import {
readMemoryType,
readyType,
resetMemoryViewSettingsType,
sessionContextChangedType,
setMemoryViewSettingsType,
setOptionsType,
setTitleType,
Expand Down Expand Up @@ -182,6 +184,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.refresh(participant, options);
}, { sender: participant }),
this.messenger.onRequest(setOptionsType, o => {
Expand All @@ -201,6 +204,7 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
this.refresh(participant);
}
}),
this.memoryProvider.onDidChangeSessionContext(context => this.setSessionContext(participant, context)),
this.memoryProvider.onDidWriteMemory(writtenMemory => this.messenger.sendNotification(memoryWrittenType, participant, writtenMemory))
];
panel.onDidChangeViewState(newState => {
Expand All @@ -219,6 +223,10 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
this.messenger.sendNotification(setMemoryViewSettingsType, webviewParticipant, this.getMemoryViewSettings(title));
}

protected setSessionContext(webviewParticipant: WebviewIdMessageParticipant, context: SessionContext): void {
this.messenger.sendNotification(sessionContextChangedType, webviewParticipant, context);
}

protected getMemoryViewSettings(title: string): MemoryViewSettings {
const memoryInspectorConfiguration = vscode.workspace.getConfiguration(manifest.PACKAGE_NAME);
const bytesPerWord = memoryInspectorConfiguration.get<number>(manifest.CONFIG_BYTES_PER_WORD, manifest.DEFAULT_BYTES_PER_WORD);
Expand Down Expand Up @@ -261,10 +269,20 @@ export class MemoryWebview implements vscode.CustomReadonlyEditorProvider {
}

protected async storeMemory(storeArguments: StoreMemoryArguments): Promise<void> {
// 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) {
throw new Error('Cannot read memory, no valid debug session.');
}
return vscode.commands.executeCommand(StoreCommandType, storeArguments);
}

protected async applyMemory(): Promise<MemoryOptions> {
// 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) {
throw new Error('Cannot write memory, no valid debug session.');
}
return vscode.commands.executeCommand(ApplyCommandType);
}

Expand Down
4 changes: 3 additions & 1 deletion src/webview/components/memory-widget.tsx
View file
Expand Up @@ -19,10 +19,11 @@ import { ColumnStatus } from '../columns/column-contribution-service';
import { Decoration, MemoryDisplayConfiguration, MemoryState } from '../utils/view-types';
import { MemoryTable } from './memory-table';
import { OptionsWidget } from './options-widget';
import { MemoryOptions, ReadMemoryArguments } from '../../common/messaging';
import { MemoryOptions, ReadMemoryArguments, SessionContext } from '../../common/messaging';
import { Memory } from '../../common/memory';

interface MemoryWidgetProps extends MemoryDisplayConfiguration {
sessionContext: SessionContext;
configuredReadArguments: Required<ReadMemoryArguments>;
activeReadArguments: Required<ReadMemoryArguments>;
memory?: Memory;
Expand Down Expand Up @@ -58,6 +59,7 @@ export class MemoryWidget extends React.Component<MemoryWidgetProps, MemoryWidge
override render(): React.ReactNode {
return (<div className='flex flex-column h-full'>
<OptionsWidget
sessionContext={this.props.sessionContext}
title={this.props.title}
updateTitle={this.props.updateTitle}
columnOptions={this.props.columns}
Expand Down
22 changes: 13 additions & 9 deletions src/webview/components/options-widget.tsx
View file
Expand Up @@ -27,12 +27,13 @@ import { MultiSelectWithLabel } from './multi-select';
import { CONFIG_BYTES_PER_WORD_CHOICES, CONFIG_GROUPS_PER_ROW_CHOICES, CONFIG_WORDS_PER_GROUP_CHOICES } from '../../plugin/manifest';
import { tryToNumber } from '../../common/typescript';
import { Checkbox } from 'primereact/checkbox';
import { Endianness } from '../../common/memory-range';
import { MemoryOptions, ReadMemoryArguments } from '../../common/messaging';
import { MemoryOptions, ReadMemoryArguments, SessionContext } from '../../common/messaging';
import { validateMemoryReference, validateOffset, validateCount } from '../../common/memory';
import { Endianness } from '../../common/memory-range';

export interface OptionsWidgetProps
extends Omit<TableRenderOptions, 'scrollingBehavior' | 'effectiveAddressLength'> {
sessionContext: SessionContext;
configuredReadArguments: Required<ReadMemoryArguments>;
activeReadArguments: Required<ReadMemoryArguments>;
title: string;
Expand Down Expand Up @@ -126,6 +127,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
this.formConfig.initialValues = this.optionsFormValues;
const isLabelEditing = this.state.isTitleEditing;
const isFrozen = this.props.isFrozen;
const readDisabled = isFrozen || !this.props.sessionContext.canRead;
const freezeContentToggleTitle = isFrozen ? 'Unfreeze Memory View' : 'Freeze Memory View';
const activeMemoryReadArgumentHint = (userValue: string | number, memoryValue: string | number): ReactNode | undefined => {
if (userValue !== memoryValue) {
Expand Down Expand Up @@ -173,6 +175,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
className='store-file-button'
icon='codicon codicon-save'
onClick={this.props.storeMemory}
disabled={!this.props.sessionContext.canWrite}
title='Store Memory as File'
aria-label='Store Memory as File'
rounded
Expand All @@ -183,6 +186,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
className='apply-file-button'
icon='codicon codicon-folder-opened'
onClick={this.props.applyMemory}
disabled={!this.props.sessionContext.canRead}
title='Apply Memory from File'
aria-label='Apply Memory from File'
rounded
Expand All @@ -196,7 +200,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{formik => (
<form onSubmit={formik.handleSubmit} className='form-options'>
<span className={'pm-top-label form-textfield form-texfield-long'}>
<label htmlFor={InputId.Address} className={`p-inputtext-label ${isFrozen ? 'p-disabled' : ''}`} >
<label htmlFor={InputId.Address} className={`p-inputtext-label ${readDisabled ? 'p-disabled' : ''}`} >
Address
</label>
<InputText
Expand All @@ -205,7 +209,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{...formik.getFieldProps('address')}
onKeyDown={this.handleKeyDown}
onBlur={ev => this.doHandleBlur(ev, formik)}
disabled={isFrozen}
disabled={readDisabled}
/>
{formik.errors.address ?
(<small className='p-invalid'>
Expand All @@ -215,7 +219,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{activeMemoryReadArgumentHint(this.props.configuredReadArguments.memoryReference, this.props.activeReadArguments.memoryReference)}
</span>
<span className='pm-top-label form-textfield'>
<label htmlFor={InputId.Offset} className={`p-inputtext-label ${isFrozen ? 'p-disabled' : ''}`}>
<label htmlFor={InputId.Offset} className={`p-inputtext-label ${readDisabled ? 'p-disabled' : ''}`}>
Offset
</label>
<InputText
Expand All @@ -224,7 +228,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{...formik.getFieldProps('offset')}
onKeyDown={this.handleKeyDown}
onBlur={ev => this.doHandleBlur(ev, formik)}
disabled={isFrozen}
disabled={readDisabled}
/>
{formik.errors.offset ?
(<small className='p-invalid'>
Expand All @@ -234,7 +238,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{activeMemoryReadArgumentHint(this.props.configuredReadArguments.offset, this.props.activeReadArguments.offset)}
</span>
<span className='pm-top-label form-textfield'>
<label htmlFor={InputId.Length} className={`p-inputtext-label ${isFrozen ? 'p-disabled' : ''}`}>
<label htmlFor={InputId.Length} className={`p-inputtext-label ${readDisabled ? 'p-disabled' : ''}`}>
Length
</label>
<InputText
Expand All @@ -243,7 +247,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
{...formik.getFieldProps('count')}
onKeyDown={this.handleKeyDown}
onBlur={ev => this.doHandleBlur(ev, formik)}
disabled={isFrozen}
disabled={readDisabled}
/>
{formik.errors.count ?
(<small className='p-invalid'>
Expand All @@ -252,7 +256,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
: undefined}
{activeMemoryReadArgumentHint(this.props.configuredReadArguments.count, this.props.activeReadArguments.count)}
</span>
<Button type='submit' disabled={!formik.isValid || isFrozen}>
<Button type='submit' disabled={!formik.isValid || readDisabled}>
Go
</Button>
</form>
Expand Down
20 changes: 18 additions & 2 deletions src/webview/memory-webview-view.tsx
View file
Expand Up @@ -30,6 +30,8 @@ import {
MemoryOptions,
memoryWrittenType,
ReadMemoryArguments,
sessionContextChangedType,
SessionContext,
} from '../common/messaging';
import { Decoration, MemoryDisplayConfiguration, MemoryState } from './utils/view-types';
import { MemoryWidget } from './components/memory-widget';
Expand All @@ -48,12 +50,18 @@ import { Endianness } from '../common/memory-range';

export interface MemoryAppState extends MemoryState, MemoryDisplayConfiguration {
title: string;
sessionContext: SessionContext;
effectiveAddressLength: number;
decorations: Decoration[];
columns: ColumnStatus[];
isFrozen: boolean;
}

const DEFAULT_SESSION_CONTEXT: SessionContext = {
canRead: false,
canWrite: false
};

const MEMORY_DISPLAY_CONFIGURATION_DEFAULTS: MemoryDisplayConfiguration = {
bytesPerWord: 1,
wordsPerGroup: 1,
Expand All @@ -64,6 +72,7 @@ const MEMORY_DISPLAY_CONFIGURATION_DEFAULTS: MemoryDisplayConfiguration = {
addressRadix: 16,
showRadixPrefix: true,
};

const DEFAULT_READ_ARGUMENTS: Required<ReadMemoryArguments> = {
memoryReference: '',
offset: 0,
Expand All @@ -81,6 +90,7 @@ class App extends React.Component<{}, MemoryAppState> {
decorationService.register(variableDecorator);
this.state = {
title: 'Memory',
sessionContext: DEFAULT_SESSION_CONTEXT,
memory: undefined,
effectiveAddressLength: 0,
configuredReadArguments: DEFAULT_READ_ARGUMENTS,
Expand All @@ -95,7 +105,8 @@ class App extends React.Component<{}, MemoryAppState> {

public componentDidMount(): void {
messenger.onRequest(setOptionsType, options => this.setOptions(options));
messenger.onNotification(memoryWrittenType, writtenMemory => this.handleWrittenMemory(writtenMemory));
messenger.onNotification(memoryWrittenType, writtenMemory => this.memoryWritten(writtenMemory));
messenger.onNotification(sessionContextChangedType, sessionContext => this.sessionContextChanged(sessionContext));
messenger.onNotification(setMemoryViewSettingsType, config => {
for (const column of columnContributionService.getColumns()) {
const id = column.contribution.id;
Expand All @@ -119,7 +130,7 @@ class App extends React.Component<{}, MemoryAppState> {
}
}

protected handleWrittenMemory(writtenMemory: WrittenMemory): void {
protected memoryWritten(writtenMemory: WrittenMemory): void {
if (!this.state.memory) {
return;
}
Expand Down Expand Up @@ -152,9 +163,14 @@ class App extends React.Component<{}, MemoryAppState> {
this.updateMemoryState();
}

protected sessionContextChanged(sessionContext: SessionContext): void {
this.setState({ sessionContext });
}

public render(): React.ReactNode {
return <PrimeReactProvider>
<MemoryWidget
sessionContext={this.state.sessionContext}
configuredReadArguments={this.state.configuredReadArguments}
activeReadArguments={this.state.activeReadArguments}
memory={this.state.memory}
Expand Down

0 comments on commit 733088f

Please sign in to comment.