Enhances the context data of the context menu #135

planger · 2024-05-06T16:05:52Z

What it does

Extends the VS Code context to augment the information available to context menus. In particular:

Endianess and bytes per MAU
Start address of memory data groups and their size (normalized to MAU size)

How to test

The easiest way to test this is to print out the arguments of a context menu command, e.g. in memory-webview-main.ts:112:

vscode.commands.registerCommand(MemoryWebview.ToggleRadixPrefixCommandType, (ctx: WebviewContext) => {
        console.log('args', ctx);
        this.setMemoryViewSettings(ctx.messageParticipant, { showRadixPrefix: !ctx.showRadixPrefix });
}),

And observe the data in the ctx object. Here is a break down of what it currently contains.

Context menu somewhere in the upper part of the memory inspector (now contains `endianness` and `bytesPerMau`)

{
    "messageParticipant": {
        "type": "webview",
        "webviewId": "memory-inspector.memory_2"
    },
    "showRadixPrefix": true,
    "showAsciiColumn": false,
    "showVariablesColumn": true,
    "endianness": "Little Endian",
    "bytesPerMau": 1,
    "activeReadArguments": {
        "memoryReference": "0x20001c2c",
        "offset": 0,
        "count": 256
    },
    "webviewSection": "optionsWidget",
    "preventDefaultContextMenuItems": true,
    "memory-inspector.messageParticipant.type": "webview",
    "memory-inspector.messageParticipant.webviewId": "memory-inspector.memory_2",
    "memory-inspector.showRadixPrefix": true,
    "memory-inspector.showAsciiColumn": false,
    "memory-inspector.showVariablesColumn": true,
    "memory-inspector.endianness": "Little Endian",
    "memory-inspector.bytesPerMau": 1,
    "memory-inspector.activeReadArguments.memoryReference": "0x20001c2c",
    "memory-inspector.activeReadArguments.offset": 0,
    "memory-inspector.activeReadArguments.count": 256,
    "memory-inspector.webviewSection": "optionsWidget",
    "memory-inspector.preventDefaultContextMenuItems": true,
    "webview": "memory-inspector.memory"
}

Context menu on a specific group in the memory data (now contains start and size of the group)

{
    "messageParticipant": {
        "type": "webview",
        "webviewId": "memory-inspector.memory_2"
    },
    "showRadixPrefix": false,
    "showAsciiColumn": false,
    "showVariablesColumn": true,
    "endianness": "Little Endian",
    "bytesPerMau": 2,
    "activeReadArguments": {
        "memoryReference": "0x20001c2c",
        "offset": 0,
        "count": 256
    },
    "webviewSection": "memoryTable",
    "preventDefaultContextMenuItems": true,
    "memory-inspector.messageParticipant.type": "webview",
    "memory-inspector.messageParticipant.webviewId": "memory-inspector.memory_2",
    "memory-inspector.showRadixPrefix": false,
    "memory-inspector.showAsciiColumn": false,
    "memory-inspector.showVariablesColumn": true,
    "memory-inspector.endianness": "Little Endian",
    "memory-inspector.bytesPerMau": 2,
    "memory-inspector.activeReadArguments.memoryReference": "0x20001c2c",
    "memory-inspector.activeReadArguments.offset": 0,
    "memory-inspector.activeReadArguments.count": 256,
    "memory-inspector.webviewSection": "memoryTable",
    "memory-inspector.preventDefaultContextMenuItems": true,
    "column": "data",
    "memory-inspector.column": "data",
    "value": "000000000000aa810000000020001c90",
    "memoryData": {
        "group": {
            "startAddress": "0x20001c2c",
            "length": 4
        }
    },
    "memory-inspector.memoryData.group.startAddress": "0x20001c2c",
    "memory-inspector.memoryData.group.length": 4,
    "webview": "memory-inspector.memory"
}

Context menu on a specific variable (already contained this information before this PR)

{
    "messageParticipant": {
        "type": "webview",
        "webviewId": "memory-inspector.memory_2"
    },
    "showRadixPrefix": true,
    "showAsciiColumn": false,
    "showVariablesColumn": true,
    "endianness": "Little Endian",
    "bytesPerMau": 2,
    "activeReadArguments": {
        "memoryReference": "0x20001c2c",
        "offset": 0,
        "count": 256
    },
    "webviewSection": "memoryTable",
    "preventDefaultContextMenuItems": true,
    "memory-inspector.messageParticipant.type": "webview",
    "memory-inspector.messageParticipant.webviewId": "memory-inspector.memory_2",
    "memory-inspector.showRadixPrefix": true,
    "memory-inspector.showAsciiColumn": false,
    "memory-inspector.showVariablesColumn": true,
    "memory-inspector.endianness": "Little Endian",
    "memory-inspector.bytesPerMau": 2,
    "memory-inspector.activeReadArguments.memoryReference": "0x20001c2c",
    "memory-inspector.activeReadArguments.offset": 0,
    "memory-inspector.activeReadArguments.count": 256,
    "memory-inspector.webviewSection": "memoryTable",
    "memory-inspector.preventDefaultContextMenuItems": true,
    "column": "variables",
    "memory-inspector.column": "variables",
    "value": "arg",
    "variable": {
        "name": "arg",
        "type": "void*",
        "value": "(void*) 0x0",
        "isPointer": true
    },
    "memory-inspector.variable.name": "arg",
    "memory-inspector.variable.type": "void*",
    "memory-inspector.variable.value": "(void*) 0x0",
    "memory-inspector.variable.isPointer": true,
    "webview": "memory-inspector.memory"
}

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the contribution guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the code of conduct

colin-grant-work · 2024-05-06T16:09:09Z

Not opposed to the change, but at the moment, it doesn't look like there's a consumer for the new data?

planger · 2024-05-06T16:20:24Z

@colin-grant-work Wow that fast! I wasn't even finished updating the comment yet :-)

Not opposed to the change, but at the moment, it doesn't look like there's a consumer for the new data?

The requirement is to prepare for external context menus that would need this context information (e.g. to properly react to context menu on a specific memory data group). A context menu item that'll likely come soon is to add data breakpoints, which also needs the start and end address of a selected group.

But I'll better let @jreineckearm give the full picture on the future requirements that necessitate this change.

colin-grant-work · 2024-05-06T16:37:34Z

I think I mistook a repo watch generated email for a review request email - sorry!

* Endianess setting * Bytes per MAU setting * Start address and length of the memory data groups

jreineckearm · 2024-05-21T08:15:06Z

src/webview/columns/data-column.tsx

@@ -90,6 +91,7 @@ export class EditableDataColumnRow extends React.Component<EditableDataColumnRow
            style={style}
            key={startAddress.toString(16)}
            onDoubleClick={this.setGroupEdit}
+            {...createGroupVscodeContext(startAddress, toOffset(startAddress, endAddress, this.props.options.bytesPerMau * 8))}


*8 looked a bit off initially. But realized later that most internal calculations are still based on bits....

jreineckearm · 2024-05-21T08:16:47Z

src/webview/utils/vscode-contexts.ts

 export function createVariableVscodeContext(variable: BigIntVariableRange): VscodeContext {
    const { name, type, value, isPointer } = variable;
    return createVscodeContext({ variable: { name, type, value, isPointer } });
 }

+function replacerForBigInt(_: string, value: unknown): unknown {
+    if (typeof value === 'bigint') {


Can we always be sure that bigint is only used for 64-bit handling?

At the moment bigint seems to be used only for memory addresses and memory data. As BigInt is not serializable directly, my intention with this replacer was that we can pass BigInt values directly into the context without worrying about that and have a consistent formatting of those values for extenders (hexadecimal).

Alternatively, we can also limit this formatting to the specific fields we currently know to be passed into the context, which would be fine too, if you prefer.

planger · 2024-06-04T08:38:12Z

@haydar-metin confirmed that what is specified in this PR is sufficient to be used and extended for #102. So we consider this to be ready for final review. Thank you!

jreineckearm

Thanks for verifying the set of new context data to be sufficient!

planger · 2024-06-04T15:17:10Z

Thank you both @colin-grant-work and @jreineckearm for your review!

Looks like the rebase of eclipse-cdt-cloud#135 causes a compile error. This change fixes the broken import.

Looks like the rebase of #135 causes a compile error. This change fixes the broken import.

planger requested a review from jreineckearm May 6, 2024 16:21

Enhances the context data of the context menu

58b226b

* Endianess setting * Bytes per MAU setting * Start address and length of the memory data groups

planger force-pushed the context-menu branch from c111b49 to 58b226b Compare May 7, 2024 11:52

planger mentioned this pull request May 15, 2024

Allow to set data breakpoints in memory view via context menu #102

Open

jreineckearm reviewed May 21, 2024

View reviewed changes

planger marked this pull request as ready for review June 4, 2024 08:36

planger requested a review from jreineckearm June 4, 2024 08:37

colin-grant-work approved these changes Jun 4, 2024

View reviewed changes

jreineckearm approved these changes Jun 4, 2024

View reviewed changes

planger merged commit 57338e7 into eclipse-cdt-cloud:main Jun 4, 2024
5 checks passed

planger deleted the context-menu branch June 4, 2024 15:17

planger added a commit to eclipsesource/vscode-memory-inspector that referenced this pull request Jun 4, 2024

Fix import

d7d7221

Looks like the rebase of eclipse-cdt-cloud#135 causes a compile error. This change fixes the broken import.

planger mentioned this pull request Jun 4, 2024

Fix import #137

Merged

1 task

planger added a commit that referenced this pull request Jun 4, 2024

Fix import

866d068

Looks like the rebase of #135 causes a compile error. This change fixes the broken import.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhances the context data of the context menu #135

Enhances the context data of the context menu #135

planger commented May 6, 2024 •

edited

Loading

colin-grant-work commented May 6, 2024

planger commented May 6, 2024

colin-grant-work commented May 6, 2024

jreineckearm May 21, 2024

jreineckearm May 21, 2024

planger May 21, 2024

planger commented Jun 4, 2024

jreineckearm left a comment

planger commented Jun 4, 2024

Enhances the context data of the context menu #135

Enhances the context data of the context menu #135

Conversation

planger commented May 6, 2024 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

colin-grant-work commented May 6, 2024

planger commented May 6, 2024

colin-grant-work commented May 6, 2024

jreineckearm May 21, 2024

Choose a reason for hiding this comment

jreineckearm May 21, 2024

Choose a reason for hiding this comment

planger May 21, 2024

Choose a reason for hiding this comment

planger commented Jun 4, 2024

jreineckearm left a comment

Choose a reason for hiding this comment

planger commented Jun 4, 2024

planger commented May 6, 2024 •

edited

Loading