-
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
Fish 3.0 #122
Comments
If you wish to switch Node.js versions automatically when function _nvm_on_pwd --on-variable PWD
test -e $PWD/.nvmrc || test -e $PWD/.node-version && nvm use
end Maybe we can offer this out of the box without degrading shell performance. Need to think about it more. |
This sounds great! I assume |
Yes, to the first question. And do nothing, to the second one. What do you think? |
I'd probably expect it to talk up at most 10 levels and stop looking after that. It should then probably print a message saying that it did not found any of the files from the current directory and 10 levels up. |
Too arbitrary? I don't really see the issue with infinite levels. How deep could you be anyway? Do you know what nvm-sh does by the way? |
nvm looks up forever until it hits /, or finds an nvmrc file. When none is found, it uses the “default” alias, or if that doesn’t exist, it errors. |
Thanks, @ljharb. We should do that too. That's also what we currently do. Do we also want an |
@jorgebucaran you mean, to allow the user to set their own aliases? |
Correct. I've always found that part, let's say "unintuitive" about nvm-sh, but that might be just me. |
I personally just use the built-in aliases (e.g. Worth seeing what others think, though! |
I personally do not care for aliases, so I cannot see the use case. |
Sounds sensible to me. And the important issue here IMHO is fixing the |
Okay. We're just going to prepend to the PATH on |
In nvm, using doesn't change or set the default - however when you install a node version when none existed, it will set the default if none exists. |
So For nvm.fish Hope that makes sense. |
No, please don’t mess with the global node version. If you have personal fish scripts or some other stuff that use The whole point of NVM is to use different Node versions side by side. I’m also don’t understand why would you even need to change the global version? There are |
Also, a subcommand akin to For example:
|
@andreiborisov Did you read this part?
Imagine nvm-sh changed the |
Sub-shells and other sessions are only part of the problem. Let’s say you have an automatically run fish script that calls Another thing to consider: what if you want to keep your default Node installation always up-to-date to have all security updates as fast as they land? With the The point is, the global installation has different requirements and use-cases. And finally, it’s easier and more predictable. You global installation = your default. Why it matters? Let’s say you’ve worked on some Node projects last week. Which is your default node version now? You can’t be sure, you need to run Which brings me to: it’s easier to implement and maintain. You don’t need to store which one is the default. You check dynamically on |
Regarding the depth of checking for |
@andreiborisov Have you used either nvm or nvm.fish? |
I’ve used |
Did I get something wrong about your proposed implementation? I've assumed you want to store the current node version and switch to it on fish startup automatically by appending |
Thanks for clarifying that. Let me just clarify that we won't touch your Now, what about functioning the same as |
What do we do if there's no system node installed? Doing the same as nvm-sh makes sense. See here. |
That's exactly the issue. Fish will run #!/usr/local/bin/fish
set log 'path/to/logs'
node app.js >> $log This will not call |
@andreiborisov Yeah, which is why I asked what about we function like nvm-sh? It's easier to implement and there's no need to run anything on startup, except for #122 (comment), but that's not so bad. |
I suggest doing nothing. 99% of people who use Node have it already installed. In my opinion, if your project needs a specific version you'll create That being said you can check for that case specifically like nvm-sh, I see no harm in that, aside from maybe it's being confusing in some circumstances. Mainly how it should behave if the user installs |
Think it would. My understanding is that nvm-sh prepends the
|
Yes, I’m against it, since it will break system-wide scripts. The main reason why I think nvm-sh is designed that way is for users not to type |
@andreiborisov I'm not sure if autoloading immediately when changing directories will be a default feature, but I do want to make this easy for users. I might not use it, but I'm willing to be convinced this is worth it. I have an update as well. I want to remove the |
👍 I would’ve removed |
There is a huge difference; just because you want to use a version doesn’t mean you have the bandwidth, cpu time, human clock time, or desire to install it. |
Frankly, I don't understand this point. We just need to download the version once and prepend it to the PATH. It's virtually instantaneous. |
Many people pay for bandwidth; many people use a version manager in CI, or on internet appliances that have very low cpu power; some may be using it in a docker container that has versions preinstalled and permissions locked down so nothing new can be installed. Additionally, many systems require compilation, and a mere download isn’t sufficient. |
After giving more thoughts I’ve realized that |
Specifically, it’s very important for any command that a user knows, with certainty, whether it will require network access, and whether it will be potentially destructive (ie, make persistent changes). At the very least, if you’re going to do this, please pick a command name that won’t be confusing - ie, something that’s neither use nor install. |
I prefer it to be automatic. Seems like a rarely encountered edge case... Maybe something like |
The mere download could cost the user money - not everyone lives in a country where internet is effectively unlimited. Disk space isn’t really my concern (which is why i didn’t mention it); disk space is effectively infinite and free at this point. |
Maybe you are fluxing "install" and "download" into one, but that's not always the case. In Fisher,
There's no need to pick a different name. The proposal is not about changing how |
Is this on-topic? Sorry, I don't think we're really arguing about that. Maybe it's a reply to #122 (comment). Not sure. FWIW I'm definitely not arguing against that. 👍
If Node is already pre-installed/downloaded,
People on those systems don't use Fish. We've never supported compilation AFAIR. It's a decent compromise. |
BTW I'm also planning to do this. From the new documentation: The set -U nvm_default_node 12.9.1 Thoughts? @andreiborisov @thernstig @jackwestmoretab |
Looks good🙏 |
Looks good to me 👍 Thanks for taking the time over this, @jorgebucaran! |
PR: #123 🙏 |
I had a few comments that I did not think of before, but I suppose those were better left as comments in the PR. So that's where they are 😃 |
I have no idea which one of those are the best - the more I stare at them the more I keep changing my opinion 😝 Would an "already installed" be too verbose? (My opinion: Yes it would be too verbose) |
I agree 100%. |
@jorgebucaran maybe #103 could be unpinned now? |
Done! Bullet list rewritten for 2.0. Also added a note! Thanks for reminding me of that. 💯 |
I'm working on a new, better nvm.fish, with a more consistent command-line interface (#81, #111), better XDG support (#108), and Fish
3.0
in mind. I also plan to fix long standing issues (#101, #112), so that switching versions only modifies the current shell. Here's the what I have in mind. All pretty straight-forward. Please let me know what we're missing and what else would you like to see?Install the specified Node.js VERSION into
$XDG_DATA_HOME
.nvm install VERSION
Activate the specified Node.js version. Automatically install the version if it isn't already installed.
nvm use VERSION
Use the version specified in the nearest
.nvmrc
or.node-version
file, walking up the directory tree from the current working directory.nvm use
Print the Node.js version currently in use.
nvm current
List all installed Node.js versions.
nvm list
List Node.js versions you can install.
nvm list-remote
Remove the specified Node.js version.
nvm remove VERSION
The text was updated successfully, but these errors were encountered: