This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/proposal-make-it-easier-to-define-manpages-shell-completions/3491/6

I haven't updated any packages to use this yet as that will require the package to be rebuilt. I wasn't sure if it was worth it when there's no functional difference. If this gets merged I'll try to start adopting it any time I update a package that has manpages or shell completions.

I just tested what happens with a multi-output derivation, and it appears to work just fine, because it seems that share/man gets moved into the man output (if it exists) in a default preFixup hook, meaning my hook doesn't need to care about the possibility of a man output.

That said, I'm thinking I should probably just go ahead and use ${!outputMan} and ${!outputDevman} after all instead of relying on that preFixup hook.

I've updated it to use ${!outputMan} and ${!outputDevman} explicitly. I also went with ${!outputBin} for the shell completions, but I'm not sold on that one, maybe I should just keep that in $prefix.

The usage of installShellFiles as a function is because attrsets can't serialize to the environment normally, so it converts the structured attrs into a format the shell hook can consume.

FYI, since Nix 2.0 you can set __structuredAttrs to get what you want. It's only used two places in nixpkgs, but since nixpkgs requires nix 2.0 now anyways, I don't see why it shouldn't be used more.

__structuredAttrs is set on the whole derivation, not on individual nested attrsets, and its use completely breaks the default builder. Even if the default builder worked with it, requiring it to be set on the derivation would be a significant hurdle to using this on arbitrary packages. And of course Bash doesn't have any built-in tools for dealing with JSON so I'd have to add a dependency on something like jq and write a complicated jq script to transform it into something usable by Bash.

Updated again so the installShellFiles function adds a __toString attribute instead of immediately serializing it to a string. This way you can still query the structured shell completions in Nix code.

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/32

Updated again because I screwed up the handling of escaped characters in shell completion paths.

Added support for renaming shell completions, including automatically renaming zsh completions because apparently zsh requires the convention _foo and packages might not necessarily have it that way in the source.

As an example I updated the nixpkgs.exa derivation to use this, including manual renaming support. The diff looks like

diff --git a/pkgs/tools/misc/exa/default.nix b/pkgs/tools/misc/exa/default.nix
index 7c43638ea56..f51fe9e2982 100644
--- a/pkgs/tools/misc/exa/default.nix
+++ b/pkgs/tools/misc/exa/default.nix
@@ -1,5 +1,5 @@
 { stdenv, fetchFromGitHub, rustPlatform, cmake, perl, pkgconfig, zlib
-, darwin, libiconv
+, darwin, libiconv, installShellFiles
 }:
 
 with rustPlatform;
@@ -17,25 +17,18 @@ buildRustPackage rec {
     sha256 = "14qlm9zb9v22hxbbi833xaq2b7qsxnmh15s317200vz5f1305hhw";
   };
 
-  nativeBuildInputs = [ cmake pkgconfig perl ];
+  nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];
   buildInputs = [ zlib ]
   ++ stdenv.lib.optionals stdenv.isDarwin [
     libiconv darwin.apple_sdk.frameworks.Security ]
   ;
 
-  postInstall = ''
-    mkdir -p $out/share/man/man1
-    cp contrib/man/exa.1 $out/share/man/man1/
-
-    mkdir -p $out/share/bash-completion/completions
-    cp contrib/completions.bash $out/share/bash-completion/completions/exa
-
-    mkdir -p $out/share/fish/vendor_completions.d
-    cp contrib/completions.fish $out/share/fish/vendor_completions.d/exa.fish
-
-    mkdir -p $out/share/zsh/site-functions
-    cp contrib/completions.zsh $out/share/zsh/site-functions/_exa
-  '';
+  manpages = [ "contrib/man/exa.1" ];
+  shellCompletions = installShellFiles {
+    bash = { name = "exa"; path = "contrib/completions.bash"; };
+    fish = { name = "exa.fish"; path = "contrib/completions.fish"; };
+    zsh  = { name = "_exa"; path = "contrib/completions.zsh"; };
+  };
 
   # Some tests fail, but Travis ensures a proper build
   doCheck = false;

I'm not committing this change because it requires rebuilding exa even though the resulting file tree doesn't change.

An alternative solution would be to have the hook introduce shell functions such that the postInstall becomes something like:

installManPage contrib/man/exa.1
installShellCompletion --bash --name exa contrib/completions.bash
installShellCompletion --fish --name exa.fish contrib/completions.fish
installShellCompletion --zsh --name _exa contrib/completions.zsh

I.e., without needing to introduce new attributes in the derivation.

I rewrote this setup hook to vend installManPage and installShellCompletion functions instead, as suggested. I pushed it as a separate commit to the PR so we can see both versions, but if this PR is accepted I'll squash them together.

With this new version the patch to exa looks like

diff --git a/pkgs/tools/misc/exa/default.nix b/pkgs/tools/misc/exa/default.nix
index 7c43638ea56..d3f27b9814e 100644
--- a/pkgs/tools/misc/exa/default.nix
+++ b/pkgs/tools/misc/exa/default.nix
@@ -1,5 +1,5 @@
 { stdenv, fetchFromGitHub, rustPlatform, cmake, perl, pkgconfig, zlib
-, darwin, libiconv
+, darwin, libiconv, installShellFiles
 }:
 
 with rustPlatform;
@@ -17,24 +17,17 @@ buildRustPackage rec {
     sha256 = "14qlm9zb9v22hxbbi833xaq2b7qsxnmh15s317200vz5f1305hhw";
   };
 
-  nativeBuildInputs = [ cmake pkgconfig perl ];
+  nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];
   buildInputs = [ zlib ]
   ++ stdenv.lib.optionals stdenv.isDarwin [
     libiconv darwin.apple_sdk.frameworks.Security ]
   ;
 
   postInstall = ''
-    mkdir -p $out/share/man/man1
-    cp contrib/man/exa.1 $out/share/man/man1/
-
-    mkdir -p $out/share/bash-completion/completions
-    cp contrib/completions.bash $out/share/bash-completion/completions/exa
-
-    mkdir -p $out/share/fish/vendor_completions.d
-    cp contrib/completions.fish $out/share/fish/vendor_completions.d/exa.fish
-
-    mkdir -p $out/share/zsh/site-functions
-    cp contrib/completions.zsh $out/share/zsh/site-functions/_exa
+    installManPage contrib/man/exa.1
+    installShellCompletion --name exa contrib/completions.bash
+    installShellCompletion --name exa.fish contrib/completions.fish
+    installShellCompletion --name _exa contrib/completions.zsh
   '';
 
   # Some tests fail, but Travis ensures a proper build

I went ahead and squashed the commits together and force-pushed. No file changes, just git history.

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/38

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/45

the previous structured versions allowed to load completion while in nix-shell (which was great). The new one will work only after postInstall :(

@teto What's your actual use-case here? You're running buildPhase and then, what, manually instructing your shell to load the completions so you can use them when invoking the tool from the build folder? Is there a reason not to just run installPhase too at this point?

yes that's my usecase.

Running installPhase tends to fail as it won't be able to write to $out unless you change it (and let's not mention multioutput scenarios). Running installPhase can take some time too. And the installPhase is written so that it works after running a buildPhase but when in a nix-shell, most of the time you don't necessarily run buildPhase. For instance python enters develop mode.

@teto Is your goal to use this generically with arbitrary packages? Because if you're working on a specific package, surely you can just source the completion files from wherever you know they are.

Also if you're not running buildPhase then the structured approach wouldn't necessarily work for you anyway, because some packages may generate their completion files while building. Though admittedly that's pretty rare.

In any case, I liked the fact that the structured approach let you find the shell completions outside of the install process, but I have to admit the approach this PR currently takes is probably simpler to get people to use, more of a drop-in replacement for existing solutions, and also supports something the structured approach didn't, which is shell globbing.

If you have any ideas on how to regain the ability to source shell completions from within nix-shell, I'm all ears. Maybe we could do something like encourage people to define a phase like

installShellFilesPhase = ''
installManPage doc/foobar.1
'';

and we could configure this automatically to run after installPhase, and then you could do something special with nix-shell that redefines those shell functions such that you can just invoke installShellFilesPhase and it would source the completions.

One downside to this approach is anyone who doesn't use the separate installShellFilesPhase wouldn't be compatible with the automatic sourcing for nix-shell.

Something along these lines.
What matters is to make available the zsh/bash paths to the shell script. __structuredAttrs looked nice but apparenlty it's not good enough. I am thinking of bash arrays/parsable string. Sthg like makeFlags that could be named zshCompletionFiles for instances.

The problem is anything complex, people aren't going to use. With the current approach, it works pretty much the way people normally expect, except instead of having to remember where exactly the files get installed, they just use this shell function to install it (but it's otherwise done at the same time, in installPhase).

I liked the structured approach because it preserved structured data in a way you could take advantage of e.g. with what you're proposing, but it was definitely more complicated and harder for people to understand exactly what it's doing.

I'm not really sure how to get the best of both worlds. Like I said, we might possibly convince people to do something like write their shell script installations in a separate var installShellFilesPhase, but I'm not all that optimistic about it when the "obvious" solution is to just stick the shell functions into installPhase.

I just had a look at __structuredAttrs, what a shame it breaks builders (but the future is bright).

What's hard about defining :
zshCompletionFiles = [ "_exa=contrib/completions.zsh" ];
(btw do we really need the _exa ? this could be deduced from pname ?)
Then the installPhase could natively install the completion (rather than a setupHook).

It would allow myself or another to add some code later on for the nix-shell case.

(btw do we really need the _exa ? this could be deduced from pname ?)

Yes because the pname isn't necessarily the same thing as the executable name, and the package could install multiple executables.

What's hard about defining :
zshCompletionFiles = [ "_exa=contrib/completions.zsh" ];
[…]
Then the installPhase could natively install the completion (rather than a setupHook).

How do you get people to write this

zshCompletionFiles = [ "_exa=contrib/completions.zsh" ];
installPhase = ''
  installShellFiles $zshCompletionFiles
'';

when they could just write

installPhase = ''
  installShellFiles _exa=contrib/completions.zsh
'';

Especially since this approach doesn't even handle edge cases like paths that have spaces in them (Nix arrays get exposed to bash just as space-separated strings, and if you escape the elements of the array, you then need to pass the whole thing to eval to get it to handle the escapes/quotes properly, so that's actually eval installShellFiles $zshCompletionFiles).

making installShellFiles a default hook would make sense. Then you don't need to modify installPhase. modifying the installPhase can be tricky too (must remember to properly trigger the pre/post hooks)

As for the space default, I believe it's so rare that we could leave it as an edgecase until __structuredAttrs becomes a thing.

As for the space default, I believe it's so rare that we could leave it as an edgecase until __structuredAttrs becomes a thing.

Given that Bash doesn't have any support for JSON I'm not expecting to be using __structuredAttrs with the default builder any time soon.

At this point, what's stopping this PR from being merged?

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/53

Thanks for your patience 🙂

Rebased to master in 43dade2.

I have a use case for making installShellCompletion accept the completions via stdin. In kitty the code for installing the completions looks something like this:

mkdir -p "$out/share/"{bash-completion/completions,fish/vendor_completions.d,zsh/site-functions}
"$out/bin/kitty" + complete setup zsh > "$out/share/zsh/site-functions/_kitty"
"$out/bin/kitty" + complete setup fish > "$out/share/fish/vendor_completions.d/kitty.fish"
"$out/bin/kitty" + complete setup bash > "$out/share/bash-completion/completions/kitty.bash"

It could currently be rewritten as

mkdir 'completions'
"$out/bin/kitty" + complete setup zsh > 'completions/_kitty'
"$out/bin/kitty" + complete setup fish > 'completions/kitty.fish'
"$out/bin/kitty" + complete setup bash > 'completions/kitty.bash'

installShellCompletion --zsh 'completions/_kitty'
installShellCompletion --fish 'completions/kitty.fish'
installShellCompletion --bash 'completions/kitty.bash'

to use installShellCompletion. This is much more verbose than before. I propose adding the possibility of making installShellCompletion read stdin. The above code could then be written as follows for example:

"$out/bin/kitty" + complete setup zsh | installShellCompletion --zsh -
"$out/bin/kitty" + complete setup fish | installShellCompletion --fish -
"$out/bin/kitty" + complete setup bash | installShellCompletion --bash -

Is this a good place for this feature request or does it belong in a new issue or in the Nix community?

I just ran into this same scenario myself. My thought was to do something like

installShellCompletion \
  --zsh --exec “$out/bin/kitty + complete setup zsh” \
  --fish --exec “$out/bin/kitty + complete setup fish” \
  --bash --exec “$out/bin/kitty + complete setup bash”

but perhaps the stdin approach is better. Note that in both cases we actually need to throw in --name flags as well or it won’t know what to actually name the file.

In any case, this should go in a separate issue. If you want to file it that would be great, and please tag me on it.