-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
z: init at 1.11 #39930
z: init at 1.11 #39930
Conversation
pkgs/tools/z/default.nix
Outdated
install -Dm 644 z.sh $out/share/z.sh | ||
gzip z.1 | ||
install -Dm 644 z.1.gz $out/share/man/man1/z.1.gz | ||
''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be written as:
diff --git a/pkgs/tools/z/default.nix b/pkgs/tools/z/default.nix
index ed5fea65916..97f7ff812b4 100644
--- a/pkgs/tools/z/default.nix
+++ b/pkgs/tools/z/default.nix
@@ -11,11 +11,11 @@ stdenv.mkDerivation rec {
sha256 = "13zbgkj6y0qhvn5jpkrqbd4jjxjr789k228iwma5hjfh1nx7ghyb";
};
- buildPhase = " "; # do nothing
+ phases = [ "unpackPhase" "installPhase" ];
+
installPhase = ''
install -Dm 644 z.sh $out/share/z.sh
- gzip z.1
- install -Dm 644 z.1.gz $out/share/man/man1/z.1.gz
+ install -Dm 644 z.1 $out/share/man/man1/z.1
'';
meta = {
pkgs/tools/z/default.nix
Outdated
sha256 = "13zbgkj6y0qhvn5jpkrqbd4jjxjr789k228iwma5hjfh1nx7ghyb"; | ||
}; | ||
|
||
phases = [ "unpackPhase" "installPhase" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to use dontBuild = true;
.
pkgs/tools/z/default.nix
Outdated
phases = [ "unpackPhase" "installPhase" ]; | ||
|
||
installPhase = '' | ||
install -Dm 644 z.sh $out/share/z.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to enable it in bash at install time? Becuase it seems you still need to do manual actions after this package installation to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(maybe the autojump package is doing this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about zsh users though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it is possible to do it for both shells
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see if the autojump package is doing this, but I know that grml-zsh-config isn't. Not sure it's possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, a common way is to provide a script that returns the path of the script (since the path of the script in the nix store is not static).
For instance, fzf
provides a script fzf-share
that can be sourced in your bashrc file source $(fzf-share)/completion.bash
.
See
nixpkgs/pkgs/tools/misc/fzf/default.nix
Line 44 in ae702c6
cat <<SCRIPT > $bin/bin/fzf-share |
pkgs/tools/z/default.nix
Outdated
install -Dm 644 z.1 $out/share/man/man1/z.1 | ||
''; | ||
|
||
meta = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta = with stdenv.lib; {
is commonly used, then you can drop lib.
from license and maintainers below.
Also homepage = https://github.com/rupa/z;
would be nice to add :)
pkgs/tools/z/default.nix
Outdated
@@ -0,0 +1,26 @@ | |||
{ stdenv, fetchFromGitHub, lib, ... }: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib
can be dropped here when the change to meta is done. I don't think ellipsis (...
) is commonly used in package derivations so it should probably be removed.
Before this is merged by someone (not me) you will need to squash the commits to one and use a correctly formatted commit message header like: I recommend reading this: https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md |
pkgs/tools/z/default.nix
Outdated
''; | ||
|
||
meta = { | ||
description = "Tracks your most used directories, based on 'frecency'."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dot at the end should be removed.
f9efcb6
to
b8d06e5
Compare
I hadn't heard of autojump before. It seems to be pretty much the same thing. Maybe I should close this in favor of that? |
@jD91mZM2 Even if this is doing something close to autojump, if you prefer |
I actually just switched to using autojump myself lol |
@jD91mZM2 oh ok :/ |
I think we can include it, add me as maintainer please |
@marsam Add or replace? |
If you don't want to maintain it, you can remove yourself |
I'm fine with either :) |
5734066
to
2e650b2
Compare
pkgs/tools/misc/z/default.nix
Outdated
dontBuild = true; | ||
|
||
installPhase = '' | ||
install -Dm 644 z.sh $out/share/z.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For shell integration something like this would be helpful:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/misc/fzf/default.nix#L44
I just realized that z doesn't have a license, and would thus have to be marked as unfree :/. Closing in favor of autojump. |
Adds https://github.com/rupa/z.