fix: preserve FZF_TAB_MODULE_VERSION when module is already loaded (fix-up #555)
#559
+7
−4
Conversation
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
Previously, FZF_TAB_MODULE_VERSION was always unset on source. This caused the variable to be lost when re-sourcing fzf-tab.zsh, because zmodload won't reload an already-loaded module. Now the variable is only unset if the module is not loaded.
Contributor
Contributor
Author
Hi! Yes, both PRs fix the same issue but with different approach. I put the comparison between them in the description |
Owner
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Fixes #555 - a regression where
FZF_TAB_MODULE_VERSIONis lost when re-sourcingfzf-tab.zsh.The problem: #555 unconditionally unsets
FZF_TAB_MODULE_VERSIONon every source. However,zmodloaddoesn't reload an already-loaded module, so the boot callback that sets this variable never runs again. Result: the variable is permanently lost after re-sourcing.The fix: Only unset the variable when the module is not loaded. If it's already loaded, the existing value is valid and should be preserved.
Why this approach and a duplicate of PR #556?
The PR #556 proposes a different approach in resolving the same issue by unloading the module (
zmodload -u) before reloading. While that also solves the problem, this PR's approach is better because:unsetonlyThe conservative approach here preserves existing behavior and only changes what's strictly necessary to fix the bug.