dotnet / vscode-csharp

Official C# support for Visual Studio Code
MIT License
2.85k stars 666 forks source link

Default launch.json should point at a startup project instead of the program #1876

Open gregg-miskelly opened 6 years ago

gregg-miskelly commented 6 years ago

Behavior Today

Currently our launch.json fully specifies the path to the .NET Core dll to launch.

Current example:

{
   "configurations": [
        {
...
            // If you have changed target frameworks, make sure to update the program path.
            "program": "${workspaceFolder}/bin/Debug/netcoreapp2.0/clinew.dll",
...
        }
    ]
}

With the new launch provider work, it seems like there might be an opportunity to simplify this, and eliminate the issue that if you change a project from targeting one framework monitor to another, launch will do the wrong thing.

Proposal

  1. Add a startupProject property to launch.json: Path to the .csproj of that project that will be launched. Either startupProject should be set or program should be set, but not both.
  2. Make it so that the extension has a notion of current configuration/platform/target framework which is used by OmniSharp (ex: if there is #if code), the build tasks (by following the current platform/configuration/framework when issuing build commands), and the debugger (by debugging that configuration of the startup project). This can be shown in the status bar like the way the current project works. Example: configuration-in-task-bar
  3. Stop generating a tasks.json.
  4. cwd shouldn't be specified by the template, and if it isn't provided, the resolver should fill it in with the directory of the project.

Proposed example:

{
   // Use IntelliSense to find out which attributes exist for C# debugging
   // Use hover for the description of the existing attributes
   // For further information visit https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md
   "version": "0.2.0",
   "configurations": [
        {
            "name": ".NET Core Launch (console)",
            "type": "coreclr",
            "request": "launch",
            "preLaunchTask": "dotnet build: ./ExampleProject.csproj",
            "startupProject": "./ExampleProject.csproj",
            "args": [],
            // For more information about the 'console' field, see https://github.com/OmniSharp/omnisharp-vscode/blob/master/debugger-launchjson.md#console-terminal-window
            "console": "internalConsole",
            "stopAtEntry": false,
            "internalConsoleOptions": "openOnSessionStart"
        }
    ]
}
WardenGnaw commented 6 years ago

@DustinCampbell @rchande

As discussed, I've investigated DebugConfigurationProviders resolveDebugConfiguration.

From the VsCode August 2017 release notes:

A method resolveDebugConfiguration is called by VS Code when a new debug session is started. The implementation of resolveDebugConfiguration can "massage" the passed debug configuration by filling in missing values or by adding/changing/removing attributes.

Original launch.json configuration: image

Modifying the launch.json in resolveDebugConfiguration: image

After F5: image

gregg-miskelly commented 6 years ago

Cool!

@DustinCampbell @rchande @WardenGnaw @caslan @TheRealPiotrP how does my proposed syntax look to you (see 'Proposed example' above)?

rchande commented 6 years ago

@WardenGnaw Nice. So it looks like VS Code doesn't modify launch.json if you "resolve" it.

@gregg-miskelly Looks ok to me. Should we also add properties that let you specify debug/release and architecture?

gregg-miskelly commented 6 years ago

@rchande Sounds good to me. I updated the proposal above.

@WardenGnaw do you know if we tweak the preLaunchTask can we actually change what configuration/platform/framework is built?

WardenGnaw commented 6 years ago

resolveDebugConfiguration is executed before the preLaunchTask.

If we pass in the configuration/platform/framework to resolveDebugConfiguration, we could set the value of it somewhere in order to pass it to TaskProvider. This would modify the build command for the specific configuration/platform/framework.

@TheRealPiotrP

gregg-miskelly commented 6 years ago

It occurs to me that another option would be to put a bit of UI somewhere is VS Code to let the user pick the configuration/platform/framework, and make both the TaskProvider and DebugProvider respect that.

Thoughts?

rchande commented 6 years ago

If it's possible to add them to the UI, I would prefer to add configuration/platform selectors over asking people to edit launch.json. Where are vs code extensions allowed to show additional UI?

WardenGnaw commented 6 years ago

@rchande We can add to the status bar. I did a quick test to see how it would look: image

When users select on that StatusBarItem it can trigger the QuickPick dialog: image

What we can do is have the quick pick menu items be populated by omnisharp-server by discovering which configuration/platform combination exists and display it as \<Configuration>|\<Platform>.

rchande commented 6 years ago

@WardenGnaw That's awesome! Sounds good to me.

TheRealPiotrP commented 6 years ago

This is great! In the proposal, is preLaunchTask necessary? It duplicates data with the startupProject. Can we reduce this to a single project path?

On the same topic, do we imagine using the tasks api to generate a task for each possible build flavor? Or can we just invoke the appropriate build via internal API call? In this scheme I don’t believe that we are asking the user to think about tasks.json at all while debugging, so perhaps we can own the end-to-end in the launcher...

WardenGnaw commented 6 years ago

@TheRealPiotrP Don't we need the preLaunchTask in order to run a build before running the application?

Also, I believe we would have a single task for the project. The resolveTask function in the task provider would retrieve the selected configuration and platform, and then pass it into msbuild with the selected parameters e.g. /p:Configuration=Debug;Platform=Any CPU

gregg-miskelly commented 6 years ago

I updated the proposal above based on what was discussed in this thread.

RE: omitting preLaunchTask -- to elaborate a bit more on what Andrew said, based on Andrew's previous experiment, I think we could dynamically inject the preLaunchTask if we wanted (so there is no preLaunchTask in the on-disk launch.json). There are three potential downsides of going to that level of hiding. None of them are horrible by any means, but personally I think that maybe it makes it so it is better to leave in something:

  1. If we leave in the preLaunchTask then hopefully we avoid the problem where if the user's application's build system gets complicated enough that they can't just build the startup project in order to get a runable app then hopefully we have left them a breadcrumb to know what to customize. For example, if they have run time dependencies that aren't compile time dependencies, in full VS they can either tweak things to build the full solution, or add solution-level (instead of project level) dependencies. But in VS Code I think folks would likely to make a new build task.
  2. Because we don't own preLaunchTask in the launch.json schema we can't provide documentation for it. So users might get confused why suddenly builds stop working if they go from a launch.json file with no preLaunchTask, to having one that points at some task of their own.
  3. Since VS Code probably didn't intentionally support this scenario we should talk to them about it before we do this. At the least we should probably make sure we have tests for it that are running regularly against Insiders.
guidobouman commented 6 years ago

Hey @gregg-miskelly,

Still working on this? We have multiple projects that run in VS, VS Code & Rider. Detecting the program allows the launch.json to be configured only once, and not break when upgrading dotnet versions. This way the VS Code setup can become transparent, and offload all of the dynamic settings to the launchSettings.json files for each sub project.