Mark variables in zsh integration functions as local by matt-sorenson · Pull Request #290298 · microsoft/vscode · GitHub
Skip to content

Mark variables in zsh integration functions as local - #290298

Merged
Anthony Kim (anthonykim1) merged 6 commits into
microsoft:mainfrom
matt-sorenson:matt-sorenson/zsh-globals/1
Oct 15, 2026
Merged

Anthony Kim (anthonykim1) merged 6 commits into
microsoft:mainfrom
matt-sorenson:matt-sorenson/zsh-globals/1

Conversation

@matt-sorenson

Copy link
Copy Markdown

I run zsh with setopt warn_create_global and got annoyed at the warnings.

I moved this updated file into my resources\app\out\vs\workbench\contrib\terminal\common\scripts folder of my vscode instance and verified I no longer got warnings about global variables and observed no strange behavior.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 09:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused declarations prevent unintended globals without changing the integration logic.

0 open findings

What changed in this PR

Marks temporary zsh shell-integration variables as function-local, preventing warn_create_global warnings and global-state pollution.

Changes:

  • Localizes the environment-cache loop index.
  • Localizes environment-reporting loop and value variables.
File Description
src/​vs/​workbench/​contrib/​terminal/​common/​scripts/​shellIntegration-rc.zsh Adds local declarations for temporary zsh variables.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@anthonykim1 Anthony Kim (anthonykim1) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you!

@anthonykim1
Anthony Kim (anthonykim1) merged commit 4151cab into microsoft:main Oct 15, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants