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

Support pwsh (PS Core) in Select Default Shell on Windows #72425

Closed
Tyriar opened this issue Apr 16, 2019 · 13 comments
Closed

Support pwsh (PS Core) in Select Default Shell on Windows #72425

Tyriar opened this issue Apr 16, 2019 · 13 comments
Assignees
Labels
feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Apr 16, 2019

image

Code pointer: https://github.com/Microsoft/vscode/blob/e4b5bccf9364a0f678606d5bdfc18800854bf678/src/vs/workbench/contrib/terminal/electron-browser/terminalService.ts#L134-L145

@Tyriar Tyriar added help wanted Issues identified as good community contribution opportunities feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors labels Apr 16, 2019
@Tyriar Tyriar added this to the Backlog milestone Apr 16, 2019
@Tyriar Tyriar self-assigned this Apr 16, 2019
@Tyriar
Copy link
Member Author

Tyriar commented Apr 16, 2019

FYI @TylerLeonhardt

@Tyriar Tyriar changed the title Support pwsh in Select Default Shell on Windows Support pwsh (PS Core) in Select Default Shell on Windows Apr 16, 2019
@roottool
Copy link

roottool commented Apr 17, 2019

Hi @Tyriar, Thank you for reviewing my PR. Can I try this issue again?
And can you check my recognition?

My recognition of this issue

I add the below between PowerShell and WSL Bash in expectedLocations.

'PowerShell Core' : [
 		`${process.env['ProgramW6432']}\\PowerShell\\6\\pwsh.exe`,
 		`${process.env['ProgramFiles']}\\PowerShell\\6\\pwsh.exe`,
 	],

Currently, the version of PowerShell Core is 6.2.
But I am worried that 6 in file path is changed by upgrading to 7.0.

@bergmeister
Copy link

I think this issue is more complex than it looks like:

  • What if the user hasn't got PowerShell Core installed? Should Code detect that and only show the option if possible or just handle it gracefully?
  • PowerShell Core is an MSI installer on Windows where the user can specify a non-default installation location. It is also cross-platform so paths on Linux will be different. We need to either get the installation location from the registry or just call pwsh as it is expected to be on the PATH variable. I assume this approach should then work for Linux as well.

@Tyriar
Copy link
Member Author

Tyriar commented Apr 17, 2019

@roottool that's the basics of it and I'd merge something like that.

@bergmeister anything on top of basic path detection would be great, if the path doesn't exist that's already handled, but pulling from the environment/registry are options to make this much more user friendly. FYI this command is only available on Windows as on Linux/Mac you normally configure your default shell via chsh and $SHELL.

@essentialexch
Copy link

Traditionally, on Windows you used %COMSPEC% for the command processor. That works in Windows, DOS, and OS/2.

For PowerShell and pwsh paths, I'd probably suggest looking at HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\PowerShell.exe and HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\pwsh.exe respectively. This would deal with the 6 vs. 7 issue that @roottool brought up.

I don't have WSL installed to see if WSL respects those also, but I'd guess it does. GIT does not.

I don't do enough js/ts to add these suggestions, but thought I'd add my 2 cents none-the-less.

Thanks.

@Tyriar
Copy link
Member Author

Tyriar commented Apr 17, 2019

The terminal used to default to COMSPEC, but ended up moving off it I think because everything assumes that's cmd.exe anyway so people don't change unlike $SHELL.

@bergmeister
Copy link

The PowerShell Core MSI adds itself to the PATH environment variable, could we just simply call pwsh?

@essentialexch
Copy link

We can do that for all of those, except git bash (and I am aware that some people choose the option to have it added to their path - I for one, did not). I had presumed from @Tyriar 's earlier comment that he didn't want to do that.

@roottool
Copy link

roottool commented Apr 18, 2019

I tried while referring to @swngdnz's suggestion.

I confirmed that I succeeded in getting the file path from the registry.
getRegistoryData

I used alwaysUnderlineAccessKeys function in src\vs\workbench\services\accessibility\node\accessibilityService.ts for reference.
I added _getAppPathFromRegistry function in src\vs\workbench\contrib\terminal\electron-browser\terminalService.ts due to get the file path from the registry.

I will create PR, but I have an one question.
@swngdnz suggested looking at HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\PowerShell.exe for PowerShell's path.
Is the change included in PR?

Thanks.

@Tyriar
Copy link
Member Author

Tyriar commented Apr 18, 2019

@roottool yeah that would be great! We'll also need verify it works on a custom pwsh path.

@roottool
Copy link

I installed PowerShell Core to C:\.
installpwshToACustomPath

I confirmed that the custom pwsh path was registered to HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\pwsh.exe.
CustomPath
(既定) means Japanese (Default).

I verified that _getAppPathFromRegistry function works on the custom pwsh path.
getpwshPath

And I checked that pwsh of the custom pwsh path was executed on VSCode.
ExecutepwshOnVSCode

@roottool
Copy link

@swngdnz I verified that we can use PowerShell's path which is got from registry.
In conclusion, we can't use it now.

First, PowerShell's path is registered in HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\PowerShell.exe as %SystemRoot%\system32\WindowsPowerShell\v1.0\PowerShell.exe.
PowerShellPathOfRegistry

But VSCode can't recognize %SystemRoot%\system32\WindowsPowerShell\v1.0\PowerShell.exe as PowerShell's path.
CanNotFindPowerShell

Because VSCode can't exchange from %SystemRoot% to a path of system root.
We have to implement a logic to convert from %SystemRoot% to a path of system root using process.env['SystemRoot'].

At the end, I installed WSL.
But WSL's path(%SystemRoot%\system32\bash.exe or %SystemRoot%\system32\wsl.exe) is not registered in registry.
Therefore, there is able for only pwsh within the Default Shell group to acquire a file path from registry.
However only while pwsh's path in registry does not use %SystemRoot%.

@essentialexch
Copy link

sorry for the trouble!

but I think pwsh was the important one anyway.

@Tyriar Tyriar modified the milestones: Backlog, May 2019 May 11, 2019
@Tyriar Tyriar added the verification-needed Verification of issue is requested label May 11, 2019
@Tyriar Tyriar closed this as completed May 13, 2019
@alexr00 alexr00 added the verified Verification succeeded label May 29, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants