[Feature] Workspace Setting for Integrated Console PS Version

[JustinGrote]

Summary of the new feature

I work different projects, some require WinPS5 due to library dependencies, some use PS6+. Currently the integrated terminal that is used appears to be a "global" setting. If I switch it to PS6+ in one project and then open the other, it stays PS6+, this leads to annoying toggle back and forth.

I want to be able to set so that when I open a certain project or workspace, it remembers what integrated terminal version I was using for that workspace

Proposed technical implementation details (optional)

User/Workspace setting for powershell.integratedterminalversion that would match the string in the session picker dropdown, e.g. "Powershell", "Windows Powershell (x64)", "Powershell Preview", etc.

[SydneyhSmith]

Thanks for opening this feature request @JustinGrote! I was looking through other issues and was wondering if you feel that #190 addresses the same concerns as you?

[JustinGrote]

That issue is from a really long time ago before multi-root workspaces were a native thing in VSCode, so I'd say not exactly, it just needs to be a setting to change which version of powershell the integrated console starts with, and it needs to be respected both as a user setting and a workspace setting.

…

On Tue, Jul 23, 2019 at 2:29 PM Sydney Smith ***@***.***> wrote: Thanks for opening this feature request @JustinGrote <https://github.com/JustinGrote>! I was looking through other issues and was wondering if you feel that #190 <#190> addresses the same concerns as you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2099?email_source=notifications&email_token=ADUNKURGHZVUY3E3YPIU4DTQA5Z4NA5CNFSM4IGGZ532YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UPYNY#issuecomment-514391095>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADUNKUWRTBPGKYMMKMHLDODQA5Z4NANCNFSM4IGGZ53Q> .

[TylerLeonhardt]

@JustinGrote is this the kind of setting you're looking for?

https://github.com/PowerShell/vscode-powershell/blob/master/package.json#L574-L576

[JustinGrote]

@TylerLeonhardt my problems are likely related to #1586. I did some more extensive testing and found the following (using powershell-preview extension):

1. powershell.powershellexepath gets populated immediately if you toggle shell from the lower right menu (or use Show Session Menu)
2. It does not update if you specify a powershell.powershelldefaultversion and reload window.
3. Killing the terminal after the reload reverts to this powershell.powershellexepath that was previously saved in (because it was never updated)

For instance if you set powershelldefaultversion to "Powershell Preview" (added via AdditionalExePaths) but your last shell was "Windows Powershell (x64)", then PS Preview will load on reload, but if you kill the terminal via the trash icon and restart it, it comes back as "Windows Powershell (x64)"

Fix
Make it so that when the extension finds the selection via powershell.powershelldefaultversion, it immediately sets the settings.json "placeholder" in the same way that the session selection menu does.

Alternatively, make a kill/reload of the terminal window follow the same discovery process as "first launch" and not just go with whatever powershellexepath is set to.

Side bug
If you set powershellexepath as a workspace setting, it doesn't get updated, so it is "sticky" and even if you toggle to another console it keeps coming back to whatever is set in workspace setting. This kind of meets my criteria, except I can't toggle to another version of powershell quickly without commenting it out if I want to test xplat compatability or something.

That aside, it does seem to work on a workspace-level as long as I don't kill the window, so if the above bug is fixed I think we are good to go and can close this.

EDIT: I can record a screengif of the behavior if you can't repro. Thanks!

[JustinGrote]

Workaround

Add this to your Microsoft.VSCode_profile.ps1

#Todo: Detect Insiders vs. regular vscode
if ($isWindows) {
    $vscodeSettingsPath = "$($env:APPDATA)\Code - Insiders\User\settings.json"
} else {
    $vscodeSettingsPath = "$HOME/.config/Code - Insiders/User/settings.json"
}
$PowershellExePath = (Get-Process -id $pid).path -replace '\\','\\'
$PowershellExePathRegex = '(?<="powershell\.powerShellExePath": ")(.+)(?=")'
$vscodeSettings = gc -raw $vscodeSettingsPath
$vscodeSettings -replace $PowershellExePathRegex,$PowershellExePath | Out-File -Encoding UTF8 $vscodeSettingsPath
Remove-Variable PowershellExePath,PowershellExePathRegex,vscodeSettingsPath,vscodeSettings

This will basically do the behavior I described above, and then whatever you set powershell.powershellDefaultVersion to will be sticky to that project, and stay that way if you terminal and reload a session. You can set a global user setting, as well as a per-workspace override, and it works fine. If you decide to switch session mid-session, will still be fine.

You will get a nag message on startup if it had to switch runtimes that the runtime had switched and ask if you want to restart it, just ignore it.

[JustinGrote]

Recommended Fix

@TylerLeonhardt There should be a call to changePowershellExePath here:

vscode-powershell/src/session.ts

Lines 293 to 295 in 0388343

	if (powerShellDefaultVersion) {
	powerShellExePath = powerShellDefaultVersion.exePath;
	} else {

[SydneyhSmith]

@JustinGrote thanks for providing all this info and proposed work around...we just discussed this and the possible ramification and are worried that this would cause unexpected behavior when choosing a version from the session menu...you are correct though in that we need to make a fix to ensure that the default version specified in the settings is being used for any session not started by the session menu

[JustinGrote]

Following up on this item:

Recommended Behavior

• Stop using powershellExePath directly, instead use the PowershellDefaultVersion references, found in this order:

1. If powershellExePath is defined (for backwards compatibility), use that above all else, however going forward this setting would not be ever set by the extension directly, it would purely be used as a "force override"
2. If PowershellDefaultVersion is defined in workspace settings.json
3. If PowershellDefaultVersion is defined in settings.json
4. If PowershellDefaultVersion is not defined in either settings.json, prefer the first listed version defined in powerShellAdditionalExePaths (workspace trumps global settings.json)
5. If none of the above defined, use default system "hints" to find Powershell Core and Windows Powershell, and then select any found in decending order of version (e.g. 6 trumps 5.1 if installed), and of architecture (x64 trumps x86)

Also: Powershell: Show Session Menu should not update any settings.json, it should be considered a command used for temporary switch.

Optionally, a toast could pop up asking if you want to set this as your default powershell, and if so, it will update PowershellDefaultVersion only, do not touch powershellExePath, however I think it's reasonable to instead say to update your user settings if you want to make a particular one persistent.

This is a much better design and flows more fluidly. It would also let a project maintainer specify a powershell version (e.g. "Windows Powershell (x64)" but leave it up to the developer as to where the actual installation resides (which they can override with PowerShellAdditionalExePaths)

Is this acceptable? I'd take a shot at a PR if there's a chance it might get accepted.

[rjmholt]

The only part that might be contentious is defaulting to an additionally configured PowerShell first. Otherwise this looks good.

I recently did a fair amount of work on the PowerShell discovery side, so this would dovetail nicely with that. It does mean that the code in preview is quite different, but now that we've committed to getting preview changes out as stable by January, I think it's safe to just make those changes there.

[JustinGrote]

@rjmholt the thinking would be is that, going forward, if you specified PowershellExePath in your settings, you explicitly wanted a particular powershell, say you downloaded a latest preview or test build to a custom folder.

This would also support backwards compatibility as existing users would be "sticky" to the version of powershell they are already using, but net-new computers and installs will not have this set in their settings and operate normally. Very clear instructions in the release notes would mention that you should remove PowershellExePath from your settings if upgrading, or maybe a first-run-after-upgrade toast warning that has an "OK" to do that for you.

[JustinGrote]

Also if you were referring to PowershellAdditionalExePaths, the idea is that normally this wouldn't be populated at all either and it would use the default Powershell discovery. A user would populate this to add their own EXEs, and those would be preferred in resolution order as the user would have to explicitly add them. However, the user could specify them here, but still specify PowershellDefaultVersion differently on a per-workspace basis, so it is not the be-all-end-all that it would always take precedence, only if PowershellDefaultVersion is not set to a preferred discovery name.

[rjmholt]

So there are a couple of considerations I think:

• Currently we look for default installations in a particular order and then for the configured addtional exe paths:

vscode-powershell/src/platform.ts

Lines 135 to 156 in fcadea5

	/**
	* Iterates through PowerShell installations on the machine according
	* to configuration passed in through the constructor.
	* PowerShell items returned by this object are verified
	* to exist on the filesystem.
	*/
	public *enumeratePowerShellInstallations(): Iterable<IPowerShellExeDetails> {
	// Get the default PowerShell installations first
	for (const defaultPwsh of this.enumerateDefaultPowerShellInstallations()) {
	if (defaultPwsh && defaultPwsh.exists()) {
	yield defaultPwsh;
	}
	}
	// Also show any additionally configured PowerShells
	// These may be duplicates of the default installations, but given a different name.
	for (const additionalPwsh of this.enumerateAdditionalPowerShellInstallations()) {
	if (additionalPwsh && additionalPwsh.exists()) {
	yield additionalPwsh;
	}
	}
	}

• Using that, you can select the default PowerShell for startup by setting the PowerShellDefaultVersion:

vscode-powershell/src/session.ts

Lines 370 to 380 in fcadea5

	if (this.sessionSettings.powerShellDefaultVersion && this.sessionStatus === SessionStatus.NeverStarted) {
	for (const pwshExe of this.powershellExeFinder.enumeratePowerShellInstallations()) {
	if (pwshExe.displayName === this.sessionSettings.powerShellDefaultVersion) {
	return pwshExe.exePath;
	}
	}
	// Default PowerShell version was configured but we didn't find it
	this.log.writeWarning(
	`Could not find powerShellDefaultVersion: '${this.sessionSettings.powerShellDefaultVersion}'`);
	}

• We made the choice to prefer a default installation by default (i.e. if you hadn't set a DefaultPowerShellVersion) on the basis that other configured PowerShells are additional, and that you can pick them as a default anyway. Also, as you say, the default installation paths are maintained, so we always know where to look. Although currently, if you configure an additional exe path and there's nothing there, we'll skip it (I believe we don't emit a warning currently but should).

• I think configuring PowerShell exe paths in user rather than workspace settings might also have been a deliberate choice for security purposes, similar to VSCode's exe path settings.

So I think some of this work has already been done in #2238. Making exe paths workspace settings rather than user settings is something we should avoid if VSCode does, but we make that work by allowing configuration of your default PowerShell as a workspace setting.

The remaining things I'm thinking we should work out are:

• Stickiness: the extension should start the session with the PowerShell you used last maybe (but how does that interact with the DefaultPowerShellVersion)
• PowerShell naming (should you be able to override the name of a default with additionalExePaths, or should they be considered reserved?)
• PowerShell versions. One thing @TylerLeonhardt and I have discussed with @joeyaiello and @SydneyhSmith is being able to give better names to PowerShell installations based on their versions. That slows down startup but gives you a better idea of which PowerShell is which. It would also mean we could meaningfully pick up and name PowerShells from the PATH.
• Deprecation of powerShellExePath, since additionalExePaths is the right way to both get a new PowerShell into the picker and to default to it at startup.

[JustinGrote]

Stickiness: the extension should start the session with the PowerShell you used last maybe (but how does that interact with the DefaultPowerShellVersion)

All things being default, I think stickiness should operate if and only if no workspace or user settings have been defined, otherwise it should follow the settings priorities as defined above. This could be enhanced with a command task "Powershell: Remember Current Shell" task or something that simply sets the user or workspace setting as appropriate, thus making it "sticky"

PowerShell naming (should you be able to override the name of a default with additionalExePaths, or should they be considered reserved?)

I think you should, for the reason that a project maintainer can specify in the repository workspace settings file that it should default to "Powershell 6 (x64)" or whatever, and then I can override that in my user settings depending on if I installed it via chocolatey, scoop, downloaded as zip to a custom path, etc.

However for Powershell 5.1 I suppose an exception could be made that the "Program Files" ones are considered hard coded/reserved, but for Powershell Core it should be flexible until such time there's an "In-box" option for powershell core on windows/etc.

Powershell Versions

A "standard" naming convention that pulls from the EXE assembly info or something like that would be nice, e.g. Powershell [PSEdition] [Version] [Architecture] with version being loose (e.g. if '6' is specified, use whatever the highest version is available in the path list that matches 6. If 6.1.2.1999-preview5 is specified, use that specific version)

Deprecation of powerShellExePath

Agreed, upon looking into this some more, probably don't need to support it for backwards compatibility, just make it ineffective.

[TylerLeonhardt]

Hey @JustinGrote - I realized that you opened this issue before the major refactoring that @rjmholt did in the last preview extension. We no longer depend on powerShellExePath first for anything. It's purely a fallback for backcompat. You should be able to use the setting PowershellDefaultVersion at the workspace level and get the desired behavior.

Have you given that a try? Is there a gap that you're seeing?

[JustinGrote]

We no longer depend on powerShellExePath first for anything

@TylerLeonhardt I don't think this statement is accurate at least with the Nov 1st Powershell Preview, were his changes incorporated in that release?

Whenever you switch shells with the lower right or the session menu command, it updates PS Exe Path at the usersettings level, and that seems to always override whatever I might have defined.

I also did the following tests with the following session menu for reference:

1. Remove powershellExePath and PowershellDefaultVersion at both user and workspace level. Get this prompt, ignore it
   

2. Start terminal session, 5.1 (x64) comes up. powershellExePath still blank

3. Manually set PowershellDefaultVersion user setting to "Powershell Preview (x64)" and reload window.

4. Integrated terminal is still 5.1 x64, all settings still the same

5. Repeat with workspace, no dice.

6. Manually update powershellExePath and reload, it is now on Preview.

7. Blank out again

8. Try setting workspace PowershellDefaultVersion to "Windows Powershell (x64)"

9. Reload window, no effect.

10. Toggle it with session menu and reload window, it now works but that's because powershellExePath was populated.

[TylerLeonhardt]

Thanks for the details @JustinGrote! This sounds like a bug. We did the work to discover PowerShell exes but we still needed to hook that up to the session mention and some other inner stuff it seems...

[TylerLeonhardt]

Alright, I have a PR out to fix this bug by removing powerShellExePath from everything and showing a notification to help the user with the transition:
#2099

[JustinGrote]

@TylerLeonhardt I think you meant #2304