Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Simplify generated launch.json #2120

Closed
4 tasks
isidorn opened this issue Nov 13, 2018 · 9 comments
Closed
4 tasks

Simplify generated launch.json #2120

isidorn opened this issue Nov 13, 2018 · 9 comments
Assignees
Labels
needs-decision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@isidorn
Copy link
Contributor

isidorn commented Nov 13, 2018

This milestone I am looking into simplifing generated launch.json for various extensions microsoft/vscode#62851

The launch.json that GO generates is attached at the end. This is cool and simple. However we can still do the following

  • Go needs to activate on onDebugInitialConfigurations
  • Consider to use quickPick in DebugConfigurationProvider to ask the user if he would like to launch file, launch package, connect to server, launch test package, launch test function and based on that create a launch configuration.
  • Remove default fields which are not necessery like env, args. Both from the configurationSnippets and from the intial configurations
  • What is the biggest pain point for go debugging setup, could we somehow help there with dynamicly providing debug configuratons or resolving them
{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "name": "Launch",
            "type": "go",
            "request": "launch",
            "mode": "auto",
            "program": "${fileDirname}",
            "env": {},
            "args": []
        }
    ]
}
@isidorn
Copy link
Contributor Author

isidorn commented Nov 20, 2018

@ramya-rao-a Are there any plans to address this?
Do you agree with my suggesstions here?
If you do not have cycles to address this but would like to see it done, I can try to get some users to provide a PR which tackles this.

Thanks a lot

@ramya-rao-a
Copy link
Contributor

Go needs to activate on onDebugInitialConfigurations

Can you elaborate on this? Is this just an addition to the activation events?

Consider to use quickPick in DebugConfigurationProvider to ask the user if he would like to launch file, launch package, connect to server, launch test package, launch test function and based on that create a launch configuration

In Go, debugging current file and current package is the same thing. All files under a folder belong to the same package.
Test files follow the convention of having the suffix _test.go. I already have code in place to use the right config based on file name.
We have codelens to debug the test function under the cursor.

That leaves the "connect to server" scenario, which requires user to provide the host, port and remote path etc. And users are aware that they have to create a configuration in the launch.json file for this.

Remove default fields which are not necessery like env, args. Both from the configurationSnippets and from the intial configurations

This was intentional for discoverablilty purposes :)

What is the biggest pain point for go debugging setup, could we somehow help there with dynamicly providing debug configuratons or resolving them

There isnt much during the setup time as far as I know. I resolve many of the defaults in the resolveDebugConfiguration call.

Debugging tests using codelens has the problem of not being able to pass args or set env, but other than that, I cannot think of any issues at setup time.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 21, 2018

@ramya-rao-a thanks for your response.

onDebugInitialConfigurations is just an activation event. Here's how node is using it here

Leaving fields for discoverability purposes makes good sense if a field is changed often by the user. So it would be great if you could go through them and decide for each if they really should be there. Also what is mode and why is this shown to the user?

Great that you are already resolving defaults in the resolveDebugConfiguration call.

I am just trying to give some suggestions on how to improve this, in the end you know much better what would lead to a better go debugging experience. So let me know if there is something on the vscode side which we could do to improve this.

@ramya-rao-a
Copy link
Contributor

Leaving fields for discoverability purposes makes good sense if a field is changed often by the user. So it would be great if you could go through them and decide for each if they really should be there.

Yes, I went through this exercise and cleared quite a few a couple of months ago

Also what is mode and why is this shown to the user?

mode is what is used to differentiate between debugging local program, remote program or a test.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 22, 2018

Awesome that you already cleared some fields.
Can't we automatically than infer the mode so the user does not have to specify it?

@isidorn
Copy link
Contributor Author

isidorn commented Jan 4, 2019

Just a ping reminder regarding this.

@ramya-rao-a
Copy link
Contributor

Yes, we can automatically infer mode if not provided.
Other 2 values for mode are remote and exec used for remote debugging and debugging an executable respectively.

@lggomez
Copy link
Contributor

lggomez commented Jan 13, 2019

Maybe not directly related with the json generation, but I think one of the main problems during debug configuration is the inconsistency between the debug test codelens and the debug viewlet launcher. Centralizing the configuration between GoRunTestCodeLensProvider and GoDebugConfigurationProvider should be the starting point towards this, but I don't know the current feasibility of this change

@stamblerre stamblerre added the needs-decision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 12, 2020
@ramya-rao-a
Copy link
Contributor

Closing this issue in favor of golang/vscode-go#131 due to the repo move

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-decision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

4 participants