bat-extras: init at 20200408 #85836
bat-extras: init at 20200408 #85836
Conversation
|
I'm not really a fan of the fact that installing It looks like the only one of these dependencies that's actually used during the build process is My inclination is to say we should instead expose As for |
| cp bin/* $out/bin | ||
|
|
||
| substituteInPlace $out/bin/batgrep \ | ||
| --replace '$PAGER' '${less}/bin/less' |
There was a problem hiding this comment.
This is going to force pager behavior always, when the pager would otherwise not be used. Not only that, but it also disables the normal reading of the PAGER env var.
By that I mean, if I build bat-extras myself and run batgrep with my current environment, it doesn't use a pager, because I don't have any pager-related env vars set up. But with this substitution, it will completely ignore the PAGER env var and always use Nix-installed less, even if I wanted to use something else.
What we do for bat is just inject Nix's less into PATH, so when bat defaults to less (or when PAGER=less) it uses the Nix less (I forget why this was important but I think it works around some bug in macOS less combined with Bat's default options). I'm inclined to say we should just do the same here and wrap each of the bat-extras scripts such that Nix's less is prefixed to PATH. This way PAGER=less batgrep foo will use the same pager that PAGER=less bat foo uses.
| wrapProgram .test-framework/lib/print_util.sh \ | ||
| --prefix PATH : "${ncurses}/bin" \ |
There was a problem hiding this comment.
This seems wrong. This is a library file that's sourced, not an executable. Wrapping it like this means the act of sourcing it will modify the path. Instead we should make ${ncurses}/bin available either in a wrapper of .test-framework/bin/best.sh, or of test.sh itself, or just globally during the build.
|
It also occurs to me that I don't see anything in here telling bat-extras where to find |
|
I got nerd-sniped and ended up rewriting this to my own satisfaction, submitted as #85982. |
|
I left you as a maintainer in that PR but added myself, I hope that's ok. |
Motivation for this change
Some wrappers tools for
bat. Not by the same dev but they are linked on https://github.com/sharkdp/bat.I don't know if I do
chmod u+xfor nothing or if there's a way to forcewrapProgrameven if the sh is not executable.The
--replace '$PAGER' '${less}/bin/less'parts are weird. I tried to replacelesswithwrapProgramat first but failed.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)/nix/store/zyb128zq9jydyv1l53yjx6bwgw09hwq8-bat-extras-20200408 36.4M
with all optional enabled
/nix/store/a4jav90xic79vn8kgxm3zi51a9zc9dy5-bat-extras-20200408 1.1G
maybe cc the bat maintainers @dywedir @lilyball