-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: cmd.exe detection heuristic #2226
Conversation
@@ -255,14 +262,17 @@ fn shlex_posix(executable: impl AsRef<Path>) -> String { | |||
} | |||
|
|||
/// Quote a path, if necessary, for safe use in `PowerShell`. | |||
fn shlex_windows(executable: impl AsRef<Path>) -> String { | |||
fn shlex_windows(executable: impl AsRef<Path>, shell: Shell) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just merge shlex_posix
and shlex_windows
, and pass in Shell
to toggle the behavior -- wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda like the separation between Posix and Windows, but I'm biased 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Summary
Follow up from discussion in #2223
Detect CMD.exe by checking if
PROMPT
env var is set on windows, otherwise assume it's PowerShell.Note, this will not work if user modifies their system env vars to include
PROMPT
by default or if they launch nested PowerShell from Command Prompt (e.g.Developer PowerShell for VS 2022
).Test Plan
Only tested locally, although we try to add some CI tests that specifically use CMD.exe
Command Prompt
Power Shell