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: allow a single debug extension to provide multiple configs #100794

Merged
merged 3 commits into from
Jun 24, 2020

Conversation

connor4312
Copy link
Member

@connor4312 connor4312 commented Jun 22, 2020

Previously if a debug extension provided multiple dynamic
configurations, we would just use the first debugger -- whatever that
was. This change now shows all configurations for which dynamic configs
are registered.

I also adjusted the picker to automatically select the first item if
there's only a single configuration provided. This works well for the
debug terminal, but also means that the user doesn't see the name of
the selected item, which might not be desirable. Open to pushback.

Together these finish the request for a separate top-level contribution
for the terminal in #98054

Finally, with that adjustment I made a tweak so that the picker shows
up in a busy state while extensions are activating. Previously you
would select a dynamic configuration title and could have a few seconds
of delay before the picker came up, which is probably not desirable.

Previously if a debug extension provided multiple dynamic
configurations, we would just use the first debugger -- whatever that
was. This change now shows all configurations for which dynamic configs
are registered.

I also adjusted the picker to automatically select the first item if
there's only a single configuration provided. This works well for the
debug terminal, but also means that the user doesn't see the name of
the selected item, which might not be desirable. Open to pushback.

Together these finish the request for a separate top-level contribution
for the terminal in  #98054

Finally, with that adjustment I made a tweak so that the picker shows
up in a `busy` state while extensions are activating. Previously you
would select a dynamic configuration title and could have a few seconds
of delay before the picker came up, which is probably not desirable.
@connor4312 connor4312 added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jun 22, 2020
@connor4312 connor4312 added this to the June 2020 milestone Jun 22, 2020
@connor4312 connor4312 requested a review from isidorn June 22, 2020 22:13
@connor4312 connor4312 self-assigned this Jun 22, 2020
@isidorn
Copy link
Contributor

isidorn commented Jun 23, 2020

@connor4312 thanks a lot for this PR. Good work overall!
I did a first review and left comments in code. Once we adress that I can do a second review and test out the flow.

I like the optimisation to not bother the user if there is only one Configuration!

As for PRs in general, I am a big fan and keep em coming. Also feel free to split PRs in multiple if they tackle seperate things since that makes it easier for me to review and merge.
Also might you be interested in improving this #96293
Since it is in this code area. I can also look into it once we merge this one in so we do not hit conflicts. Thanks!

@connor4312 connor4312 requested a review from isidorn June 23, 2020 17:29
@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2020

Looks great. Merging in. 🍻 ☀️

@isidorn isidorn merged commit fe66be1 into master Jun 24, 2020
@isidorn isidorn deleted the connor4312/allow-multiple-dynamic branch June 24, 2020 14:26
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants