ohmyzsh / ohmyzsh

🙃 A delightful community-driven (with 2,400+ contributors) framework for managing your zsh configuration. Includes 300+ optional plugins (rails, git, macOS, hub, docker, homebrew, node, php, python, etc), 140+ themes to spice up your morning, and an auto-update tool that makes it easy to keep up with the latest updates from the community.
https://ohmyz.sh
MIT License
173.95k stars 25.92k forks source link

Plugin subdirectories that contain completion files are not loaded #10412

Open felipecrs opened 3 years ago

felipecrs commented 3 years ago

Describe the bug

I have my own completions set at $ZSH_CUSTOM/plugins/my-completions/src and a my-completions.plugin.zsh to add src to fpath.

However, while the completions for the built-in plugins of OMZ seems to be working fine, my completions at my custom simply does not get loaded.

I have to manually run compinit (or add it after the source .oh-my-zsh.zsh, but it brings some consequences like not leveraging cache).

Steps to reproduce

Here is my .zshrc:

https://github.com/felipecrs/dotfiles/blob/master/home/dot_zshrc

and here is my-completions:

https://github.com/felipecrs/dotfiles/tree/master/home/dot_oh-my-zsh/custom/plugins/exact_my-completions

both deno and volta does not work.

Expected behavior

It's expected to simply work, without me having to re-run compinit.

Screenshots and recordings

https://user-images.githubusercontent.com/29582865/141393793-2082836e-8777-44fc-a0ca-34aecb23ab58.mp4

OS / Distro

Ubuntu 20.04

Zsh version

5.8

Terminal emulator

Windows Terminal

If using WSL on Windows, which version of WSL

WSL2

Additional context

In the zsh-completions install guide for OMZ, they suggest adding autoload -U compinit && compinit but I guess that was not supposed to be needed.

And yes, I already tried to disable everything in my zshrc to isolate the issue, and it does not work.

mcornella commented 3 years ago

The compinit happens before the plugin is loaded, so the $fpath modification happens afterwards. You'd need to manually call compinit, or hack around it by autoloading the files and running compdef after that.

The easier solution though is to move them outside the src folder.

felipecrs commented 3 years ago

Shouldn't we switch this order? I mean, can't we make compinit be called only after all plugins are loaded?

mcornella commented 3 years ago

This was a possibility, but a lot of plugins heavily use compdef which requires compinit to be loaded.

felipecrs commented 3 years ago

I could move my completions out of the src folder, but that does not solve the problem for zsh-completions. Should some alternative subfolders be supported, like src as zsh-completions, since it's a noticeably big project?

felipecrs commented 3 years ago

Or, perhaps, a subfolder called completions, and then we can propose zsh-completions to setup a symlink in their repo from completions to src.

mcornella commented 3 years ago

Thanks for keeping on this, I believe we could have logic to check whether there are subfolders with completion files (files named _*). Then the fpath= setting wouldn't be needed in the plugin. Note that in order for the directory to be considered a plugin there would still need to be either a $plugin_name.plugin.zsh file or a _$plugin_name file, even if they're empty.

felipecrs commented 3 years ago

Thanks for keeping on this, I believe we could have logic to check whether there are subfolders with completion files (files named _*).

Interesting approach. But I just wonder if it would not be too risky. Regardless, as it's just as dangerous as adding the root of the plugin folder to fpath itself automatically as it is being done today, so if there isn't any concern in doing what is being done today I guess there should not be any concern in checking subfolders too.

But there is also an alternate solution: OMZ could try to read a file named, let's say, completions-root.txt (name here is not the case), in the root of the plugins directory, which would have in its contents the path for the completions folder inside of the repository. Something like:

$ if [[ -f $plugin_root/completions-root.txt ]]; then
  fpath+="$plugin_root/$(cat $plugin_root/completions-root.txt)"
else
  fpath+="$plugin_root" # as it is being done today
fi

Note that in order for the directory to be considered a plugin there would still need to be either a $pluginname.plugin.zsh file or a $plugin_name file, even if they're empty.

I believe that is an okay compromise. As I'm doing on my dotfiles too:

https://github.com/felipecrs/dotfiles/tree/6e19c62a92107bf29f5d2022e062ec48f523d336/home/dot_oh-my-zsh/custom/plugins/exact_my-completions

felipecrs commented 3 years ago

My only concern though would be about the current contents of:

https://github.com/zsh-users/zsh-completions/blob/bebaa6126ede6bda698a6788c6cf3fa02ff1679c/zsh-completions.plugin.zsh

fpath+="${0:h}/src"

As fpath would be filled with the same entry twice (unless there is a builtin mechanism in ZSH to filter such case that I'm not aware of). I suppose that removing the contents of this file would be a non backwards compatible change for some already using zsh-completions without OMZ.

Perhaps, just adding an IF to check if the /src path is not currently in fpath would be good enough.

millbj92 commented 3 years ago

How does typescript solve this this tsconfig? Can we piggyback,?

felipecrs commented 3 years ago

How does typescript solve this this tsconfig? Can we piggyback,?

I'm not sure what's the relationship between this and typescript. Could you elaborate?

millbj92 commented 2 years ago

Tsconfig tells the assembler in what order to load, how to load, and what not to load. Also there are options to load but ignore exceptions. I'm sorry I didn't read the entire thread and it's been a minute, but it seems like this may be a scenario where something like this could be useful? Tbh something like this could be useful for the entire plugin system. I know omz has a great config file, but it can, at certain times, accidentally load things in the wrong order, try to compile things that ought not be compiled, etc. I guess my main suggestion is structure. If you read a tsconfig you can see the structure there is pretty hard to mess up unless you're trying to.

felipecrs commented 2 years ago

A fix was proposed at https://github.com/ohmyzsh/ohmyzsh/pull/10565.