Add validation for invalid memory references (#131)

* Avoid dispatching of a fetch memory request if there is not enough information available i.e. no memory address is set * Add validation for the default values of the memory address widget * Prompts a validation error if the memory inspector has been opened for an empty/not set address * Ensure that value of address option field gets trimmed before requesting a memory update * Don't render option field hints (Actual...) if the memory view is still in initial state Fixes #126
eclipse-cdt-cloud · May 8, 2024 · 78f313c · 78f313c
1 parent 1ec6ea9
commit 78f313c
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 17 deletions.
diff --git a/src/webview/components/options-widget.tsx b/src/webview/components/options-widget.tsx
@@ -28,7 +28,7 @@ import { MemoryOptions, ReadMemoryArguments, SessionContext } from '../../common
 import { tryToNumber } from '../../common/typescript';
 import { CONFIG_BYTES_PER_MAU_CHOICES, CONFIG_GROUPS_PER_ROW_CHOICES, CONFIG_MAUS_PER_GROUP_CHOICES } from '../../plugin/manifest';
 import { TableRenderOptions } from '../columns/column-contribution-service';
-import { AddressPaddingOptions, MemoryState, SerializedTableRenderOptions } from '../utils/view-types';
+import { AddressPaddingOptions, DEFAULT_READ_ARGUMENTS, MemoryState, SerializedTableRenderOptions } from '../utils/view-types';
 import { createSectionVscodeContext } from '../utils/vscode-contexts';
 import { MultiSelectWithLabel } from './multi-select';
 
@@ -105,7 +105,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
 
     protected validate = (values: OptionsForm) => {
         const errors: FormikErrors<OptionsForm> = {};
-        const addressError = values.address.trim().length === 0 ? 'Required' : validateMemoryReference(values.address);
+        const addressError = values.address.trim().length === 0 ? 'Required' : validateMemoryReference(values.address.trim());
         if (addressError) {
             errors.address = addressError;
         }
@@ -120,11 +120,14 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
         return errors;
     };
 
-    componentDidUpdate(_: Readonly<OptionsWidgetProps>, prevState: Readonly<OptionsWidgetState>): void {
+    componentDidUpdate(prevProps: Readonly<OptionsWidgetProps>, prevState: Readonly<OptionsWidgetState>): void {
         if (!prevState.isTitleEditing && this.state.isTitleEditing) {
             this.labelEditInput.current?.focus();
             this.labelEditInput.current?.select();
         }
+        if (prevProps.activeReadArguments === DEFAULT_READ_ARGUMENTS) {
+            this.formConfig.initialErrors = this.validate(this.optionsFormValues);
+        }
     }
 
     override render(): React.ReactNode {
@@ -134,9 +137,11 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
         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) {
-                return <small className="form-options-memory-read-argument-hint">Actual: {memoryValue}</small>;
+            if (userValue === memoryValue || this.props.activeReadArguments === DEFAULT_READ_ARGUMENTS) {
+                return undefined;
             }
+            return <small className="form-options-memory-read-argument-hint">Actual: {memoryValue}</small>;
+
         };
 
         return (
@@ -438,12 +443,12 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
                 this.props.updateMemoryState({
                     configuredReadArguments: {
                         ...this.props.configuredReadArguments,
-                        memoryReference: value,
+                        memoryReference: value.trim(),
                     }
                 });
                 break;
             case InputId.Offset:
-                if (!Number.isNaN(value)) {
+                if (!!value && !Number.isNaN(value)) {
                     this.props.updateMemoryState({
                         configuredReadArguments: {
                             ...this.props.configuredReadArguments,
@@ -453,7 +458,7 @@ export class OptionsWidget extends React.Component<OptionsWidgetProps, OptionsWi
                 }
                 break;
             case InputId.Length:
-                if (!Number.isNaN(value)) {
+                if (!!value && !Number.isNaN(value)) {
                     this.props.updateMemoryState({
                         configuredReadArguments: {
                             ...this.props.configuredReadArguments,

diff --git a/src/webview/memory-webview-view.tsx b/src/webview/memory-webview-view.tsx
@@ -28,7 +28,6 @@ import {
     logMessageType,
     MemoryOptions,
     memoryWrittenType,
-    ReadMemoryArguments,
     readMemoryType,
     readyType,
     resetMemoryViewSettingsType,
@@ -51,7 +50,7 @@ import { AddressHover } from './hovers/address-hover';
 import { DataHover } from './hovers/data-hover';
 import { hoverService, HoverService } from './hovers/hover-service';
 import { VariableHover } from './hovers/variable-hover';
-import { Decoration, MemoryDisplayConfiguration, MemoryState } from './utils/view-types';
+import { Decoration, DEFAULT_READ_ARGUMENTS, MemoryDisplayConfiguration, MemoryState } from './utils/view-types';
 import { variableDecorator } from './variables/variable-decorations';
 import { messenger } from './view-messenger';
 
@@ -82,12 +81,6 @@ const MEMORY_DISPLAY_CONFIGURATION_DEFAULTS: MemoryDisplayConfiguration = {
     showRadixPrefix: true,
 };
 
-const DEFAULT_READ_ARGUMENTS: Required<ReadMemoryArguments> = {
-    memoryReference: '',
-    offset: 0,
-    count: 256,
-};
-
 class App extends React.Component<{}, MemoryAppState> {
     protected memoryWidget = React.createRef<MemoryWidget>();
 
@@ -249,13 +242,19 @@ class App extends React.Component<{}, MemoryAppState> {
         if (this.state.isFrozen) {
             return;
         }
-        this.setState(prev => ({ ...prev, isMemoryFetching: true }));
         const completeOptions = {
             memoryReference: partialOptions?.memoryReference || this.state.activeReadArguments.memoryReference,
             offset: partialOptions?.offset ?? this.state.activeReadArguments.offset,
             count: partialOptions?.count ?? this.state.activeReadArguments.count
         };
 
+        // Don't fetch memory if we have an incomplete memory reference
+        if (completeOptions.memoryReference === '') {
+            return;
+        }
+
+        this.setState(prev => ({ ...prev, isMemoryFetching: true }));
+
         try {
             const response = await messenger.sendRequest(readMemoryType, HOST_EXTENSION, completeOptions);
             await Promise.all(Array.from(

diff --git a/src/webview/utils/view-types.ts b/src/webview/utils/view-types.ts
@@ -59,6 +59,12 @@ export interface MemoryState {
     isMemoryFetching: boolean;
 }
 
+export const DEFAULT_READ_ARGUMENTS: Required<ReadMemoryArguments> = {
+    memoryReference: '',
+    offset: 0,
+    count: 256,
+};
+
 export interface UpdateExecutor {
     fetchData(currentViewParameters: ReadMemoryArguments): Promise<void>;
 }