-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
adds better windows shell support #5053
adds better windows shell support #5053
Conversation
if WINDOWS: | ||
return env.execute(self.path) | ||
if self._name == "powershell": | ||
args = ["-NoExit", "-File", str(activate_path)] |
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.
Subjective opinion: I like launching with -nologo
to not have to see
PowerShell 7.2.1
Copyright (c) Microsoft Corporation.
https://aka.ms/powershell
Type 'help' to get help.
each time.
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.
Given that it is cosmetic, I'd rather not add more arguments past the bare minimum, personally.
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.
Understood. It is definitely subjective on my part -- I like the "clean sheet" in front of me after typing poetry shell
, but typing cls
isn't a big deal.
In my "wish list" for a nicer Windows
if WINDOWS:
set_console_history_info()
|
I'm a little leery of doing too much -- That being said, I'd still consider the suggestions made by @jrobbins-LiveData if they are added to this PR and don't look to be a big maintenance burden, since they seem to be minor and relatively stable. |
I agree, @neersighted, regarding the minimalist approach. Of all my feedback, I think I'd prioritize As to command history support, technically perhaps that's more properly a Windows Terminal concern. The reason I bring it up here is that if one "manually" simply runs In a regular command prompt window (not Windows Terminal), one can address this problem in its Properties page in the spot I've circled |
@neersighted , perhaps there could be an optional "user script" hook (configured via poetry's config mechanism?) , that the internal |
uses subprocess.run() adds support for pwsh
@neersighted updated the PR. Only handled the changes that keep everything minimal. @jrobbins-LiveData Thanks for all the feedback, appreciate it. I know this doesn't cover your wish list, but there are some minor upgrades 😊. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
This change resolves a lot duplicated issues.
Resolves: #5050, Resolves #4996, Resolves #4993, Resolves #4560, Resolves #2793, Resolves #2156, Resolves #1642
There are not tests for activating shells and this doesn't require a change in documentation. This lines up Windows behavior with the documentation.