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.

Sign up for GitHub

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

Jump to bottom

Support debug/createConfiguration proposed menu API #14215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support debug/createConfiguration proposed menu API #14215

wants to merge 1 commit into from

Conversation

rschnekenbu
Copy link
Contributor

What it does

This PR adds the support of the newly introduced debug/createConfiguration menu. This menu is currently a proposed API.

fixes #14114

contributed on behalf of STMicroelectronics

How to test

A test extension has been developed to test this feature:

The steps to reproduce are almost the same as in VS code:

  • delete the launch.json file from your .theia/.code folder
  • Go to the debug view and click on the cog wheel that opens or create the launch.json file.
  • if the extension is not installed, the behavior should be the same as usual, e.g. generate a skeleton of launch.json and edit it.
  • if the extension is installed, some quick pick menu shall come, that allows you to select between the available commands from the new menu or the usual launch.json generation and edition.
  • The command, once invoked, does not do anything currently except showing a notification. It would be up to the extension to generate a file as far as I understand the feature on vs code.
issue14114.mp4

Follow-ups

None

Review checklist

Reminder for reviewers

@rschnekenbu
Support debug/createConfiguration proposed menu API
8b2a407 
fixes eclipse-theia#14114

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
tsmaeder
tsmaeder reviewed Sep 25, 2024
View reviewed changes
Copy link
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

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

As I've commented, this PR duplicates some code which I find problematic. For me, the question is: do we really need this PR? It's a proposed menu item which AFAIC causes no breakage if we don't implement it.

packages/debug/src/browser/debug-configuration-manager.ts
const menu = this.menuRegistry.getMenu(DEBUG_CREATE_CONFIGURATION_MENU);
const quickPickItems = [];
for (const child of menu.children) {
if (child.children) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This code duplicates the code from TabbarToolbarRegistry#visibleItems(). That code is problematic and I'm not too happy to spread it around more.

@tsmaeder
Copy link
Contributor

I've filed an issue about refactoring menus to simplify the code #14217

@rschnekenbu
Copy link
Contributor Author

We can delay this PR until the issue #14217 is fixed.

@rschnekenbu rschnekenbu self-assigned this Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsmaeder tsmaeder tsmaeder left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@rschnekenbu rschnekenbu

Labels
None yet
Projects
PR Backlog
Status: Waiting on author
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[vscode] Add debug/createConfiguration menu as a proposed menu
2 participants
@rschnekenbu @tsmaeder