Restore aliases via 'builtin' consistently - #985
Open
yfwmaniish wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens zsh-syntax-highlighting.zsh’s alias restoration so it remains reliable even when a user has overridden/aliased the alias builtin, aligning restoration with the script’s existing defensive use of builtin.
Changes:
- Restore captured aliases by evaluating each
alias -Lline asbuiltin …(instead of a plainevalthat may invoke an overriddenalias). - Add explanatory comments referencing issue #972.
- Document the fix in
changelog.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| zsh-syntax-highlighting.zsh | Restores aliases via eval "builtin $line" for consistency and robustness against overridden alias. |
| changelog.md | Adds a HEAD entry describing the alias restoration fix (#972). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Restore the aliases we unned | ||
| eval "$zsh_highlight__aliases" | ||
| builtin unset zsh_highlight__aliases | ||
| # Restore the aliases we unned. |
Contributor
Author
There was a problem hiding this comment.
Good catch, thanks. Reworded to "Restore the aliases that were unaliased above (via 'builtin unalias')." in 49fa220.
Aliases were unaliased defensively with 'builtin unalias' before this script runs, but restored afterwards via a plain 'eval "$aliases"', which calls 'alias' rather than 'builtin alias'. If the user had aliased or otherwise overridden 'alias' itself, the script would wipe out their aliases and then silently fail to restore them. Fixes zsh-users#972.
Per review feedback on zsh-users#985: "we unned" was unclear/informal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
zsh-syntax-highlighting.zshunaliases user aliases defensively viabuiltin unaliasbefore setup, but restored them afterwards via a plaineval "$aliases", which invokesaliasrather thanbuiltin alias.alias(e.g. via a shell function), the script would successfully clear their aliases and then silently fail to restore them.eval "builtin $line"instead, matching the defensive pattern already used when capturing them.Fixes #972.
Test plan
make testpasses on this branch (exit 0). The 28not oklines in the output are all pre-existing# TODO "issue #NNN"known-failures unrelated to aliases; the harness's own alias-preservation sanity check ('alias -- +foo=bar' is preserved) passes.alias fooexits 1 — the alias is lost.alias fooprintsfoo=bar— the alias survives.