fix(cli/self-update): fix Nushell-related suggestions and scripts

[rami3l]

Supersedes #4230. Closes #4242.

PS: Thanks for your contribution, @Yethal! I've made you the co-author of two three main commits in this PR.

@alluring-mushroom @Yethal thanks again for your report! I don't use Nushell personally and I'm not sure if this has covered all the corner cases (it would be difficult to test automatically due to the amount of different setups we need to support). Would you mind trying out this patch and see if the fix works for you?

[Yethal]

@rami3l Just ran this in a VM
Preinstall prompt:

This path will then be added to your PATH environment variable by
modifying the profile files located at:

  /home/vagrant/.profile
  /home/vagrant/.bashrc
  /home/vagrant/.config/nushell/env.nu
  /home/vagrant/.config/nushell/config.nu

Post install prompt:

...
To configure your current shell, you need to source
the corresponding env file under $HOME/.cargo.

This is usually done by running one of the following (note the leading DOT):
. "$HOME/.cargo/env"            # For sh/bash/zsh/ash/dash/pdksh
source "$HOME/.cargo/env.fish"  # For fish
source $"($nu.home-path)/.cargo/env.nu"  # For nushell

Generated env.nu:

if ("$HOME/.cargo/bin" not-in ($env.Path | split row (char esep))) {
  $env.Path = ($env.Path | prepend "$HOME/.cargo/bin")
}

$env.PATH contents:

╭────┬──────────────────────────╮
│  0 │ $HOME/.cargo/bin         │
│  1 │ /home/vagrant/.cargo/bin │
│  2 │ /usr/local/sbin          │
│  3 │ /usr/local/bin           │
│  4 │ /usr/sbin                │
│  5 │ /usr/bin                 │
│  6 │ /sbin                    │
│  7 │ /bin                     │
│  8 │ /usr/games               │
│  9 │ /usr/local/games         │
│ 10 │ /snap/bin                │
╰────┴──────────────────────────╯

Unfortunately generated env.nu still uses posix-like $HOME syntax instead of nu-specific $"($nu.home-path)/.cargo/bin" so this still doesn't work as advertised (but no longer breaks existing configs so there's that)
Other than that you may want to consider only appending to config.nu since env.nu is about to be deprecated

[rami3l]

@Yethal Thanks for your quick response. As per #4265 (comment) this is the expected behavior (for now); will fix.

[alluring-mushroom]

@Yethal

env.nu is about to be deprecated

Thank you for this, I hadn't been following the recent changes.

@rami3l if you look at the documentation for the next release

It shows out nushell intends for it to be done in the future

[rami3l]

@Yethal @alluring-mushroom Sorry for the back and forth, but I have tried to apply the suggestions from both of you. Would you mind giving this PR another shot?

[Yethal]

~> $env.PATH
╭───┬──────────────────────────╮
│ 0 │ /home/vagrant/.cargo/bin │
│ 1 │ /usr/local/sbin          │
│ 2 │ /usr/local/bin           │
│ 3 │ /usr/sbin                │
│ 4 │ /usr/bin                 │
│ 5 │ /sbin                    │
│ 6 │ /bin                     │
│ 7 │ /usr/games               │
│ 8 │ /usr/local/games         │
│ 9 │ /snap/bin                │
╰───┴──────────────────────────╯

@rami3l We're good. Path is properly appended so the issue is resolved.

For easier multiplatorm support (once that happens) you may want to switch over to using the path add function from the Nushell standard library which will take care of deduplicating Path entries for you as well as path join for platform specific path separators instead of hardcoding /. Code works fine as is though so treat this as a suggestion and not something that needs to be implemented right now.

use std/util "path add"
path add ($nu.home-path | path join .cargo bin)

[rami3l]

@Yethal Thanks for your feedback! I'm glad to see that this PR is working for you.

Regarding your two suggestions, I think since we are handling path joining from Rust's side, it might be better to postpone the change regarding path join.

On the other hand, your path add suggestion does look like a good improvement that we can integrate right now. Regarding the final env.nu template, I'm thinking about something like the following:

-if ($"{cargo_bin}" not-in ($env.Path | split row (char esep))) {
-  $env.Path = ($env.Path | prepend $"{cargo_bin}")
-}
+use std/util "path add"
+path add $"{cargo_bin}"

... I hope that looks reasonable to you. Besides, do you think this use std/util "path add" API is stable enough for the inclusion?

[Yethal]

@Yethal Thanks for your feedback! I'm glad to see that this PR is working for you.

Regarding your two suggestions, I think since we are handling path joining from Rust's side, it might be better to postpone the change regarding path join.

On the other hand, your path add suggestion does look like a good improvement that we can integrate right now. Regarding the final env.nu template, I'm thinking about something like the following:

-if ($"{cargo_bin}" not-in ($env.Path | split row (char esep))) {
-  $env.Path = ($env.Path | prepend $"{cargo_bin}")
-}
+use std/util "path add"
+path add $"{cargo_bin}"

... I hope that looks reasonable to you. Besides, do you think this use std/util "path add" API is stable enough for the inclusion?

Yes, this looks reasonable. path add is part of Nushell's standard library so I'd say it's stable enough

[djc]

Good stuff!