Add -Login startup option #2400
Conversation
src/session.ts
Outdated
|
|
||
| private isLoginShell(pwshPath: string): boolean { | ||
| try { | ||
| cp.execFileSync(pwshPath, ["-Login", "-NoProfile", "-NoLogo", "-Command", "exit 0"]); |
There was a problem hiding this comment.
What does this throw if it's not the login shell?
Also, did your change to allow PowerShell to be a login shell make it in PS 6.2?
There was a problem hiding this comment.
What does this throw if it's not the login shell?
The child_process docs say:
If the process times out or has a non-zero exit code, this method will throw an Error that will include the full result of the underlying child_process.spawnSync().
So when the -Login switch is not recognised and PowerShell exits with a non-zero exit code, node throws, we catch and return false.
Also, did your change to allow PowerShell to be a login shell make it in PS 6.2?
No, only in 7 I think. But since you can install PowerShell to any path and configure VSCode to pick that up, we have no way of knowing what version a PS executable is, so are forced to run it to check.
I'll add a comment.
I should also say I haven't tested this, since I've been working on Windows. Why it's still in draft.
There was a problem hiding this comment.
Sorry I meant that I think -Login existed in 6.2 but it was a no-op so that might be a false positive... Or maybe that was just -l I can't remember.
There was a problem hiding this comment.
Ohhh, I see! We might need @SteveL-MSFT's thoughts there. I think there was indeed a -l in either 6.2 or a 7 preview, although it possibly was short for -LoadProfile
There was a problem hiding this comment.
@rjmholt is correct, 6.2 only supported -l as short for -loadprofile making it "compatible" with Unix expecting -l to be accepted. -login will fail on 6.2
There was a problem hiding this comment.
This is in 6.2.2 @SteveL-MSFT . This is a weird error output, isn't it?
pwsh -l
Invalid argument '-l', did you mean:
-nologo
-noprofile
-windowstyle
-file
-executionpolicy
-settingsfile
-help
-removeworkingdirectorytrailingcharacter
There was a problem hiding this comment.
My memory is failing me. Looks like it was added in 7.0 but has been superceded by Rob's change to support -login: PowerShell/PowerShell#9528
|
Based on this comment, we should default the config setting to |
|
@rjmholt go ahead and review your own PR, I've made the changes lol |
|
LGTM! |
|
@rjmholt Now it's released. Thank you!! It is working as expected 🎉 |
PR Summary
Resolves #2384.
Adds a config setting to start the PSIC with
-Login.Needs testing on *nix platforms, so still WIP.
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready