-
Notifications
You must be signed in to change notification settings - Fork 265
feat: windows support pwsh 7, and push environment in the shell #142
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
base: main
Are you sure you want to change the base?
Conversation
ekzhang
left a comment
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!! Do you mind removing the last paragraph of the comment above that line that talks about the powershell not working?
Also adding powershell\6 as a location for v6, then will merge
|
powershell does not work now, so the comment above that line that talks about the powershell not working is valuable. powershell 6 can not be installed in my computer, don't know if it works. |
|
If Powershell does not work, then what is this change doing? |
|
This only applies to PowerShell 7 installed manually; the built-in PowerShell that comes with Windows still has issues. |
| // Inherit all environment variables from the current process. | ||
| // This ensures PATH and other important variables are preserved. | ||
| for (key, value) in std::env::vars() { | ||
| command.env(key, value); |
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.
Can you try with removing this line? It shouldn't be necessary, child processes inherit from their parent process by default https://doc.rust-lang.org/beta/std/process/struct.Command.html
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 test, without this, $PATH does not have claude and others commands.
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.
Hmm, that's very strange. I don't think we can merge this without understanding why PATH is being set strangely though, it may cause other edge cases to surface. Perhaps PowerShell is overriding your PATH on startup?
It works at my Windows 11 PC.
