-
Notifications
You must be signed in to change notification settings - Fork 511
Add optional UI to prompt for script args. #707
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
Conversation
This is not quite ready yet. I'm waiting on a resolution or workaround to this issue - microsoft/vscode#25712 |
This currently has an issue where you can't clear the previous args to send no args. This is has been addressed in VSCode for May 2017 drop - microsoft/vscode#25712 (comment)
Awesome! I suppose we'll wait for the next VS Code update before shipping this in the PS extension? |
Yes. I keep testing daily builds of VSCode Insiders but haven't observed the fixed behavior yet. |
OK, the issue is fixed in today's insiders build. It should ship in the (late) May release. |
This is for the prompt for script args UI. If on >= 1.13 then save state. If less than that do not save state because there is a bug in VSCode that will make it impossible to specify no args after specify some args -even across different sessions.
One more thing to do. Need to figure out a debug config snippet for this. I'll propose one here in a bit. |
Also reworded the dbg configuration names. The drop down control in VSCode is on the narrow side. By putting in PowerShell, we take up most of the width. I notice that Node doesn't bother to prefix their dbg configs "Launch Program", "Mocha Tests". However I've already used cross language dbg configs in a single project and it is nice to know which language the dbg config is for. Therefore I think a good compromise is to prefix our dbg snippets with "PS". In the launch.json file, you can still see the config type = "PowerShell". I'll post some screen shots so you can see what this results look like.
We can ship this before the May VSCode release drops. I added code to check the VSCode version and I only enable the "save the prev args" features if we're on 1.13 or later. |
I'm not sure I like the args being saved across separate VSCode sessions. Let me see if I can find a way to store the args just for the current session. |
The args are remembered across VSCode invocations but at least each workspace has its own args remembered. That actually might be a feature.
Using "PS" as the prefix just feels wrong to me even though I can understand your reason for doing it. As far as I know, nobody has complained about not being able to see the name in the debug UI. Maybe we should leave it the way it is so that it's clearer? I suppose there's also the possibility of rewording them slightly. Instead of "PowerShell Launch Current File" it could be "Launch Current PowerShell File", "Interactive PowerShell Session" instead of "PowerShell Interactive Session", etc. What do you think? |
OK putting PS back to PowerShell. I think it is better to leave PowerShell first because so that in the mixed debug configuration scenario, the PowerShell configs sort next to one another. PR update coming. |
Thanks Keith! This change is pretty awesome, I like the new feature a lot. Is it ready to be merged now? |
I think so. Do you think it is OK to remember scripts args between sessions using workspaceState? |
Yeah, I think that's a great way to do it. We might need a way to clear the stored args in the future, but I wouldn't worry about it until someone asks for it. Merging it now, thanks man! |
I think this will make for a nice little debugging enhancement. |
Address one issue #705