Fix shell-integration-features being ignored with manual shell integration#5048
Fix shell-integration-features being ignored with manual shell integration#5048mitchellh merged 4 commits intoghostty-org:mainfrom liby:fix-shell-integration-features-none
Conversation
jparise
left a comment
jparise
left a comment
There was a problem hiding this comment.
Note that the previous behavior was intentional: when shell integration is disabled, we didn't want to add anything to the shell environment.
I apologize for not linking the issue in the PR description. This PR is actually implementing the solution proposed by @mitchellh in #5046, where he stated:
I'll update the PR description for better tracking. |
Got it. I think that makes sense. If someone doesn't want (automatic) shell integration and doesn't want the environment variables, they can still use: I think the config documentation for
... which reads more ambiguously now that |
Thanks for the feedback! I've updated the documentation for Please let me know if the updated documentation looks good to you. |
jparise
left a comment
jparise
left a comment
There was a problem hiding this comment.
Just a couple more suggestions, but looks good to me overall!
mitchellh
left a comment
mitchellh
left a comment
There was a problem hiding this comment.
One small but important change and it looks good. Thank you!
Add a unit test to prevent regressions in our failure state. For example, we always want to set GHOSTTY_SHELL_FEATURES, even if automatic shell integration fails, because it's also used for manual shell integration (e.g. ghostty-org#5048).
Add a unit test to prevent regressions in our failure state. For example, we always want to set GHOSTTY_SHELL_FEATURES, even if automatic shell integration fails, because it's also used for manual shell integration (e.g. ghostty-org#5048).
Add a unit test to prevent regressions in our failure state. For example, we always want to set GHOSTTY_SHELL_FEATURES, even if automatic shell integration fails, because it's also used for manual shell integration (e.g. ghostty-org#5048).
Add a unit test to prevent regressions in our failure state. For example, we always want to set GHOSTTY_SHELL_FEATURES, even if automatic shell integration fails, because it's also used for manual shell integration (e.g. #5048).
Add a unit test to prevent regressions in our failure state. For example, we always want to set GHOSTTY_SHELL_FEATURES, even if automatic shell integration fails, because it's also used for manual shell integration (e.g. ghostty-org#5048).
4 participants
Descriptions
The code was short-circuiting the shell integration setup when
shell-integration = none, which prevented the feature environment variables from being set. These environment variables are needed even for manual shell integration to work properly.Changes
Extracted feature environment variables setup into a separate
setup_featuresfunctionModified the shell integration initialization to ensure features are set up even when
shell-integration = noneFixes #5046