feat: add nushell support #35
Conversation
github-actions
bot
added
the
patch
| @@ -110,8 +114,7 @@ fn main() -> eyre::Result<()> { | |||
| let current_exe = | |||
| current_exe().wrap_err("Unable to determine absolute path of `nix-your-shell`")?; | |||
| if args.absolute || !executable_is_on_path(¤t_exe)? { | |||
| shell_code = | |||
| shell_code.replace("nix-your-shell", &format!("{current_exe} --absolute")); | |||
| shell_code = shell_code.replace("nix-your-shell", current_exe.as_str()); | |||
There was a problem hiding this comment.
Looks like adding --absolute doesn't add anything useful to the shell code, but it messes up with nushell integration
There was a problem hiding this comment.
I think I wanted --absolute to be reflected here:
$ nix-your-shell fish env
# If you see this output, you probably forgot to pipe it into `source`:
# nix-your-shell fish | source
...
$ nix-your-shell --absolute fish env
# If you see this output, you probably forgot to pipe it into `source`:
# /Users/wiggles/.nix-profile/bin/nix-your-shell --absolute fish | source
...The idea being it would show you the exact command you need to run to get the same output.
I'm not convinced it's super useful, though, so maybe we can just leave it out.
This is also straining the limits of "do a literal string replace for nix-your-shell" as a templating mechanism. It might be worth integrating an actual templating engine like MiniJinja or tera here -- what do you think? I'm loathe to lose the property of the shell snippets in data/ being actual shell code you can run unmodified, but maybe it's fine (and it seems like it'll happen sooner or later).
There was a problem hiding this comment.
I see now and for me this is not very useful either so I suggest to remove adding --absolute to not overcomplicate the code.
If taking about integrating templating I'd prefer not to introduce it until there is real need for this as having shell ready code could simplify integrations in more cases.
There was a problem hiding this comment.
Hi! Thanks for sending this in!
I haven't used nushell, although I've been watching it from afar and think it's pretty neat. I'll need your expertise here and I'll also want you to maintain the nushell support in nix-your-shell as well — are you willing to do that?
Overall, this PR looks good. Let's move generate-config to nix-your-shell.passthru.generate-config and then I think we can merge this.
| ```sh | ||
| nix-your-shell nu | save nix-your-shell.nu | ||
| ``` |
There was a problem hiding this comment.
Non-blocking: Maybe we should have a nix-your-shell nu install command, which generates the correct configuration and puts it in a place the shell will notice, e.g. appending it to ~/.config/nushell/config.nu?
There was a problem hiding this comment.
we can do something like this, but I do prefer more explicit configurations instead of messing up with user configurations. I'm still not sure what the best way to provide the integrations with nushell. One of the aspect of this - the nixos + home-manager users that won't benefit from such command and for them would be better to provide integration on home-manager side.
flake.nix
Outdated
| } | ||
| // | ||
| { | ||
| generate-config = shell: final.runCommand "nix-your-shell-config" {} '' |
There was a problem hiding this comment.
We might want this to have a more unique name in case downstream users apply this overlay on their entire nixpkgs. Maybe we can put it in nix-your-shell.passthru.generate-config?
There was a problem hiding this comment.
I've put generate-config to passthru property. Also tested that it works as before
| @@ -110,8 +114,7 @@ fn main() -> eyre::Result<()> { | |||
| let current_exe = | |||
| current_exe().wrap_err("Unable to determine absolute path of `nix-your-shell`")?; | |||
| if args.absolute || !executable_is_on_path(¤t_exe)? { | |||
| shell_code = | |||
| shell_code.replace("nix-your-shell", &format!("{current_exe} --absolute")); | |||
| shell_code = shell_code.replace("nix-your-shell", current_exe.as_str()); | |||
There was a problem hiding this comment.
I think I wanted --absolute to be reflected here:
$ nix-your-shell fish env
# If you see this output, you probably forgot to pipe it into `source`:
# nix-your-shell fish | source
...
$ nix-your-shell --absolute fish env
# If you see this output, you probably forgot to pipe it into `source`:
# /Users/wiggles/.nix-profile/bin/nix-your-shell --absolute fish | source
...The idea being it would show you the exact command you need to run to get the same output.
I'm not convinced it's super useful, though, so maybe we can just leave it out.
This is also straining the limits of "do a literal string replace for nix-your-shell" as a templating mechanism. It might be worth integrating an actual templating engine like MiniJinja or tera here -- what do you think? I'm loathe to lose the property of the shell snippets in data/ being actual shell code you can run unmodified, but maybe it's fine (and it seems like it'll happen sooner or later).
Thank you. I'll be glad to help with maintaining the nushell support! I'll make the requested changes to this mr shortly |
|
Should we change the labels of the mr as it looks like it'll affect version bump action? |
9999years
added
minor
|
Looks good, I'll get this merged and cut a new release. |
|
OK my release CI broke but I got it fixed and shipped this in v1.3.0. |
No description provided.