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

nix-zsh-completions package and module support #11251

Merged
merged 2 commits into from
Nov 25, 2015

Conversation

spwhitt
Copy link
Contributor

@spwhitt spwhitt commented Nov 25, 2015

  1. Adds a package for nix-zsh-completions
  2. Configures zsh's fpath to include completions installed through Nix packages
  3. Adds an enableCompletion option ZSH module

I set up the enableCompletion option to also pull in the nix-zsh-completions package, under the assumption that anyone using completions on NixOS is probably interested in completions for the Nix utilities as well. If anyone doesn't like this, feel free to let me know and I'll remove that line, and people will have to add nix-zsh-completions to their systemPackages manually.

@jagajaga @offlinehacker @garbas

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @oxij, @edolstra and @ttuegel to be potential reviewers.

@@ -73,6 +73,14 @@ in
type = types.lines;
};

enableCompletion = mkOption {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion True as a default here is better :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say so :)

@jagajaga
Copy link
Member

👍

jagajaga added a commit that referenced this pull request Nov 25, 2015
nix-zsh-completions package and module support
@jagajaga jagajaga merged commit 451858b into NixOS:master Nov 25, 2015
@globin
Copy link
Member

globin commented Nov 25, 2015

see #11155 (enable shared site-function in pathsToLink)

@spwhitt
Copy link
Contributor Author

spwhitt commented Nov 26, 2015

@abbradar thanks - it must have worked for me because my desktop environment links /share. The actual required path is /share/zsh/$ZSH_VERSION/functions so I'm guessing I should just link all of /share/zsh since I won't be able to use $ZSH_VERSION inside pathsToLink - is this correct?

@abbradar
Copy link
Member

Strange, it works for me with /share/zsh/functions but I guess there's both that and $ZSH_VERSION path so we actually need four paths to add to pathsToLink. For $ZSH_VERSION -- what do you think of using lib.getVersion zsh to make this path static?

@spwhitt
Copy link
Contributor Author

spwhitt commented Nov 26, 2015

My system doesn't have /share/zsh/functions and if yours does it's either a bug in some other package or it needs to be added to $fpath as well... using lib.getVersion sounds brittle, simple formatting changes could break it. /share/zsh doesn't have anything we don't want to link anyway (in the zsh package), so linking the whole thing shouldn't effect performance. I argume we play it safe and link /share/zsh

@abbradar
Copy link
Member

nix-zsh-completions uses this path, for example. Also my fpath already contains needed paths, added by the very spwhitt@ff58711#diff-0740f57e63af61694d14796286cb9204R115 ^_^.

OK, let's link the whole thing -- shouldn't hurt.

@abbradar
Copy link
Member

Oh, sorry, it uses /share/zsh/site-functions -- sleepy me.

@abbradar abbradar added 0.kind: enhancement Add something new 8.has: package (new) This PR adds a new package 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 26, 2015
@nbp nbp added this to the 16.03 milestone Nov 26, 2015
@spwhitt spwhitt deleted the nix-zsh-completions branch December 17, 2015 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants