-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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: Shell completions are broken when login shell is different than currently active shell #1884 #1969
Conversation
Hey, can you tell us more about how it works? |
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 see. Thank you, this change LGTM. I'm holding off with merge for other maintainers to take a look.
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.
Looks good to me!
FWIW, we should look into backporting this to v2 as well. What do you think? |
I think this is worth backporting, yeah |
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.
@imransolanki Thanks !!!!
This actually doesn't fix anything, but breaks printing descriptions in I think this is also a lesson for us, maintainers of this module, to not trust that contributors did actually test their changes. Before this PRScreen.Recording.2024-10-28.at.02.06.53.movAfter this PRScreen.Recording.2024-10-28.at.02.08.49.mov |
What type of PR is this?
What this PR does / why we need it:
This is a fix to bug- Shell completions are broken when login shell is different than currently active shell #1884
$0
as an env variable instead ofSHELL
as$0
gives the currently active shellWhich issue(s) this PR fixes:
Fixes the bug- Shell completions are broken when a login shell is different than currently active shell #1884
Special notes for your reviewer:
Testing
Updated the existing test case.
Release Notes