microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.35k stars 28.6k forks source link

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

Closed DanTup closed 2 years ago

DanTup commented 2 years ago

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:

roblourens commented 2 years ago
  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 commented 2 years ago
  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 commented 2 years ago

@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:

@DanTup @roblourens @connor4312 what do you think?

DanTup commented 2 years ago

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 commented 2 years ago

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

isidorn commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

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 commented 2 years ago

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 https://github.com/microsoft/vscode/issues/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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

fyi @bobbrow

roblourens commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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..

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 commented 2 years ago

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

elahehrashedi commented 2 years ago

@weinand We don't necessarily need to bypass the "select environment" prompt when other debuggers are available. But we would like to group C++ ones together as a single entry in the list.

weinand commented 2 years ago

@elahehrashedi There is always at least one "other debugger" available (js-debug). So the "select environment" prompt will always be there.

"Grouping C++ debuggers" together as a single entry does not make any sense for VS Code. VS Code needs to know which debugger needs to be activated in order to send requests to it. A "group" doesn't help at all.

If there is a "group entry" for all C++ debuggers, should we activate all of them and then send the requests to all of them and sort out or merge what they return? That's not a business VS Code wants to be part of.

If you need this behavior you can introduce a single "umbrella debugger" that delegates to the individual debuggers. VS Code has used this approach when we introduced a new JS debugger for newer version of node.js: the original debugger was delegating requests to the new version whenever it detected that the new version of node.js was used.

elahehrashedi commented 2 years ago

@weinand let's explore the "umbrella debugger" idea. Here are the two debggers in package.json:

"debuggers": [
      {
        "type": "cppdbg",
        "label": "C++ (GDB/LLDB)",
        "languages": [
          "c",
          "cpp",
          "cuda-cpp"
        ],
        // some different args
      },
      {
        "type": "cppvsdbg",
        "label": "C++ (Windows)",
        "when": "workspacePlatform == windows",
        "languages": [
          "c",
          "cpp",
          "cuda-cpp"
        ],
        // some different args
      }
]

Let's say we define the "umbrella debugger" like below:

     {
        "type": "all",
        "label": "C++",
        "when": "workspacePlatform == windows" ,
        "languages": [
          "c",
          "cpp",
          "cuda-cpp"
        ]
      },

I can design this debugger to provide all debug configurations that we need from all debuggers.

  1. However, doing so will create a new debug type all that is not meant to be used by users in their launch.json.
  2. Also, in order to hide the two cppvsdbg and cppdbg debuggers from the menu 1, I will need to change the "when" clause to "when": "never" for the other two debuggers, to have the menu 2 that we want:

menu 1 image

menu 2 image

However, doing this change will result in an error in launch.json, as the other two types are not valid anymore: image

Kindly, how did you resolve this issue? C.C. @isidorn @sinemakinci1

weinand commented 2 years ago

@elahehrashedi the "umbrella" debugger type must be the type that users see and use in their launch configs. So that will define the "cppdbg" type. The individual sub-debuggers will use private debug types that users should not see or use. The resolveDebugConfiguration hook of the umbrella will rewrite the type to the desired sub-type.

weinand commented 2 years ago

A new ("non-magic") proposal that avoids unintended creation of a launch.json and shows the "select environment" prompt only once:

Given the following situation:

pressing "F5" (or running the "Start Debugging" command) results in this behavior:

DanTup commented 2 years ago

@weinand I think this problem still has the original issue that led me to raise this, but makes it even more difficult to fix:

As things are today, the user needs to know the launch.json file that was created is causing the problem. With the new proposal, it seems like the remembered selection is now even less visible than the launch.json (if at all). How would the user discover the issue and fix it so they can then select the Dart&Flutter debugger?

Perhaps renaming the "Chrome" debugger to be a bit more specific ("Progressive Web App (PWA) - Chrome") or something might help?

roblourens commented 2 years ago

@DanTup do you think that this mainly happens to confused users on their first try, or do users successfully use the Dart debugger then at some point accidentally pick Chrome instead?

Do you recommend users not create a launch.json, or would you want to include one in the default setup so projects will always have a working configuration?

roblourens commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Filed https://github.com/microsoft/vscode-js-debug/issues/1270 to talk about the first part.

Sent https://github.com/microsoft/vscode-js-debug/pull/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 commented 2 years ago

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:

DanTup commented 2 years ago

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.

isidorn commented 2 years ago

@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 commented 2 years ago

Closing this for this month's improvements - can continue in https://github.com/microsoft/vscode/issues/150663