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

Add debug settings for changing variable editor #197458

Closed
wants to merge 2 commits into from

Conversation

thegecko
Copy link
Contributor

@thegecko thegecko commented Nov 4, 2023

With reference to #155597, this PR adds the ability to specify the extension and editor view to use for viewing and editing variables in the debugger.

This is a simple implementation to allow the user to choose a different extension rather than the hard-coded HexEdit extension.

While this works, I fully expect a better (and more complicated) approach would be to have some sort of capability offered by extensions which can be detected by VS Code to expose the options in a better way.

To test this, observe the extension ID and editor ID are exposed in the debug configuration and default to the current behaviour. These can be updated to match the details of the eclipse.cdt-memory-inspector extension as follows:

  • Variable Editor Editor ID: memory-inspector.inspect
  • Variable Editor Extension ID: eclipse-cdt.memory-inspector

Screenshot 2023-11-04 at 20 24 21

This extension has been updated to use the same API as the HexEditor and should show as expected when selecting the View Binary Data button:

Screenshot 2023-11-04 at 20 25 32

The specified extension should be installed on demand if required.

@connor4312
Copy link
Member

connor4312 commented Nov 4, 2023

I'm still not a fan of this because it makes the debug memory URIs implicit API, which I don't want to do. The URI and interaction is done via the filesystem API, and this may not be a permanent solution: if there's ever functionality we add to debug memory that cannot be expressed with the filesystem API, the debug memory URIs will be removed and replaced with something else. There's technically already functionality like this in the form of unreadable memory regions, but we don't support them in VS Code yet.

Another possible way to get custom variable viewers in that doesn't add API: #197287 Extensions using that point would interact via DAP and could issue read/writeMemory requests.

@thegecko
Copy link
Contributor Author

thegecko commented Nov 4, 2023

I'm still not a fan of this because it makes the debug memory URIs implicit API, which I don't want to do. The URI and interaction is done via the filesystem API, and this may not be a permanent solution: if there's ever functionality we add to debug memory that cannot be expressed with the filesystem API, the debug memory URIs will be removed and replaced with something else.

I agree and this seems to be an implementation decision of the existing extension. Our memory viewer only uses the URI to find the address being passed and uses DAP memory read to get the actual data

Another possible way to get custom variable viewers in that doesn't add API: #197287

Great, this looks a lot cleaner and agreeing a proper solution was the partial point of this PR ;)

@connor4312
Copy link
Member

Let's close this PR in favor of that investigation, then. I have it slated for this iteration, so it may land in proposed in this month depending how much deliberation there is. Please do leave input/feedback on the issue, community feedback is especially important for API work.

@connor4312 connor4312 closed this Nov 5, 2023
@thegecko
Copy link
Contributor Author

thegecko commented Nov 5, 2023

Let's close this PR in favor of that investigation, then.

OK, as long as the default HexEdit functionality is either re-implemented or removed once the proposal has been agreed. I want the user to be able to choose the extensions to compose their experience and not be forced to have specific ones.

@connor4312
Copy link
Member

Ideally the hex editor would move to using the new APIs (though we'd still have some hint built-in so that users who don't have it installed yet would know a binary editor is available)

@thegecko
Copy link
Contributor Author

thegecko commented Nov 5, 2023

though we'd still have some hint built-in so that users who don't have it installed yet would know a binary editor is available

Is there any way this hint can also mention other extensions which could be more appropriate? Would this hint disappear if a different extension has been installed which undertakes this functionality to avoid confusion around multiple extensions doing the same job?

I'm keen to avoid the flow we have today where users are guided to install a pretty basic extension which seems favoured or "blessed" by VS Code.

@connor4312
Copy link
Member

connor4312 commented Nov 6, 2023

Would this hint disappear if a different extension has been installed which undertakes this functionality to avoid confusion around multiple extensions doing the same job?

I doubt it, but we'll certainly design for the case where there are multiple candidate providers to visualize a given variable, and e.g. users should be able to set defaults (or we memoize defaults automatically as they're chosen)

Discoverability is always a challenge in VS Code. The ideal of a simple and streamlined editor is often at odds with feature discoverability. It would be nice if these new types of extensions were discoverable, but I don't have any specific ideas around that at the moment. Suggestions welcome. I'm probably going to use the menu from VS included in the other issue as a starting point.

@connor4312
Copy link
Member

Btw, let's move these discussions to the linked issue so they're visible to others participating in the conversation

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants