Add PS Core in select default shell on windows#72425#72689
Add PS Core in select default shell on windows#72425#72689Tyriar merged 14 commits intomicrosoft:masterfrom roottool:add-PS-Core-in-Select-Default-Shell-on-Windows#72425
Conversation
|
|
| useWSLexe = true; | ||
| } | ||
|
|
||
| const Registry = await import('vscode-windows-registry'); |
There was a problem hiding this comment.
Let's move the await into _getShellPathFromRegistry and make that function async as well
| * @param shellName The shell name to get the executable file path | ||
| * @returns The executable file path of shell or `'ShellNotFound'` | ||
| */ | ||
| private _getShellPathFromRegistry(Registry: typeof import('vscode-windows-registry'), shellName: string): string { |
There was a problem hiding this comment.
I think this function should be a little more targetted to just pwsh for now and then return either [] or [ 'path' ]
| let shellPath; | ||
|
|
||
| try { | ||
| shellPath = Registry.GetStringRegKey('HKEY_LOCAL_MACHINE', `SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths\\${shellName}.exe`, ''); |
There was a problem hiding this comment.
This could be simplified:
try {
return (grab from reg);
} catch () {
}
return [];
There was a problem hiding this comment.
Registry.GetStringRegKey() function returns string or undefined.
https://github.com/Microsoft/vscode/blob/3a12b7ac2efd2f1f01a646ef5272313ad50bf618/src/typings/vscode-windows-registry.d.ts#L8
Therefore, this function needs a logic to convert from undefined to [].
But I referred your review when I fixed the code.
Thanks.
There was a problem hiding this comment.
I realized that using ! makes the logic unnecessary. So I deleted the logic.
sorry for the trouble!
| const Registry = await import('vscode-windows-registry'); | ||
|
|
||
| try { | ||
| return [Registry.GetStringRegKey('HKEY_LOCAL_MACHINE', `SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths\\${shellName}.exe`, '')!]; |
There was a problem hiding this comment.
While this does compile, you'll still want to put it in a variable and check for undefined just in case as ! tells the typescript compiler to don't type check this making it less safe. If the key didn't exist (for some reason) this would return [undefined] which could break whatever is consuming the function.
There was a problem hiding this comment.
Thank you for review,
That was a failure due to my lack of understanding.
I fixed this code.
Tyriar
left a comment
Tyriar
left a comment
There was a problem hiding this comment.
Looks great! I'll verify and merge in the May release 🙂
4 participants
Issue number
#72425
Description
This PR adds pwsh(PowerShell Core) in Select Default Terminal UI when VSCode is operated on Windows.
A file path of pwsh is got from registry.
Screenshot
Here is a screenshot of a pattern with pwsh installed with a custom path.
Note
I wrote the below in added a function.
We will find
ShellNotFoundin a shell path of Select Default Terminal UI iffile://ShellNotFoundexists and added a function can't get a shell path.Because I can not write the below by #72601.
I suggest to change from
'ShellNotFound'to''if #72601 is fixed.