Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
- Improve wording in setting descriptions
- Move shared types into common area instead of using type imports
- Fix 'fetchMemory' not returning the correct promise
  • Loading branch information
martin-fleck-at committed Apr 18, 2024
1 parent dc631e7 commit 96d8750
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 37 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -260,7 +260,7 @@
],
"enumDescriptions": [
"Refresh when the debugger stops (e.g. a breakpoint is hit)",
"Refresh automatically when the view is newly focussed.",
"Refresh automatically each time the view receives focus.",
"Refresh automatically after the configured `#memory-inspector.autoRefreshDelay#`",
"Memory Inspector needs to be manually refreshed by the user"
],
Expand Down
2 changes: 1 addition & 1 deletion src/common/messaging.ts
Expand Up @@ -18,9 +18,9 @@ import type { DebugProtocol } from '@vscode/debugprotocol';
import type { NotificationType, RequestType } from 'vscode-messenger-common';
import { URI } from 'vscode-uri';
import { VariablesView } from '../plugin/external-views';
import { MemoryViewSettings } from '../webview/utils/view-types';
import { DebugRequestTypes } from './debug-requests';
import type { VariableRange, WrittenMemory } from './memory-range';
import { MemoryViewSettings } from './webview-configuration';
import { WebviewContext } from './webview-context';

// convenience types for easier readability and better semantics
Expand Down
46 changes: 46 additions & 0 deletions src/common/webview-configuration.ts
@@ -0,0 +1,46 @@
/********************************************************************************
* Copyright (C) 2024 EclipseSource and others.
*
* 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 { WebviewIdMessageParticipant } from 'vscode-messenger-common';
import { AutoRefresh, Endianness, GroupsPerRowOption } from './manifest';
import { Radix } from './memory-range';

/** The memory display configuration that can be specified for the memory widget. */
export interface MemoryDisplayConfiguration {
bytesPerMau: number;
mausPerGroup: number;
groupsPerRow: GroupsPerRowOption;
endianness: Endianness;
scrollingBehavior: ScrollingBehavior;
addressPadding: AddressPadding;
addressRadix: Radix;
showRadixPrefix: boolean;
autoRefresh: AutoRefresh;
autoRefreshDelay: number;
}

export type ScrollingBehavior = 'Paginate' | 'Grow' | 'Auto-Append';

export type AddressPadding = 'Minimal' | number;

export interface ColumnVisibilityStatus {
visibleColumns: string[];
}

/** All settings related to memory view that can be specified for the webview from the extension "main". */
export interface MemoryViewSettings extends ColumnVisibilityStatus, MemoryDisplayConfiguration {
title: string
messageParticipant: WebviewIdMessageParticipant;
}
2 changes: 1 addition & 1 deletion src/plugin/memory-webview-main.ts
Expand Up @@ -46,8 +46,8 @@ import {
WriteMemoryResult,
writeMemoryType,
} from '../common/messaging';
import { MemoryViewSettings, ScrollingBehavior } from '../common/webview-configuration';
import { getVisibleColumns, WebviewContext } from '../common/webview-context';
import type { MemoryViewSettings, ScrollingBehavior } from '../webview/utils/view-types';
import { isVariablesContext } from './external-views';
import { outputChannelLogger } from './logger';
import { MemoryProvider } from './memory-provider';
Expand Down
3 changes: 2 additions & 1 deletion src/webview/components/memory-table.tsx
Expand Up @@ -28,10 +28,11 @@ import { Memory } from '../../common/memory';
import { WebviewSelection } from '../../common/messaging';
import { MemoryOptions, ReadMemoryArguments } from '../../common/messaging';
import { tryToNumber } from '../../common/typescript';
import { MemoryDisplayConfiguration, ScrollingBehavior } from '../../common/webview-configuration';
import { TableRenderOptions } from '../columns/column-contribution-service';
import { DataColumn } from '../columns/data-column';
import type { HoverService } from '../hovers/hover-service';
import { Decoration, isTrigger, MemoryDisplayConfiguration, ScrollingBehavior } from '../utils/view-types';
import { Decoration, isTrigger } from '../utils/view-types';
import { createColumnVscodeContext, createSectionVscodeContext } from '../utils/vscode-contexts';

export interface MoreMemorySelectProps {
Expand Down
3 changes: 2 additions & 1 deletion src/webview/components/memory-widget.tsx
Expand Up @@ -19,9 +19,10 @@ import { WebviewIdMessageParticipant } from 'vscode-messenger-common';
import { Memory } from '../../common/memory';
import { WebviewSelection } from '../../common/messaging';
import { MemoryOptions, ReadMemoryArguments, SessionContext } from '../../common/messaging';
import { MemoryDisplayConfiguration } from '../../common/webview-configuration';
import { ColumnStatus } from '../columns/column-contribution-service';
import { HoverService } from '../hovers/hover-service';
import { Decoration, MemoryDisplayConfiguration, MemoryState } from '../utils/view-types';
import { Decoration, MemoryState } from '../utils/view-types';
import { createAppVscodeContext, VscodeContext } from '../utils/vscode-contexts';
import { MemoryTable } from './memory-table';
import { OptionsWidget } from './options-widget';
Expand Down
3 changes: 2 additions & 1 deletion src/webview/hovers/hover-service.tsx
Expand Up @@ -17,8 +17,9 @@
import * as React from 'react';
import { HOST_EXTENSION } from 'vscode-messenger-common';
import { logMessageType } from '../../common/messaging';
import { MemoryDisplayConfiguration } from '../../common/webview-configuration';
import { MemoryAppState } from '../memory-webview-view';
import { Disposable, MemoryDisplayConfiguration } from '../utils/view-types';
import { Disposable } from '../utils/view-types';
import { messenger } from '../view-messenger';

export interface HoverableDetails {
Expand Down
5 changes: 3 additions & 2 deletions src/webview/memory-webview-view.tsx
Expand Up @@ -46,6 +46,7 @@ import {
WebviewSelection,
} from '../common/messaging';
import { Change, hasChanged, hasChangedTo } from '../common/typescript';
import { MemoryDisplayConfiguration } from '../common/webview-configuration';
import { AddressColumn } from './columns/address-column';
import { AsciiColumn } from './columns/ascii-column';
import { columnContributionService, ColumnStatus } from './columns/column-contribution-service';
Expand All @@ -56,7 +57,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, MemoryState } from './utils/view-types';
import { variableDecorator } from './variables/variable-decorations';
import { messenger } from './view-messenger';

Expand Down Expand Up @@ -303,7 +304,7 @@ class App extends React.Component<{}, MemoryAppState> {
// may happen when we initialize empty
return;
}
this.doFetchMemory(completeOptions);
return this.doFetchMemory(completeOptions);
};

protected async doFetchMemory(memoryOptions: Required<MemoryOptions>): Promise<void> {
Expand Down
32 changes: 3 additions & 29 deletions src/webview/utils/view-types.ts
Expand Up @@ -16,11 +16,11 @@

import deepequal from 'fast-deep-equal';
import type * as React from 'react';
import { WebviewIdMessageParticipant } from 'vscode-messenger-common';
import { AutoRefresh, Endianness, GroupsPerRowOption } from '../../common/manifest';
import { Endianness } from '../../common/manifest';
import { Memory } from '../../common/memory';
import { areRangesEqual, BigIntMemoryRange, Radix } from '../../common/memory-range';
import { areRangesEqual, BigIntMemoryRange } from '../../common/memory-range';
import { ReadMemoryArguments } from '../../common/messaging';
import { MemoryDisplayConfiguration } from '../../common/webview-configuration';

export interface SerializedTableRenderOptions extends MemoryDisplayConfiguration {
columnOptions: Array<{ label: string, doRender: boolean }>;
Expand Down Expand Up @@ -73,39 +73,13 @@ export interface FullNodeAttributes extends StylableNodeAttributes {
content: string;
}

/** All settings related to memory view that can be specified for the webview from the extension "main". */
export interface MemoryViewSettings extends ColumnVisibilityStatus, MemoryDisplayConfiguration {
title: string
messageParticipant: WebviewIdMessageParticipant;
}

/** The memory display configuration that can be specified for the memory widget. */
export interface MemoryDisplayConfiguration {
bytesPerMau: number;
mausPerGroup: number;
groupsPerRow: GroupsPerRowOption;
endianness: Endianness;
scrollingBehavior: ScrollingBehavior;
addressPadding: AddressPadding;
addressRadix: Radix;
showRadixPrefix: boolean;
autoRefresh: AutoRefresh;
autoRefreshDelay: number;
}
export type ScrollingBehavior = 'Paginate' | 'Grow' | 'Auto-Append';

export type AddressPadding = 'Minimal' | number;
export const AddressPaddingOptions = {
'Minimal': 'Minimal',
'Unpadded': 0,
'32bit': 32,
'64bit': 64,
} as const;

export interface ColumnVisibilityStatus {
visibleColumns: string[];
}

export type ReactInteraction<E extends Element = Element> = React.MouseEvent<E> | React.KeyboardEvent<E>;

export function isTrigger(event: ReactInteraction): boolean {
Expand Down

0 comments on commit 96d8750

Please sign in to comment.