Skip to content

Fix zsh and bash shell integration when using set -u#185425

Merged
Tyriar merged 4 commits intomicrosoft:mainfrom
demccormack:185324
Jul 31, 2023
Merged

Fix zsh and bash shell integration when using set -u#185425
Tyriar merged 4 commits intomicrosoft:mainfrom
demccormack:185324

Conversation

@demccormack
Copy link
Contributor

Fixes #185324

Some users (like me) use the set -u aka set -o nounset option in their ~/.bashrc or ~/.zshrc. This causes shell commands to fail if there are any unset variables, which gives the user some protection against making silly mistakes. Currently, having this option set causes VS Code shell integration to

  • print lots of warning messages but still mostly work (in bash), or
  • fail completely (in zsh)

The fix is to use parameter expansion to remove the problem variables if they are unset.

This is my first PR to this repo. I don't think we can test shell scripts, but let me know if I'm wrong.

@demccormack
Copy link
Contributor Author

@microsoft-github-policy-service agree

@Tyriar Tyriar added this to the August 2023 milestone Jul 29, 2023
@Tyriar Tyriar enabled auto-merge July 29, 2023 22:06
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @demccormack and sorry about the delay. The change looks good, I'm not sure if it's complete even with my little change as there are a few variables that may not be set still like VSCODE_INJECTION but I'll move ahead with this and we can fix stragglers in a different PR.

@Tyriar Tyriar merged commit 51aefd9 into microsoft:main Jul 31, 2023
@demccormack
Copy link
Contributor Author

Thanks very much @Tyriar! I'll be using it and can open another PR if there are other variables that need the same treatment 🙂

@demccormack demccormack deleted the 185324 branch July 31, 2023 20:01
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell integration fails with set -u

3 participants