-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
fishPlugins: create scope and add some plugins #107834
Conversation
439b3e5
to
682e498
Compare
682e498
to
18d0ca4
Compare
18d0ca4
to
04a3aa4
Compare
It would be good to add documentation on how to add/use these plugins. |
That's strange. This should be enough for the plugin to be detected by fish. I'm using fish enabled through home-manager's |
Uhm, I don't use home-manager, so I tried:
but
|
6bd678e
to
3a68bc4
Compare
It seems like the This is however a breaking change. Should I remove the package alias? |
3a68bc4
to
35841ad
Compare
Uhm, it's likely it was never used outside of the NixOS module. |
35841ad
to
036ba09
Compare
Note added to the release notes and amended. |
Thank you. I tested the NixOS module and built the manual: looks all good. |
036ba09
to
8657075
Compare
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 don't have any other suggestions to make, the changes look good to me.
I'd wait for more fish users/maintainers to comment before merging, 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.
8657075
to
f24b958
Compare
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.
Diff LGTM. Thanks!
And relocate the installed fish functions to the `vendor_functions.d` so that they're automatically loaded.
f24b958
to
d94921d
Compare
(Rebased to fix merrge conflict.) |
This adds a wrapper for fish which allows creating shells pre-initialised with some completions, functions, and configuration scripts from given paths or from fish plugin packages (`pkgs.fishPlugins.*`). This is especially handy when one wants to try a plugin in an ephemeral shell. GitHub: see NixOS#107834 (comment)
@pacien I'm not a fish user so I don't have much insight, but it sounds like this breaking change is sneaking up on some nix-darwin users (LnL7/nix-darwin#269). I would guess it's doing the same for HM users? |
Yeah, nix-darwin and probably home-manager need to change the foreign-env path as in this diff: edit: Yeah, they're already onto it: nix-community/home-manager#1703 |
Should we remove the alias |
Yeah, I agree with that. |
The fish-foreign-env and the fishPlugins.foreign-env packages aren't compatible due to changes in directory layout. It's better to remove the alias so that the evaluation explicitly fails instead of allowing silent runtime breakage. GitHub: see NixOS#107834 (comment) GitHub: see LnL7/nix-darwin#269 GitHub: see nix-community/home-manager#1701 GitHub: see nix-community/home-manager#1702
Motivation for this change
This bootstraps the fishPlugins scope and build tools for packaging plugins for the fish shell.
Two plugin packages are added:
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)Poking people
CC'ing the most recent committers on the fish module and package as they might be interested in this PR: