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

Debug viewContainer menu contribution points #200880

Closed
thegecko opened this issue Dec 14, 2023 · 14 comments · Fixed by #212499 · May be fixed by #212501
Closed

Debug viewContainer menu contribution points #200880

thegecko opened this issue Dec 14, 2023 · 14 comments · Fixed by #212499 · May be fixed by #212501
Assignees
Labels
api-proposal debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@thegecko
Copy link
Contributor

Apologies if this is too similar to existing requests, but I'm keen to pull together a common request for adding debug contribution points.

Similar requests:

And related:

Rationale:

Debugging often needs deep integration between views (e.g. embedded debugging) where they are all driven from a common context and can interact with other. A common way to add these links/commands is through contribution points and some of the provided debug views lack these extension points. Can I suggest contribution points in context menus for the following:

  • Watch view (e.g. to open the value in a memory viewer)
  • Breakpoints view (e.g. to expose a 'run to breakpoint' command)
  • Disassembly view (e.g. to show source, manage breakpoints, show memory)
  • Debug viewlet bar (e.g. to add a command to launch a custom GUI to create a complex debug configuration)

I assume this is the debug viewlet:

Screenshot 2023-12-14 at 19 03 34

Happy to work on a PR if this is of interest.

@roblourens
Copy link
Member

I am open to these for areas where the data type in the list is represented in the extension API already (eg Breakpoints). For others, like variables, that would involve having to think about how that type should look presented to an extension, which would take more effort.

@roblourens roblourens added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Dec 14, 2023
@vscodenpa vscodenpa added this to the Backlog Candidates milestone Dec 14, 2023
@vscodenpa
Copy link

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@roblourens
Copy link
Member

I guess we actually have this for variables already

@thegecko
Copy link
Contributor Author

I guess we actually have this for variables already

We do, the ask is to consider this for some of the other views

@vscodenpa
Copy link

🙂 This feature request received a sufficient number of community upvotes and we moved it to our backlog. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodenpa vscodenpa modified the milestones: Backlog Candidates, Backlog Dec 20, 2023
@theIDinside
Copy link

theIDinside commented Jan 16, 2024

I am open to these for areas where the data type in the list is represented in the extension API already (eg Breakpoints). For others, like variables, that would involve having to think about how that type should look presented to an extension, which would take more effort.

I definitely understand the difficulty or that this isn't a trivial feature. But at the same time, when it comes to the debug adapter interface, we have a clear spec to adhere to. That's the API - and in a sense, no more, no less (though I'm sure there's always a bunch of new cool magical stuff one can add).

I think it would be neat if VSCode therefore exposed this functionality in a "bi-directional" fashion (sort of) as it's already well defined in the DAP spec. The debug adapter knows what data it gives VSCode, the DAP spec is clear on life times and object references and therefore it should not be that troublesome to pass that data back and forth - even outside of a normal "debug adapter request chain"; since the spec is clear that, for instance, variablesReference: 1 is only valid until the next resume request.

I don't know if this rant made any sense, but it's an attempt to spark a discussion in hopes of possibly moving something forward.

@thegecko
Copy link
Contributor Author

Proposed contribution points covering this request can now be found in these PRs:

cc @roblourens @isidorn

@roblourens roblourens modified the milestones: Backlog, July 2024 Jul 1, 2024
@roblourens
Copy link
Member

Sorry for taking so long to look at this. I'll try to make some progress on it this week and get a discussion going about best practices for context menus here. It's not straightforward when we want to expose context types that don't exist elsewhere in extension API

@thegecko
Copy link
Contributor Author

thegecko commented Jul 1, 2024

Sorry for taking so long to look at this. I'll try to make some progress on it this week and get a discussion going about best practices for context menus here. It's not straightforward when we want to expose context types that don't exist elsewhere in extension API

Great! I put some thought into the context types being exposed, feedback welcome.

@roblourens roblourens modified the milestones: July 2024, August 2024 Jul 24, 2024
@vs-code-engineering vs-code-engineering bot added the unreleased Patch has not yet been released in VS Code Insiders label Jul 31, 2024
@vs-code-engineering vs-code-engineering bot removed the unreleased Patch has not yet been released in VS Code Insiders label Jul 31, 2024
Copy link

		Issue marked as unreleased but unable to locate closing commit in issue timeline. You can manually reference a commit by commenting `\closedWith someCommitSha`, or directly add the `insiders-released` label if you know this has already been releaased

@vs-code-engineering vs-code-engineering bot added unreleased Patch has not yet been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jul 31, 2024
@roblourens roblourens added the verification-needed Verification of issue is requested label Aug 26, 2024
@roblourens
Copy link
Member

Verification steps: you can enable the contribViewContainerTitle proposal, contribute a command to the viewContainer/title menu, but it only works for the debug viewcontainer and you must set viewContainer == workbench.view.debug in the when clause.

@amunger amunger added the verified Verification succeeded label Aug 28, 2024
@amunger
Copy link
Contributor

amunger commented Aug 28, 2024

from the sample here, I could get a contribution into the callstack context menu and the debug toolbar, but not in the watch viewlet.

also, this contrib didn't seem to change anything

"viewContainer/title": [
				{
					"command": "extension.helloWorld",
					"when": "viewContainer == workbench.view.debug"
				}
			]

@thegecko
Copy link
Contributor Author

roblourens closed this as completed in https://github.com/microsoft/vscode/pull/212499[last month](#200880 (comment))

Please be aware this request is implemented through multiple PRs and isn't complete.
The addition for disassembly, watch and breakpoints is in #212501
@roblourens could you reopen please?

....I could get a contribution into the callstack context menu and the debug toolbar, but not in the watch viewlet.

Please be aware the example covers multiple PRs which aren't yet all merged:

...also, this contrib didn't seem to change anything

The restriction for viewContainer == workbench.view.debug was added after I wrote the example.

@amunger amunger reopened this Aug 29, 2024
@roblourens roblourens changed the title Debug context menu contribution points Debug viewContainer menu contribution points Aug 29, 2024
@roblourens
Copy link
Member

Opened #227169 to track the rest of it. This issue is just for the view container menu. @amunger if you're expecting to see the icon for your command, you can ad "group": "navigation", otherwise it's in the ... menu.

@roblourens roblourens removed the verified Verification succeeded label Aug 29, 2024
@amunger amunger added the verified Verification succeeded label Aug 30, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-proposal debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
5 participants