feat: auto invoke nvm use on PWD change

[edouard-lopez]

Hello @jorgebucaran,
long time no see :)

As I was not satisfied by the Deeper Shell Integration for Fish solution from _classic NVM, I sought something simpler.

I added a mechanism to automatically invoke nvm use on PWD changes based on @ygormutti's idea wbyoung/avn#12 (comment).

Current version is pretty dumb, but wanted to check if that was of interest to you first. So please let me know if you want such feature?
Feature behind a flag:

set --universal auto_invoke_nvm true

[jorgebucaran]

Hi, @edouard-lopez! Perhaps this should be a plug-in? Definitely not the default behavior.

[edouard-lopez]

Can you elaborate on the plugin idea? Do you mean with feature flag, a dedicated repo, or something else?

[jorgebucaran]

For starters this function could be a different plugin, installed separately. Or as you say, behind a feature flag (universal variable).

[edouard-lopez]

update with feature flag

[jorgebucaran]

I'd rather have the flag prevent _nvm_auto_invoke from being defined at all, rather than define the --on-variable PWD hooks and incur this check every time.

[edouard-lopez]

Doing something like below break the autoloading mechanism:

if set --query nvm_use_on_pwd_change; and test "$nvm_use_on_pwd_change" = true
    function _nvm_use_on_pwd_change \
        --on-variable PWD \
…
    end
end

Should I hook _nvm_use_on_pwd_chang on both the flag variable ($nvm_use_on_pwd_change) AND $PWD? Or do something like https://github.com/jorgebucaran/hydro/blob/d4875065ceea226f58ead97dd9b2417937344d6e/conf.d/hydro.fish#L119-L120 ?

[jorgebucaran]

Thank you for adding tests, good work. Left you several comments.

[jorgebucaran]

Sorry, more comments left. Thank you!

[jorgebucaran]

We'll end up running the function whenever the file is sourced.

[edouard-lopez]

ok the --on-variable option is enough, my bad. I will remove the call

[jorgebucaran]

👍

[jorgebucaran]

We only need to define the PWD change function when the feature flag is set. This may involve defining another function inside the function or some other trick. 🤔

[edouard-lopez]

I was thinking about using funcsave but the manual states:

Note that because fish loads functions on-demand, saved functions will not function as event handlers until they are run or sourced otherwise. To activate an event handler for every new shell, add the function to your configuration file instead of using funcsave.

So need to add a call in conf.d/nvm.fish

[jorgebucaran]

Could we get even fancier? Consider:

function _nvm_use_on_pwd_change --argument-names _ op --on-variable nvm_use_on_pwd_change
    if test "$op" = ERASE
        functions --erase _nvm_pwd_change
    else
        function _nvm_pwd_change --on-variable PWD
            nvm use --silent 2>/dev/null
        end
    end
end

[jorgebucaran]

Also reasonable would be to check if the variable is just set. Maybe that's even fishier.

[jorgebucaran]

Could we get even fancier? Consider:

function _nvm_use_on_pwd_change --argument-names _ op --on-variable nvm_use_on_pwd_change
    if test "$op" = ERASE
        functions --erase _nvm_pwd_change
    else
        function _nvm_pwd_change --on-variable PWD
            nvm use --silent 2>/dev/null
        end
    end
end

[jorgebucaran]

Good work, @edouard-lopez. Left you one suggestion. 👋

[bastianwegge]

Just pinging, we just ran into the same scenario, used your code and it works flawlessly. Is there any chance for this to make it to master/release? :)

[jorgebucaran]

I left some comments that weren't resolved. I also don't want to bloat this tool with features, hence the long silence.

[edouard-lopez]

I haven't the need for this currently, so won't push forward, @bastianwegge feel free to push on my MR or fork it.

You might be interested in volta.

[lydell]

In case someone else comes across this PR and wants this functionality, here’s a way to do it in userland:

#186 (comment)

[jorgebucaran]

Took another peek while fixing bugs and closing old issues. This one's a bit of a tricky issue, as it adds a lot of complication to nvm.fish for everyone. Personally, I don't have a need for it, but I get that some folks do. That's why we worked on #186 to improve switch times and encourage folks to come up with their own tiny functions (remember how cool it is to extend your Fish shell?), like @lydell did. 💯