-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
nushell: allow installing plugins #6176
Conversation
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.
There are a couple of comments that need to be addressed before merging, but other than that it looks like it works nicely, thanks!
modules/programs/nushell.nix
Outdated
activateNushellPluginsNuScript = pkgs.writeTextFile { | ||
name = "activateNushellPlugins"; | ||
text = '' | ||
#!${pkgs.lib.getExe pkgs.nushell} |
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.
Specifying a shebang is unnecessary if you're passing the program to nushell. Would it be easier to use the form nu --commands 'plugin add /nix/.../formats; plugin add /nix/.../polars'
?
If not, this can be simplified with
pkgs.writeText "activateNushellPlugins.nu" ''
# file contents in here
'';
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.
Oh, of course. I had tried using --commands
instead of generating and passing a script and got a command-not-found error, but your comment prompted me to try again, and it worked, so I must have been doing something else wrong. I think it looks much tidier now, thanks!
a9cda04
to
219bef1
Compare
@JoaquinTrinanes Thanks for your feedback, and apologies for missing your existing PR that tried doing the same thing. 🤦 |
Nice! No worries about the existing PR, it was stale for some months already 😅 Everything looks good now, I'd replace usages of |
219bef1
to
23ab167
Compare
When the version of Nushell or any Nushell plugin changes, the plugin registry must be regenerated.
23ab167
to
8772bae
Compare
Thanks for the contribution @aidalgol and the review @JoaquinTrinanes! Merged to master now 🙂 |
Description
When the version of Nushell or any Nushell plugin changes, the plugin registry must be regenerated. This adds an option that takes a list of Nushell plugin derivations and recreates the plugin registry file (
~/.config/nushell/plugin.msgpackz
) on profile activation.Thanks to @crabdancing for most of the work: nushell/nushell#12997 (reply in thread)
I believe this is worth adding a news entry even though it is only adding an option, because this has been a significant pain point for anyone using plugins with Nushell managed by home-manager.
Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
ornix develop --ignore-environment .#all
using Flakes.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
Maintainer CC
@Philipp-M @JoaquinTrinanes