Auto-expand tilde (home directory) for universal variable `nvm_data`

[thernstig]

Auto-expand tilde (home directory) for universal variable `nvm_data`

[jorgebucaran]

Fixed. Thanks, everyone! 🙌

[jorgebucaran]

We could just revert the scope of nvm_data as per @debashisbiswas' suggestion.

See the commit above to see why we changed the scope to universal. Thank you, @rhenning! 😁

[lorthirk]

👋🏼 just stumbled across this while syncing config with chezmoi across a couple of machines

The world is such a small place... I am in the very same situation! I've been watching this issue for a long time, and I am also available for anything!

[jorgebucaran]

Or perhaps just change the variable scope to global? 🤓

[thernstig]

If that works ok, sure :)

[debashisbiswas]

Following the above recommendation, I changed nvm_data from universal to global, moving it out of fish_variables and into conf.d/nvm.fish (not config.fish, which happens after the default version is loaded), which worked for my use case.

Is this something you'd be interested in this project doing? If so, I wouldn't mind making the PR.

[jorgebucaran]

nvm_data is universal because 2f2af43. If we can find a way to change it back to global without creating new issues, that'd be great.

[rhenning]

👋🏼 just stumbled across this while syncing config with chezmoi across a couple of machines, which inadvertently picked up the statically-rendered interpolation of nvm_data in fish_variables. i began to work around it with a template but then started to wonder how it got set... glad to see others are thinking about it too.

glad to help PR, test, buy coffee, etc. 😆

p.s. many thanks for your contributions to the community, @jorgebucaran. awesome work!

[debashisbiswas]

nvm_data is universal because 2f2af43. If we can find a way to change it back to global without creating new issues, that'd be great.

I read through the issues that this commit was meant to fix. To provide context for this discussion: the reason it was made universal is to avoid requiring the user to restart their terminal after installing, and to avoid issues in which their config was set up to use nvm before nvm.fish was run.

[lorthirk]

I would love to think of a way to fix this, but I'm afraid that the context @debashisbiswas provided is not enough for me to fully understand ths issue 😕 any other hint would be greatly appreciated

[debashisbiswas]

@lorthirk It's been a few months, so I've forgotten the specifics, but I can try to give you a bit more context. Looking at 2f2af43 and the related issues, there seem to be a few reasons why the variable was made universal. The issues in question are #130, #139, and #140.

Universal variables are applied immediately, and available to all sessions. On the other hand, global variables are per-session, and must be set before they are used within the session. Previously, the global variables were set in conf.d/nvm.fish, which runs when the session starts. Right after a fresh install, this file hasn't been run yet, so trying to use nvm immediately after installing resulted in errors. This means that users needed to start a new session after install. This caused some confusion for users, so universal variables were used instead. I believe this is what happened in #130 and #140. It should be possible to achieve the same behavior with global variables if they're managed correctly.

Also, config files in fish are loaded on startup in a specific order. If the user's config is set up to use nvm before conf.d/nvm.fish is run (for example, by using nvm install in conf.d/aaa.fish, which would run before), there will be issues with variables being uninitialized, which results in the same error as the previous paragraph. This is what happened in #139, and universal variables solve this as well. I am not sure that there's a way around this with global variables, but this case may not be necessary for this plugin to handle, as this is more specifically related to how a user's fish config is set up.

[lorthirk]

Thanks @debashisbiswas. Now that you explained it in such an easy to understand way, I'm not so sure any fix we could come up with would be better than "hackish". I have a naive question: would be that bad to add a warning message when installing nvm.fish that warns about such issues? Honestly, restarting a session is not that bad once you know you have to do, and in general I wouldn't let this drive the architecture of an application. As per the other issue, as you said, this really sounds like something a user should be warned of and arrange his/her custom configuration accordingly.

What do you guys think?