Conversation
.zi-unload resolved the Standard hook from $plugin verbatim:
(( ${+functions[${plugin}_plugin_unload]} )) && ${plugin}_plugin_unload
Two kinds of plug-in could never be reached.
A hyphenated repository, because ADR-0020's namespace rules require the shell
prefix to use underscores. `zsh-eza' can define `zsh_eza_plugin_unload' and
satisfy the standard, or define `zsh-eza_plugin_unload' and be unloadable by Zi,
but not both. That is the reported case.
A plug-in loaded by path, because such a plug-in is identified as `%<absolute
path>' and $plugin is the directory, so `/path/to/thing_plugin_unload' names
nothing. Found while adding unload coverage in z-shell/zd#119, and not in the
original report.
Try candidate names in order and call the first that exists: the literal name,
so plug-ins already defining it keep working; the underscore form; then the same
pair derived from the directory basename, which is the closest analogue of a
repository name for a path-loaded plug-in. Duplicates collapse through (u), and
only one function is called, since the Standard defines a single unload function
per plug-in.
The fix is additive. No name that resolved before stops resolving.
tests/unload-hook-dispatch.zsh loads and unloads two plug-ins, one plainly named
and one hyphenated, each defining only the name its own rules permit, and
asserts both hooks ran. Against the previous dispatch it reports `the unload
function of a path-loaded plug-in was not called: []'.
Closes #482
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.
Makes the Plugin Standard unload function reachable. Fixes the reported case and a second one found while building z-shell/zd#119.
The defect
$pluginwas used verbatim, so two kinds of plug-in could never be reached.Hyphenated repositories, the reported case. ADR-0020's namespace rules require the shell prefix to use underscores, so
zsh-ezacan definezsh_eza_plugin_unloadand satisfy the standard, or definezsh-eza_plugin_unloadand be unloadable by Zi. Not both.Path-loaded plug-ins, which were not in the report. Such a plug-in is identified as
%<absolute path>and$pluginis the directory, so the derived name is/path/to/thing_plugin_unload, which names nothing. Demonstrated while adding unload coverage:@zsh-plugin-run-on-unloadworked in both cases, which is why the gap stayed invisible: plug-ins using the callback were fine, and only the named-hook contract was broken.The fix
Try candidate names in order and call the first that exists:
Duplicates collapse through
(u), and exactly one function is called, since the Standard defines a single unload function per plug-in.The change is additive. No name that resolved before stops resolving.
Test
tests/unload-hook-dispatch.zshloads and unloads two plug-ins, one plainly named and one hyphenated, each defining only the name its own rules permit, then asserts both hooks ran. Against the previous dispatch:Registered in
zsh-n.yml, both path filters and a job. Full suite 21/21.Note on scope
This restores the hook's reachability. It does not change what the hook is expected to do, and it does not touch #113 or #483, which are separate unload defects. z-shell/zd#119 now asserts the four unload contracts that already worked, so a case for each of those can land with its fix.
Closes #482