Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow explicit zsh global completion activation #467

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

evanunderscore
Copy link
Collaborator

This repairs explicit global completion activation in zsh which was removed in #466.

I find this to be a useful feature if only for testing without needing to modify my local environment. It also allows users to add eval "$(activate-global-python-argcomplete --dest=-)" to their ~/.zshrc file if they would prefer to dynamically get the global completion script every time they start their shell.

The tests now test both methods, though running every test case for both activation methods is likely overkill and the tests could be refactored to only run one basic test for one of them.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3274413) 81.12% compared to head (3163855) 81.12%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #467   +/-   ##
========================================
  Coverage    81.12%   81.12%           
========================================
  Files           10       10           
  Lines          784      784           
========================================
  Hits           636      636           
  Misses         148      148           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kislyuk
Copy link
Owner

kislyuk commented Dec 10, 2023

Not sure if I fully understand why #466 broke explicit sourcing or why this fixes it, but happy to merge and test.

@kislyuk kislyuk merged commit 568d327 into kislyuk:develop Dec 10, 2023
24 checks passed
@kislyuk
Copy link
Owner

kislyuk commented Dec 10, 2023

Ah, I think I understand now.

So basically starting in #466 we are kinda abusing the zsh completion definition autoload mechanism a little bit. Autoload is intended to let us load our completion script once and register all our completer functions. Because we don't actually register completion definitions in our script but instead just call the completer directly and produce the completions themselves, there is no work being cached and the whole script will be run for the default completer (i.e. if no other completers are activated) the next time tab is pressed.

This is not a big deal because ultimately the shellcode itself is quite fast compared to loading and running Python, but ideally I'd like to figure out a way to follow zsh convention more closely by registering the completion definition/calling compdef when the autoloader calls us, not just when sourcing the file directly.

@kislyuk
Copy link
Owner

kislyuk commented Dec 10, 2023

Released in v3.2.1

@evanunderscore
Copy link
Collaborator Author

@kislyuk I'm not completely sure I understand how this works myself. The documentation says this (emphasis mine):

The file will be made autoloadable and the function defined in it will be called when completing names, each of which is either the name of a command whose arguments are to be completed or one of a number of special contexts in the form -context- described below.

Our script defines more than one function, so there's no way it could implicitly know which one to call. According to this answer, autoload will generally call a function from a script if it's the only thing defined:

The first time you run command which Zsh determines is an autoloaded function, the shell sources the definition file. Then, if there's nothing in the file except the function definition, or if the shell option KSH_AUTOLOAD is set, it proceeds to call the function with the arguments you supplied. But if that option is not set and the file contains any code outside the function definition (like initialization of variables used by the function), the function is not called automatically. In that case it's up to you to call the function inside the file after defining it so that first invocation will work.

So it sounds like autoload has special logic to call a function if it's the only thing in the script. What is not clear is whether it will parse the entire script again the next time you run it, or if it will just call the function. I'm not even sure how you would test it, since adding any global side effect like an echo would break the automatic function calling.

To the best of my understanding, the intention is not for you to call compdef explicitly in your completer. It feels like it wants you to define your completer on its own in one file, and any utility functions in other files on the fpath with an #autoload comment at the top.

However, it seems like calling compdef in the completer does work, since you're effectively just overwriting the entry that was automatically registered from the comment. So with this change, we will be invoked by the autoload mechanism the first time, but the reintroduction of the explicit call to compdef in this MR should overwrite that with the explicit registration of the function, and continue to recycle that function for all future completions. (This was also how it was working prior to #466, but the implicit and explicit compdef patterns didn't match after #463.)

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.

None yet

3 participants