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

Implement VS Code support for dynamic debug configs #95835

Closed
weinand opened this issue Apr 22, 2020 · 13 comments
Closed

Implement VS Code support for dynamic debug configs #95835

weinand opened this issue Apr 22, 2020 · 13 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Apr 22, 2020

please see #88230 for the detailed discussion.

@weinand weinand added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Apr 22, 2020
@weinand weinand added this to the April 2020 milestone Apr 22, 2020
@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2020

VS Code now supports dynamic debug configs. Thus closing this.
Here's a test plan item placeholder #95837
Here are my related comments from the other issue

Pushed an initial version of this for the "Select and Start Debug" action. Gif attached.
Note the following:

  • As agreed we show each dynamic configuration provider once user chooses it we fetch the configurations for that provider (for multi root workspace we call provide for every root)
  • We pass the cancelation token, so if the user cancles a long running quick pick the cancelation should be passed to the debug adapter
  • group names: first is "launch.json", second is "contributed". That was more clear to me than "configured" and "contributed" as tasks does

I will now look into adding this into the debug dropdown.
Try it out and let me know what you think.

dynamicLaunch

I have pushed support to also show dynamic providers in the debug dropdown.
For now I decded to use the "LABEL..." as the label. The ... at the end to make it distinguishable and that the user is aware that an additional dialog will follow. Also there is a separator.
Note that I do not use a separator between the launch configurations and a

Try it out and let me know how it behaves for you.
Since we have done what we planned for this item I am closing this.
For pinning launch configurations and other ideas we have discussed here please open follow up feature requests.
I plan to make a test plan item for this issue

debugDropdown

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

@isidorn to make the new folder-like entries in the quick pick really stand out, we should use either the folder icon that task is using or the new debug icon we are using for sessions in the call stack view.

2020-04-22_09-52-28

isidorn added a commit that referenced this issue Apr 22, 2020
@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2020

@weinand makes sense. Pushed a change

Screenshot 2020-04-22 at 10 20 17

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

Nice!

I noticed that you've added an ellipsis too (which IMO makes sense).
@alexr00 you might want to consider this too...

@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2020

@weinand yeah, I pinged also @alexr00 for that, but since they mostly show more of those items she thought it looks a bit like clutter.

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

@isidorn for sake of consistency then let's remove the ellipsis from debug too.

@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2020

I would like debug to be consistent across quick pick and our debug dropdown where we also show elipses (we can not show icons so it is a good diferentiator).
Also I prefer corectness over consisntency, so I would like to keep the elipses :)

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

@isidorn fair enough, so let's put some pressure on Tasks... 😁

fyi @alexr00

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

@isidorn IMO the interaction with the new "Dynamic" items in the drop down menu is rather strange: selecting them has no effect but one has to press the play button to get the quickpick.
This is unexpected and very different from the "Add configuration..." item which works without the play button.
Could you not just open the Quickpick?

@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2020

@weinand this was a delibarate choice to tackle two problems:

  • What if a user wants to change the dynamic configuration provided, he would have to choose another item in the dropdown and choose this one again.
  • What if a user reloads, should it be the same one as last time, or we should ask again? Then we need to store.

Also it is about starting debugging so it made more sense to behave like the other options which are also configurations and nothing happens once you choose them.
"Add configuration" felt like a special case and I did not think others should align to it.

All in all I deliberatly made it this way because I thought it will be smother.
I might be wrong and we can easily change though.

@weinand
Copy link
Contributor Author

weinand commented Apr 22, 2020

@roblourens @connor4312 what do you think?
if you do not have a debug extension that implements a dynamic debug configuration provider, just checkout mock-debug and run the extension via F5.
in the new instance open the debug dropdown and play with the "Mock Debug" item...
You can find my assessment above: #95835 (comment)

@connor4312
Copy link
Member

What if a user wants to change the dynamic configuration provided, he would have to choose another item in the dropdown and choose this one again.

I'm not sure what you mean by this. If I have a configuration in the launch configuration, I can hit the gear to jump to it. But for dynamic configurations that isn't useful. Is there a different action you're thinking of?

What if a user reloads, should it be the same one as last time, or we should ask again? Then we need to store.

Imo a good behavior would be to still select the dynamic configuration (so I can hit the play button or F5 to run it again) but automatically open the quickpick as soon as I select it. I think it's safe to assume that when a user makes a selection, especially for a dynamic configuration, they intend to start debugging as well.

@alexr00
Copy link
Member

alexr00 commented Apr 23, 2020

The folder icon alone signifies that there is more menu to be shown (file pickers don't show an ellipse for folders). Having both is redundant, and the ellipse does add more noise.

If you really want tasks to have both though I will add it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

4 participants