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

zsh: remove search for installed completions #54

Closed
wants to merge 1 commit into from
Closed

zsh: remove search for installed completions #54

wants to merge 1 commit into from

Conversation

jpotier
Copy link
Contributor

@jpotier jpotier commented Sep 3, 2017

  1. Takes a lot of time: seriously, this takes 2 seconds at first launch, and like .25 second for the following launches;
  2. Is redundant with using oh-my-zsh, completions is set in there, works well, and does not lag as much (why?). For using the oh-my-zsh config, one must enable zsh first.

@dermetfan dermetfan mentioned this pull request Sep 3, 2017
@rycee
Copy link
Member

rycee commented Sep 3, 2017

I'm agnostic w.r.t. this but perhaps @uvNikita has opinions?

@jpotier
Copy link
Contributor Author

jpotier commented Sep 3, 2017

Yeah, I'm curious too

@uvNikita
Copy link
Collaborator

uvNikita commented Sep 3, 2017

Hmm, the thought behind this code was that the completions should work for users who want to use vanilla zsh:

  1. zsh is not enabled in the system
  2. zsh is enabled via Home Manager and enableCompletion is set to true
  3. oh-my-zsh is not enabled via Home Manager

This code was taken from NixPkgs programs.zsh module, which will be executed only if programs.zsh was enabled in the system configuration. However, I just tested the standard completions with the scenario described above and it works. In my case, fpath with such settings is set to:

/usr/local/share/zsh/site-functions /nix/store/<hash>-zsh-5.4.2/share/zsh/site-functions /nix/store/<hash>-zsh-5.4.2/share/zsh/5.4.2/functions

Which seems to be correct. Maybe zsh package was fixed at some point, but programs.zsh was not updated? In any case, I think we can delete this code from Home Manager, at least it works correctly on my setup.

Thank you for raising the issue!

@rycee
Copy link
Member

rycee commented Sep 3, 2017

Thanks @uvNikita (btw, check your Matrix chat)!

@jpotier Would you mind changing the first commit line to "zsh: remove search for installed completions" (module name + ":" + description in imperative mood). Also please wrap the body at 72 characters.

@jpotier jpotier changed the title Removing search for installed completions zsh: remove search for installed completions Sep 4, 2017
1. Takes a lot of time: seriously, this takes 2 seconds at first launch,
and like .25 second for the following launches;
2. Is redundant with using `oh-my-zsh`, completions is set in there,
works well, and does not lag as much (why?). For using the `oh-my-zsh`
config, one must enable zsh first.
@jpotier
Copy link
Contributor Author

jpotier commented Sep 4, 2017

Thanks @uvNikita for your comments. @rycee Commit title and body have been changed.

@uvNikita
Copy link
Collaborator

uvNikita commented Sep 4, 2017

I rephrased the commit message a bit and rebased into master 721f924. Thanks again for PR!

@uvNikita uvNikita closed this Sep 4, 2017
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