-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NVM.fish — 2.0 🎏 #123
NVM.fish — 2.0 🎏 #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started from a validation perspective i.e. only commenting on the usage from the README.md perspective. I'll see if I have time to review the code later.
- Use uninstall instead of remove - Document nvm_mirror and nvm_default_version vars in help
I am happy with the commit 🥇 💯 👍 |
- Fix bug in list-remote showing no output - Improve node/npm version info shown when installing/using
I think we're ready here. I won't be adding tests until Fishtape
|
I tested the new versions and found something. Is it expected? Without relaunching the shell, I tried e.g.
Even if I specify a version manually and install I get another error. Is that expected? Relaunching the shell fixes it. I will try some functionality now when I have relaunched it. |
I thought I had fixed that bug in da2cf70. Are you able to reproduce it? Thanks. |
A few things noticed.
This might actually also be the case where 1) is seen if I directly do a
|
Was that fix included in jorgebucaran/[email protected] which was what I installed and tested. I just tested it 30 mins ago and got that failure. |
Yes, everything I push to this branch is what you get when you install
It shouldn't be too hard to support
I'm not sure what's happening. From the two Lines 77 to 82 in 4517822
I also found this on SO, but we're piping the output to Could this be OS-specific? Do you happen to have a |
I have no |
Did direnv put it there? |
nvm puts it there, of course, when you install a version, and removes it when you uninstall it or use a different version (or switch to system if there's one). So, this could be a bug, but to be honest, I still don't understand what the issue you're having is. Please provide me a longer dump of your terminal usage where the issue is clear. |
I was about to edit my reply. Yes, it sets it via the two env vars: set -gx NODE_VERSIONS ~/.local/share/nvm
set -gx NODE_VERSION_PREFIX 'v' See https://github.com/direnv/direnv/blob/f5b1566ab3f89c7e912ce0d1eb82ad8814247d30/stdlib.sh#L1068 calling https://github.com/direnv/direnv/blob/f5b1566ab3f89c7e912ce0d1eb82ad8814247d30/stdlib.sh#L625 |
Still get it: |
In that case, you did set it, albeit indirectly. Okay, we can re-implement
Hmm. |
@thernstig Last attempt using curl's --no-buffer flag. If this doesn't do it, I'll have to disable the progress bar, but that would be a shame. Or avoid the pipeline and redirect to a file, which wouldn't be so elegant. This is why we can't have nice things! Please try again. 🙏 |
Yes, to be fair, I did say I was about to edit my reply 😛
Maybe give wget a go? I'd just redirect to a file to be done with it to keep the progress bar. |
@thernstig Good news: the About
|
It is indeed fixed. You're awesome, great work! What was the exact problem?
Yes, I think that might be more efficient and also less code for direnv. I'll write an issue once NVM 2.0 is released. I assume the only thing to figure out there in that project is to check if nvm.fish or nvm-sh is used and do an elif branch to instead just call |
@thernstig Wouldn't it be better to use |
direnv handles a lot more than just nvm for me, and we use it as a shared tool to setup config for one project and when I juggle between project dirs I use it to not have to do any "use nvm" or other tools to setup the env for each project. That extra Anyhow if we go back to this PR, it gets 5 thumbs up from me. |
@thernstig Sorry, I was not referring to I was trying to say: wouldn't it better if
Totally. 🙆♂️ |
Ah, yes, great idea. I will see if I can do a nice write-up of an issue for direnv later. |
NVM.fish completely rewritten using Fish 3.0, introducing a new CLI. Not a new CLI you need to learn. One that is familiar to you, if you've used the original NVM project, but without the cruft. Implements #122.
The usual suspects are included: install, use,
removeuninstall, list, list-remote, and current.NVM
2.0
presents a more consistent command-line interface (#81, #111), better XDG support (#108), and leverages modern Fish idioms for improved performance. Also fixed (#101, #112), so that switching versions only lasts for the lifetime of the current shell. Want to change that? There's a way too.