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

Simplify generated launch.json #1611

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

Simplify generated launch.json #1611

isidorn opened this issue Nov 13, 2018 · 9 comments
Assignees
Labels
Area-Configuration Issue-Enhancement A feature request (enhancement). Resolution-Fixed Will close automatically.

Comments

@isidorn
Copy link
Contributor

isidorn commented Nov 13, 2018

Hey,

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

The launch.json that Powershell generates is attached at the end. This is a bit too complex for the avarage user and I suggest to simplify it the following way:

  • You are already using a DebugConifugarionProvider for resolving configuration. I suggest to aslo use it for providing debug conifiguration as it should make the following step possible
  • DebugConfigurationProvider should use the quickPick to ask the user if he would like to launch or to attach. If he chooses launch I would ask a follow up question if he would like to launch a current file, Current File in Temporary Console, Current File w/Args Prompt or just an interactive powershell session.
  • Remove default fields which are not necessery like args, runspaceId
  • You are already contributing configuraitonSnippets which is great. However these snippets also need to be simplified. I suggest to remove all attributes that have the default value. Example: stopAtEntry, cwd, args

If you agree with the suggestions I am making here I am also here to help with any potential questions you might have. The changes should not require a lot of work but will simplify the flow a lot imho. It should be much less complex and not too much like a wizard experience

{
    // 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": [
        {
            "type": "PowerShell",
            "request": "launch",
            "name": "PowerShell Launch Current File",
            "script": "${file}",
            "args": [],
            "cwd": "${file}"
        },
        {
            "type": "PowerShell",
            "request": "launch",
            "name": "PowerShell Launch Current File in Temporary Console",
            "script": "${file}",
            "args": [],
            "cwd": "${file}",
            "createTemporaryIntegratedConsole": true
        },
        {
            "type": "PowerShell",
            "request": "launch",
            "name": "PowerShell Launch Current File w/Args Prompt",
            "script": "${file}",
            "args": [
                "${command:SpecifyScriptArgs}"
            ],
            "cwd": "${file}"
        },
        {
            "type": "PowerShell",
            "request": "attach",
            "name": "PowerShell Attach to Host Process",
            "processId": "${command:PickPSHostProcess}",
            "runspaceId": 1
        },
        {
            "type": "PowerShell",
            "request": "launch",
            "name": "PowerShell Interactive Session",
            "cwd": ""
        }
    ]
}

@isidorn
Copy link
Contributor Author

isidorn commented Nov 20, 2018

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

@rkeithhill
Copy link
Contributor

rkeithhill commented Nov 20, 2018

Removing fields set to their default value makes sense. I think we originally put in the ones we thought our users were likely to change - to "advertise" them.

Regarding the quick pick approach, when the user goes through that once, will it save that debug configuration? If our users have to go through a quick pick wizard every time they start a debug session, that will get old real quick.

Regarding starting this work, we have a deprecated API (webview preview) issue we really need to address first but after that we could tackle this. But PRs are always welcome. :-)

@rjmholt
Copy link
Contributor

rjmholt commented Nov 20, 2018

Yes, there's definitely intent to address this and we want to do what we can to (1) simplify the debug experience and (2) accord with VSCode's recommendations, but we're currently locked up in some other things and don't have an ETA on this work.

@isidorn
Copy link
Contributor Author

isidorn commented Nov 20, 2018

Great news that you plan to look into this.
Yeah this is not really urgent but we just wanted to know that you are on it.

Keeping fields around to advertise them makes perfect sense. But imho only fields which the user change often should be advertised.

As for the quick pick approach: it would appear only when the launch.json is being generated, based on the users choice we would generate a different launch.json and the user would not be asked again.
Here's an example in docker for reference microsoft/vscode-docker#618

@rkeithhill rkeithhill self-assigned this Jul 13, 2019
rkeithhill added a commit to rkeithhill/vscode-powershell that referenced this issue Jul 14, 2019
@rkeithhill
Copy link
Contributor

@isidorn I've implemented this in PR #2084. Could you give this a look? It has gone through some iterations so look at the latest comments on the PR.

rkeithhill added a commit that referenced this issue Jul 28, 2019
* Address PR comments, change to single promptClean up debug configuration snippet names & descriptions. Remove Launch Pester Tests debug config snippet. The Launch Scriptsnippet gives an example of invoking Pester plus we have code lens to debug Pester tests.

* Remove w/Args prompt debug config snippet

* Switch to int id check in provideDebugConfig

* Address PR feedback, remove path module as it wasn't being used
TylerLeonhardt pushed a commit to TylerLeonhardt/vscode-powershell that referenced this issue Sep 20, 2019
…2084)

* Address PR comments, change to single promptClean up debug configuration snippet names & descriptions. Remove Launch Pester Tests debug config snippet. The Launch Scriptsnippet gives an example of invoking Pester plus we have code lens to debug Pester tests.

* Remove w/Args prompt debug config snippet

* Switch to int id check in provideDebugConfig

* Address PR feedback, remove path module as it wasn't being used
TylerLeonhardt added a commit that referenced this issue Sep 21, 2019
* Address PR comments, change to single promptClean up debug configuration snippet names & descriptions. Remove Launch Pester Tests debug config snippet. The Launch Scriptsnippet gives an example of invoking Pester plus we have code lens to debug Pester tests.

* Remove w/Args prompt debug config snippet

* Switch to int id check in provideDebugConfig

* Address PR feedback, remove path module as it wasn't being used
@TylerLeonhardt
Copy link
Member

I think we can close this now :) @rkeithhill

@UnicodeTreason
Copy link

With these simplified options, whats the preferred method for debugging a script that has parameters.
Used to use "PowerShell Launch Current File w/Args Prompt" and enter the parameters I wanted to test with.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Jan 29, 2020
@UnicodeTreason
Copy link

Ah apoligies, I follow the documentation rabbit hole further and found my answers.

From here: https://code.visualstudio.com/docs/languages/powershell#_debugging
You then have to go to here: https://devblogs.microsoft.com/scripting/debugging-powershell-script-in-visual-studio-code-part-2/

And then it's explained how to hard code some parameters which is nice enough for most users.
Though it could be nice to mention what the old launch.json did with "args": [ "${command:SpecifyScriptArgs}" ] to let the user enter Param's on debug launch.

@rjmholt rjmholt added the Resolution-Fixed Will close automatically. label Jan 29, 2020
@SydneyhSmith SydneyhSmith removed the Needs: Maintainer Attention Maintainer attention needed! label Jan 30, 2020
@rkeithhill
Copy link
Contributor

@JustinAddams Also note that the tooltip to the args parameter mentions ${command:SpecifyScriptArgs}:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Configuration Issue-Enhancement A feature request (enhancement). Resolution-Fixed Will close automatically.
Projects
None yet
Development

No branches or pull requests

6 participants