Fix duplicate variable of `multishell_path` in `PATH`

[0x221A]

This PR contains the change of the env command and shell.

See: #441

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/schniz/fnm/MGz4VD3R25BRSJpRT3s6J76B5zcT
✅ Preview: https://fnm-git-fork-rootblack45-fix-generate-too-many-tm-b9b580-schniz.vercel.app

[Schniz]

Hi! thanks for the contribution.

I'm all in for filtering the PATH to not add directories that are not necessary and just messes up everything. Do you think it should be a shell code or can it be in a single place? We can get PATH with std::env and solve it once for all shells

This change will avoid the env command generating new multishell_path whenever it was exist in the fnm config

I don't understand why doing this: is there a reason of calling fnm env if you don't want to set up a new shell? Can you please explain the use case? 😃

Personally I use tmux in zsh, so when my .zshrc file initializes on the main shell, it creates a FNM_MULTISHELL_PATH for the global shell, and then it creates a new multishell_path for each tmux pane/tab/window I open. So with this code, I will have to conditionally evaluate fnm if I'm within tmux, or to do something like eval "$(FNM_MULTISHELL_PATH= fnm env)"

[0x221A]

I'm all in for filtering the PATH to not add directories that are not necessary and just messes up everything. Do you think it should be a shell code or can it be in a single place? We can get PATH with std::env and solve it once for all shells

I think it can be in a single place in env.rs. The code will be like this:

fn dedup_and_merge_path_variable(path: &Path) -> String {
    let current_path = std::env::var_os("PATH").expect("Can't read PATH env var");
    let mut split_paths: Vec<_> = std::env::split_paths(&current_path).collect();
    split_paths.dedup_by(|a, b| a == b || a.to_str().unwrap().contains("fnm_multishells"));
    split_paths.insert(0, path.to_path_buf());
    let new_path = std::env::join_paths(split_paths).expect("Can't join paths");
    new_path.to_str().expect("Can't read PATH").to_owned()
}

impl Command for Env {
    ...
    fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
        ...
        println!(
            "{}",
            shell.set_env_var("PATH", dedup_and_merge_path_variable(&binary_path).as_str())
        );
        ...
    }
}

I don't understand why doing this: is there a reason of calling fnm env if you don't want to set up a new shell? Can you please explain the use case?

My point is whenever we need to reload the session the shell setup command will run. From the previous codes when we invoke the make_symlink function it returns a path of the symlink and uses that path to set into PATH and FNM_MULTISHELL_PATH. Hence if FNM_MULTISHELL_PATH was already in the config we don't need to make a new symlink. This will be an easier way to handle the duplicate PATH variables and we can guarantee that the path variable in PATH and FNM_MULTISHELL_PATH are the same.

You can try by opening a terminal and trying to run source .zshrc multiple times you will see that from the previous code FNM_MULTISHELL_PATH will change every time.

[Schniz]

My point is whenever we need to reload the session the shell setup command will run.

how many times do you find yourself reloading a session? (by reloading I assume you mean source ~/.zshrc)

[Schniz]

I don't say that enough, but I'm really thankful for the contribution. I hope my comments and questions don't annoy you 🙏

[0x221A]

@Schniz No problem 😁. This is my first time contributing to the Rust project. Thanks for your advice.

[0x221A]

@Schniz Hi, according to this commit https://github.com/Schniz/fnm/commit/ad0ecd7fff864430e072c0df2fe984ef0875324b. I just updated the code to support the local cache directory.

[0x221A]

@Schniz I just updated the code according to this commit https://github.com/Schniz/fnm/commit/c85ff97f1404b0042f7378acb0ad67b3fd607189. And fixed failed test

[Schniz]

🙏 let's see if tests pass 🙏

[0x221A]

🙏 let's see if tests pass 🙏

It's failed. Look like I need to install Windows to test it...

[Schniz]

I wish GitHub Codespaces had a Windows option. 😢

[0x221A]

@Schniz Just fixed. I hope this is the last 😂

[0x221A]

Finally, it passed 🎉