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

fix: correctly identify powershell #1134

Merged
merged 6 commits into from
Jul 9, 2024
Merged

Conversation

mdonnalley
Copy link
Contributor

Correctly distinguish between powershell and cmd.exe

@W-11446819@

@cristiand391 cristiand391 self-requested a review July 8, 2024 17:14
src/config/config.ts Outdated Show resolved Hide resolved
Copy link
Member

@cristiand391 cristiand391 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left one suggestion to improve detection on linux/mac

cristiand391
cristiand391 previously approved these changes Jul 8, 2024
@cristiand391
Copy link
Member

QA notes:

setup:

linked this branch of oclif into plugin-autocomplete
win: patched oclif/core in sf install folder

general:
SHELL still takes precedence. This is usually set at the system-level, can be modified in terminals/multiplexers when needed

mac:
🟡 powershell detected:
because SHELL is set by default, the only way to make oclif detect powershell correctly is to make your term/multiplexer unset it/set to SHELL=powershell. So, the current behavior of detecting powershell on mac/linux doesn't change.

win:
✅ powershell detected
SHELL isn't set and os.userInfo().shell is always null:
https://nodejs.org/api/os.html#osuserinfooptions

Returns information about the currently effective user. On POSIX platforms, this is typically a subset of the password file. The returned object includes the username, uid, gid, shell, and homedir. On Windows, the uid and gid fields are -1, and shell is null.

so oclif does the process.title check.
I can run sf autocomplete alone instead of sf autocomplete powershell and get valid pwsh instructions.

Screenshot 2024-07-08 at 4 11 45 PM

here I ran sf autocomplete --dev-debug on windows's cmd to verify config.shell value is still cmd.exe (this is also set by the process.title check b/c the COMPSEC one has lower precedence.
Screenshot 2024-07-08 at 4 12 35 PM

cristiand391
cristiand391 previously approved these changes Jul 8, 2024
@cristiand391 cristiand391 merged commit bcec7df into main Jul 9, 2024
84 checks passed
@cristiand391 cristiand391 deleted the mdonnalley/identify-pwsh branch July 9, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants