Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom editor support #39

Merged
merged 8 commits into from
Oct 16, 2023
Merged

Custom editor support #39

merged 8 commits into from
Oct 16, 2023

Conversation

thegecko
Copy link
Contributor

@thegecko thegecko commented Oct 5, 2023

What it does

This PR adds initial support for opening the memory inspector using VS Code's build in memory button. It follows the pattern used by the existing hexedit extension.

How to test

Opening the inspector in this way will be possible when this issue is resolved in the VSCode repository: microsoft/vscode#155597

This will require a change in VS Code to allow a user to specify the settings for the memory viewer to use, however I'd like to use this extension as an example when I open that PR.

To test this at the moment, you need to build a custom VS Code, manually changing the extension ID and editor ID found here: https://github.com/microsoft/vscode/blob/e933c1d7b293f77565763d4a85154e1cb1b4cf45/src/vs/workbench/contrib/debug/browser/variablesView.ts#L527

to be:

const HEX_EDITOR_EXTENSION_ID = 'clipse-cdt.memory-inspector';
const HEX_EDITOR_EDITOR_ID = 'memory-inspector.inspect';

Review checklist

Reminder for reviewers

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Oct 5, 2023

@thegecko, just changed the wording of your description a bit so GitHub didn't think that this issue would fix the VSCode issue. :-)

src/plugin/memory-webview-main.ts Show resolved Hide resolved
src/plugin/memory-webview-main.ts Outdated Show resolved Hide resolved
src/plugin/memory-webview-main.ts Outdated Show resolved Hide resolved
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Oct 13, 2023

@thegecko, do want to merge this for easier distribution, or just have it available in PR form as an example for reference?

@thegecko
Copy link
Contributor Author

@thegecko, do want to merge this for easier distribution, or just have it available in PR form as an example for reference?

I'm keen it gets merged and released with our first release. It creates a good test example for the VS Code changes

@colin-grant-work
Copy link
Contributor

I'm keen it gets merged and released with our first release. It creates a good test example for the VS Code changes.

Alright, I'll build a VSCode variant and give it a good test today.

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General functionality looks good. Even plays nicely with the preferences @kenneth-marut-work implemented. Just a minor issue with the URI encoding, but in my testing, using decodeUriComponent did indeed fix that.

src/plugin/memory-webview-main.ts Show resolved Hide resolved
src/plugin/memory-webview-main.ts Show resolved Hide resolved
src/plugin/memory-webview-main.ts Outdated Show resolved Hide resolved
@colin-grant-work colin-grant-work merged commit 0bc5800 into main Oct 16, 2023
5 checks passed
@thegecko thegecko deleted the custom-editor branch October 16, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants