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-history: Add module and tests #75622

Closed
wants to merge 1 commit into from
Closed

Conversation

@kampka
Copy link
Contributor

@kampka kampka commented Dec 13, 2019

Motivation for this change

Adds a module to configure the zsh-history software on zsh startup.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @worldofpeace

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Dec 13, 2019

Umm, there's an issue here. We don't want to proliferate single modules like this one because; all it does is install a zsh plugin and source it in your shell. This is exactly what zsh plugin managers were created for. There is one other, but I'm not fond of this https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/programs/zsh/zsh-autoenv.nix. It would make more sense if you could use just nix for this even. Home-manager has a functionality for this https://github.com/rycee/home-manager/blob/master/modules/programs/zsh.nix#L258, but I believe it makes much more sense there. This is because it can be scoped per user, and not the global shell. But I'm not opposed to that in nixos.

And about the test, it does make verifying the function easy.
At most, I could merge it. But it also makes me think zsh-history could provide integration testing to obsolete a nixos test, and better.

A pure way could maybe be done with https://github.com/google/goexpect, but I don't code go.
I just know Nix has a restriction with interactive shell during the build, so to work in the most compatible way would be something like expect. And a buildtime testsuite could be nice too, but it's just a zsh plugin.

@kampka
Copy link
Contributor Author

@kampka kampka commented Dec 13, 2019

Alright, since this is fairly trivial, I have no issues with maintaining it off-branch.
Thanks for reviewing anyways 👍

@kampka kampka closed this Dec 13, 2019
@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Dec 13, 2019

@kampka The test was fine though. Lmk if you want that merged.

@kampka kampka mentioned this pull request Dec 13, 2019
4 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.