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

Make the "Select environment" prompt when launching a debugger clearer #146338

Closed
DanTup opened this issue Mar 30, 2022 · 34 comments
Closed

Make the "Select environment" prompt when launching a debugger clearer #146338

DanTup opened this issue Mar 30, 2022 · 34 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@DanTup
Copy link
Contributor

DanTup commented Mar 30, 2022

If you press F5 in a project with no active file, you're prompted to select a debugger like this:

Screenshot 2022-03-30 at 19 16 19

This dialog is quite confusing to Dart/Flutter developers, because when they want to run a Flutter app in Chrome, they must not select Chrome at the top, but rather Dart&Flutter. If they do select Chrome, it creates a launch.json that has "type": "pwa-chrome" which means they are not prompted at all the next time they press F5, the launch just doesn't do what they'd expect. They need to know to manually go and delete the launch.json to resolve the issue.

This has come up a few times:

I think there are some tweaks that would make this clearer and avoid the confusion:

  • Don't call this "environment", since that's easily confused with the environment in which you want to run the app. Saying "Select debugger" would be clearer
  • Use some heuristics to sort the entries.. If the Dart/Flutter extensions are activated it's almost certain that Chrome is the wrong option
  • Don't create a launch.json by default for pwa-chrome. There are entries on the Debug side bar for creating launch.json, one with default values is pointless and makes it harder to recover from a mistake like this
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality labels Mar 30, 2022
@weinand weinand added this to the On Deck milestone Mar 30, 2022
@roblourens
Copy link
Member

  1. I will switch to "Select Debugger" right now if no objection from @weinand or @connor4312
  2. Not sure how this would work, and I don't want to hardcode anything for these extensions in particular
  3. Sorry, not sure what you mean for this point. You mean the "Chrome" entry shouldn't show up?

@DanTup
Copy link
Contributor Author

DanTup commented Apr 5, 2022

  1. Perhaps this could be influenced by languages? VS code already knows to use the Dart debugger if the active file is Dart, so perhaps it could infer that if a project is 90% Dart (or 90% Python) that it could "suggest" the Dart/Python/whatever extension at the top as a "Suggested debugger"?

  2. I mean selecting the Chrome entry should not create a launch.json unnecessarily. It can be created by the user from the debug side bar if they need to customise it. A lot of debuggers just run without the file altogether now (like Dart). The issue with creating it here is that it forces every subsequent Run/Debug to go through that path, so if the user accidentally picks Chrome (as has happened to at least a few Dart users), it changes future behaviour to do the wrong thing unless they understand exactly what happened here (which is, their bad selection wrote a file that causes that same selection to be made automatically next time).

@weinand
Copy link
Contributor

weinand commented Apr 5, 2022

@DanTup I like your suggestion to make it "harder" for a user to create the "wrong" launch.json (or to create a launch.json at all, given that more and more debuggers try to avoid the need for launch.json anyway).

... and I agree that we could use the "debuggers.languages" contribution to determine the "debugger nature" of a workspace folder:

CleanShot 2022-04-05 at 16 39 46@2x

Today this information is used to map the active editor contents to a specific debugger (if a user has pressed F5 and no launch.json exists). But we could use the same information to determine the "language" frequency distribution for the files in a workspace and assume that this results in a ranking for the available debuggers.

If this ranking shows a clear winner, we could actually avoid the "Select environment" QuickPick completely and use the same code path that we use for F5 in an active editor (the only difference for the underlying debugger extension would be the missing document URL in this case...)

But even if we do not want to avoid the "Select environment" QuickPick, we could change its behavior like this:

  • selecting a debugger will forward the "F5" to it (without creating a launch.json),
  • a "launch.json" is only created by pressing a "gear" action at the end of an debugger entry.

@DanTup @roblourens @connor4312 what do you think?

@DanTup
Copy link
Contributor Author

DanTup commented Apr 5, 2022

Both sound good to me. Avoiding the prompt altogether if it can be reliable prevents the user from picking the wrong thing at all. If that's not possible (or not in all cases), then not creating launch.jsons just from this selection at least prevents the bad choice being made automatically next time.

@weinand
Copy link
Contributor

weinand commented Apr 6, 2022

@isidorn what do you think about my proposal from above?

@isidorn
Copy link
Contributor

isidorn commented Apr 6, 2022

@weinand I like the push towards reducing the wrong automatic creation of launch.json. Since having a wrong launch.json can put the user in a very hard spot. Also it will be more consistent with the other flow. Auto creation of launch.json was the first step we did before dynamic providing of launch configurations, and now this feels like debt behaviour to me.

While I like your idea to count files and rank debuggers, I am not sure if that would help here. Since both Chrome and Dart are probably interested in same files, so they would most likely be ranked the same. Thus you would still have to ask the user. Or am I missing something?

@DanTup
Copy link
Contributor Author

DanTup commented Apr 6, 2022

Since both Chrome and Dart are probably interested in same files, so they would most likely be ranked the same

This probably isn't the case. Flutter projects will likely only have one (or a very small number) of .html files (and probably no .js), and be mostly .dart files.

If there are more .html/.js files in a workspace than .dart, then ranking the Chrome debugger above wouldn't be unreasonable to me (although, whether it should be automatically picked rather than just "suggested" at the top of the list, I'm less certain).

That said - it's possible after running the Flutter: New Project command you'll end up with exactly one .dart file, and exactly one .html file from the default template, so in some cases at least, it might not be a clear winner. For most non-skeleton projects, it's likely there will be more .dart files added (and no additional .html files) though.

@weinand
Copy link
Contributor

weinand commented Apr 6, 2022

@isidorn in order to verify that my proposal actually works, I did an experiment:

  • I determined the set of "supported languages" for the Dart and Chrome/Edge debuggers:
    • Dart: "dart"
    • Chrome/Edge: "javascript", "typescript", "javascriptreact", "typescriptreact", "html", "css", "coffeescript", "handlebars", "vue"
  • then I connected VS Code to a Dart docker image and created a "Dart Web app".
  • I found one dart file, one html file, and one css file.

So you are right: Chrome debug would win with 2:1!

But I think that could be fixed if Dart would more aggressively capture more web related types. If Dart would just add "html" and "css", it would win with "3:1" ;-)

In addition, the second part of my proposal would address this problem:
We would show the "Select Debugger" Quickpick because there is no clear winner.
If users know what they are doing, they would probably pick the Dart debugger if they want to develop a Dart web app.
But then we would no longer create a launch.json but instead transfer directly into the Dart debugger's "resolveDebugConfiguration" hook which can handle the case.

@DanTup
Copy link
Contributor Author

DanTup commented Apr 6, 2022

btw, the reason users are used to selecting Chrome in a picklist like this, is for Flutter we let you change the target device and since there aren't many UI options in VS Code, it's done via a picker like this:

Screenshot 2022-04-06 at 17 27 47

then I connected VS Code to a Dart docker image and created a "Dart Web app".

FWIW, Dart web apps that don't use Flutter are not so common compared to those using Flutter. The Flutter: New Project command is a more interesting case to me which looks like this:

Screenshot 2022-04-06 at 17 30 29

Because of the default test created, there are actually two .dart files, not one. If #74671 was implemented, things could be a bit more specific because Dart could list pubspec.yaml too, which is very Dart-specific (as is analysis_options.yaml).

@elahehrashedi
Copy link

elahehrashedi commented Apr 11, 2022

As the issue #146956 is closed, allow me to mention our request in this thread:
In C/C++ extension we have two debuggers that we can't combine them into one debugger.
When there are few debuggers from other extensions, it makes sense to ask for "Select environment" or "select debugger", but is it possible not to select the environment/debugger when the debuggers are from the same extension?

in other words, Is it possible to skip the step of selecting the debugger type and instead provide a list of configurations across all debugger types in the extension? c.c. @jureid

@weinand
Copy link
Contributor

weinand commented Apr 20, 2022

@elahehrashedi with the strategy sketched above you could contribute languages only for one of your debuggers. This should result in skipping the "select step" (at least if no other C++ debuggers exist in other extensions).

@isidorn
Copy link
Contributor

isidorn commented Apr 22, 2022

@weinand yesterday I discussed with C++ and I believe your proposal would not solve their problem. Because they want to contribute languages by all their debuggers and can not use when clauses to disable some of the debuggers.
For example on Windows they really want to show the results of all three debuggers combined - and that is what their play button does. There they have full control since it is their action.

I am not sure if there is a good way to solve this for C++. Merging results from multiple debuggers just because they come from the same extension sounds like a workaround.

@weinand
Copy link
Contributor

weinand commented Apr 25, 2022

@elahehrashedi Is there something like a "main" C++ debugger that is always there and contributes the single "smart" play button that can delegate to the other C++ debuggers?

If the answer is "yes" then only this debugger needs to contribute "languages" (and then there will be no selection dialog).

If the answer is "no" and all C++ debuggers contribute the same "languages" then you are creating exactly the problem that requires the selection dialog: the same files can be debugged by different C++ debuggers and users will have to pick the debugger they want.

@isidorn
Copy link
Contributor

isidorn commented Apr 25, 2022

fyi @bobbrow

@roblourens
Copy link
Member

But we could use the same information to determine the "language" frequency distribution for the files in a workspace and assume that this results in a ranking for the available debuggers.

I don't really like this. I don't want to add one file to my workspace, shift the balance, and suddenly get different behavior. Also, counting files in a workspace will be slow.

But I like this:

But even if we do not want to avoid the "Select environment" QuickPick, we could change its behavior like this:

  • selecting a debugger will forward the "F5" to it (without creating a launch.json),
  • a "launch.json" is only created by pressing a "gear" action at the end of an debugger entry.

@weinand
Copy link
Contributor

weinand commented Apr 25, 2022

I agree, that the second part (the "I like this") is a good first step.

But that does not really solve the first problem: many debug extension authors don't like the first part: the "debugger selector". The real "problem" here is that VS Code has one built-in debugger and that always gets into the way when users just want to debug something that is not JS or TS. Python users doesn't care about JS or TS and if they have created a new Python project and press F5 they just want to debug Python.

How can we avoid the debugger selector in this case?

My proposal tries to eliminate the debugger selector with a heuristics based on the existing static "language" contribution. If it cannot decide clearly what debugger to use it will fall back to the current debugger selector.

Delegating the decision what debugger to use back to some pieces of code running in the installed debugger extensions has the problem that too many extension are activated (which will be slow).

Do you have a better idea how to tackle this?

@bobbrow
Copy link
Member

bobbrow commented Apr 25, 2022

@elahehrashedi Is there something like a "main" C++ debugger that is always there and contributes the single "smart" play button that can delegate to the other C++ debuggers?

There is no "main" debugger. I believe @WardenGnaw commented on another issue (I forget which) that bundling the different debug types into a single debugger entry makes it difficult for the debugger team to organize their code.

If the answer is "no" and all C++ debuggers contribute the same "languages" then you are creating exactly the problem that requires the selection dialog: the same files can be debugged by different C++ debuggers and users will have to pick the debugger they want.

We just want to provide all the launch configurations all the time. If the debuggers we register in package.json could be part of a "group" (new concept) and then we provide all the configurations for that group, I think we could get the behavior we want. The only thing selecting an environment does for C++ right now is filter our results. When users click our "play" button, they see all the possibilities at once. I still think it would be nice if we could bypass the "select environment" prompt altogether, but that might be slightly different from what this issue is asking for and I don't mean for us to hijack the discussion. Should we open a separate issue for what C++ needs? (potentially 2 if the debugger "group" idea has merit in your opinion)

@DanTup
Copy link
Contributor Author

DanTup commented Apr 26, 2022

The real "problem" here is that VS Code has one built-in debugger and that always gets into the way when users just want to debug something that is not JS or TS.

I don't think it's only the built-in debuggers either. I think debuggers that contribute onDebugDynamicConfigurations are activated when you press F5, so the list could includes additional debuggers from extensions that weren't even activated before then?

@weinand
Copy link
Contributor

weinand commented Apr 26, 2022

@bobbrow sorry that the term "main debugger" was unclear. I was not suggesting to bundle the different debug types into a single debugger entry.
What I meant was: is there one C++ debugger that is available on every platform, e.g. if debugger A and B are used on Windows, and A and C are used on linux, then "A" is the debugger used on both (intersection).
Then the "A" debugger could contribute all the launch configurations and "languages" contributions and the smart "Play" button.


You said:

I still think it would be nice if we could bypass the "select environment" prompt

That's exactly what we are discussing here.

Today we need the "select environment" prompt iff..

  • there is more than one debugger installed and enabled,
  • there is no "launch.json", and
  • no file is open in the editor.

In this case VS Code does not know what debugger to use, so the "select environment" prompt is needed.

Even if we would group the C++ debuggers into one, then we would still have VS Code's builtin JS debugger and would still need the "select environment" prompt.

The C++ extension cannot just "bypass" the "select environment" prompt because that would be unfair to all Non-C++ debuggers: users would not have a chance to pick the JS or Python debuggers if C++ "takes them all".

Above I've proposed a heuristic to automatically pick a debugger for a given project by matching some project files against file types listed in the "languages" contribution of the debuggers.
But @roblourens pointed out (correctly so) that this introduces some randomness and in-transparency into the behavior of "F5" debugging.

An alternative would be to let the user use the "select environment" prompt only once per folder/project and then remember the answer.
The drawback with this approach is that new users have to answer another question before they can debug...

@weinand
Copy link
Contributor

weinand commented Apr 26, 2022

@DanTup
yes, your observation about dynamic launch configs is probably right and we should address this too...

@roblourens
Copy link
Member

roblourens commented May 14, 2022

Sent a PR to sort debug extensions that were already activated to the top. And the result is cached before the first debugger activation so it should hopefully just pick up extensions that got activated by workspaceContains or other actions.

Example where mock-debug got activated by a markdown file
image

@DanTup
Copy link
Contributor Author

DanTup commented May 14, 2022

We discussed asking more questions after the user picks chrome - we can have them enter the server URL or show another set of options which will generate different configs. Python does something like this. And if a Dart user accidentally picks Chrome, that might give them a hint that they are headed the wrong direction.

That sounds good. If you need that info from the user anyway, I think prompting for it is a better experience than making them go and add it to launch.json (this is what Dart does for Attach, we use an input box to ask for the VM Service URI so it can just be pasted in and we still don't need to create a launch.json - although we do support it there too). It also gives people an opportunity to "cancel" if they realise it's wrong, without it leaving a permanent launch.json to confuse things further next time.

I would also consider renaming it to something like "Web App (Chrome)" but wouldn't use PWA there. That would incidentally sort it to the bottom in alpha order

That would also be good IMO (I used PWA as an example as that's what's in the ID, but anything more explicit would probably help).

We can sort the debuggers based on whether the extensions are activated

That sounds good too. Yes it'll only help the first time, but I suspect the number of times people press F5 without a Dart file open is probably heavily skewed towards the first run. Once they open a Dart file (as they eventually probably will), they won't see the prompt.

The problem is that these patterns can trigger really expensive searches, so it might be limited to static patterns only, or no **

That could help too. It's not uncommon for projects to be nested in Dart, but I suspect people with multiple/nested projects are probably not encountering this for the first time, so probably more likely to make the correct selection. It's likely peoples first experience where the most confusion is.

Sent a PR to sort debug extensions that were already activated to the top. And the result is cached before the first debugger activation so it should hopefully just pick up extensions that got activated by workspaceContains or other actions.

Do you mean that even after other extensions are activated, the earliest one would still be sorted to the top? If so, that sounds great :-)

Out of interest, what causes the "suggested" label in your screenshot? Is it just the top one, or does it only appear after that item has been previously selected?

Thanks!

@roblourens
Copy link
Member

Filed microsoft/vscode-js-debug#1270 to talk about the first part.

Sent microsoft/vscode-js-debug#1271 to rename the Chrome/Edge DAs.

To explain my change some more - yes, even after other extensions are activated, the ones that were activated before the first debug session still go to the top. The "Suggested" section is these debuggers from extensions that were activated before the first debug session. The next section is everything else. Here's what it will look like in a Dart project.

image

@roblourens
Copy link
Member

Something sort of related about this experience that still bugs me, but I am not sure what to do about, so I will just put notes here for now:

The provided "initial" launch configs go under the Run and Debug button, and the "dynamic" configs go under the link text, "Show all automatic debug configurations":

image

The dynamic configs are like this:
image

When the user runs a dynamic config, e.g. I pick Node.js > Javascript Debug Terminal, you switch into this UI mode
image
The blue button is gone, you only have dynamic configs and "Add configuration" so it looks very different from what you had before.
image
And, the selected dynamic config is remembered for this workspace. After reloading the window, if I press Run and Debug, it runs the last selected dynamic config and I can't see the initial configs that I would have seen before. This might make a user feel stuck. The only way to get unstuck is to use the "Add configuration" button in the dropdown and create a launch.json, but this will be a different workflow than what they had before.

I need to do more research to understand the history but here are some things to investigate that might help make this flow more consistent:

  • The docs and vscode.d.ts don't quite explain the "initial" launch configs correctly, they imply they are only used for generating a new launch.json. I'm sure it used to work that way but not the dynamic and initial configs seem more similar.
  • Why remember the selected dynamic launch config, but not the selected initial launch config? Remember both or neither.
  • When there is no launch.json, and a dynamic launch config has taken over the Run and Debug button, we should show the dropdown so a user can still pick a different config, and this dropdown should also show the initial config options. With this, we can get the behavior of remembering the last F5, but they can also change the selected debugger.

connor4312 pushed a commit to microsoft/vscode-js-debug that referenced this issue May 17, 2022
* More explicit DA names
See microsoft/vscode#146338

* Fix Edge debugger label
roblourens added a commit that referenced this issue May 17, 2022
@DanTup
Copy link
Contributor Author

DanTup commented May 17, 2022

Yeah, I've been caught out by those dynamic entries appearing in the Debug side bar dropdown too. I spent some time trying to find a launch.json that has an unexpected named debug session before I realised where it had come from. It would be nice if the view was more consistent before/after running the first session, and it was clearer in that drop-down what the source of the configurations were.

connor4312 pushed a commit to microsoft/vscode-js-debug that referenced this issue May 17, 2022
* More explicit DA names
See microsoft/vscode#146338

* Fix Edge debugger label
roblourens added a commit that referenced this issue May 17, 2022
wannieman98 pushed a commit to wannieman98/vscode that referenced this issue May 17, 2022
@isidorn
Copy link
Contributor

isidorn commented May 19, 2022

@roblourens these are cool improvements 👏
For the history behind those decisions feel free to schedule a sync with me. I will be happy to try to help :)

@roblourens
Copy link
Member

Closing this for this month's improvements - can continue in #150663

@github-actions github-actions bot locked and limited conversation to collaborators Jul 14, 2022
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

6 participants