nvm icon indicating copy to clipboard operation
nvm copied to clipboard

show only versions newer than NVM_MIN if set

Open ryenus opened this issue 1 year ago • 20 comments

This is to fix #3250.

ryenus avatar Jan 29 '24 13:01 ryenus

For example, running NVM_MIN_VER=18 nvm ls-remote --lts when v14 and v16 are installed:

image

ryenus avatar Jan 29 '24 14:01 ryenus

Performance-wise there's not really much change or overhead, you can trust that awk is fast :-) Sometimes it might even run faster, esp. when many older versions are skipped.

Meanwhile the other sibling functions and even the tests are something I'm not really familiar with, may you can chime in here? @ljharb

ryenus avatar Jan 29 '24 16:01 ryenus

Sure - for the tests, there's existing tests for nvm ls-remote that employ mocks for the remote data, and a test that uses the env var, and asserts that it matches the mock minus the first N lines (since older versions aren't going to change, that number won't break over time), that would be sufficient. Similar approaches will work for the others.

For adding an option, if you grep for "ls-remote" you'll find the chunk of code where top-level commands are defined, and there's a bunch of existing examples that process the arguments.

ljharb avatar Jan 29 '24 16:01 ljharb

@ljharb just added a test, however Travis CI seems stuck, any insight?

ryenus avatar Jun 29 '24 10:06 ryenus

BTW, I had to adjust the travis job a bit, due to the error

pyenv: /opt/pyenv/versions/2.7.18 already exists

See the screenshot below (https://app.travis-ci.com/github/nvm-sh/nvm/jobs/623514514):

image

ryenus avatar Jun 29 '24 12:06 ryenus

https://app.travis-ci.com/github/nvm-sh/nvm/jobs/624025129#L797

  ✗ Running 'nvm unload' should unset all function and variables
+BEFORE=./before.tmp
+AFTER=./after.tmp
+typeset -f
./Running 'nvm unload' should unset all function and variables: 11: typeset: not found

Here the error is about typeset (or declare) which are not available in POSIX sh, according to https://www.shellcheck.net/wiki/SC3044

https://github.com/nvm-sh/nvm/blob/762f9ef9d17623b45095e8ca1a996c8928f3f424/test/fast/Running%20'nvm%20unload'%20should%20unset%20all%20function%20and%20variables#L10-L12

ryenus avatar Jul 14 '24 07:07 ryenus

As we can see from the below screenshot, for v16.15.1, now all of its updates are listed:

ryenus avatar Jul 14 '24 11:07 ryenus

When I run nvm ls locally with this PR, I get empty as the first item in the list.

When I run nvm ls-remote --min=4 locally with this PR, all versions are displayed.

ljharb avatar Jul 27 '24 20:07 ljharb

  • When I run nvm ls locally with this PR, I get empty as the first item in the list.

    Interesting, any idea where could this empty come from? Asking because here the change to nvm_print_versions only determines whether to skip certain versions or not, meanwhile all the versions are supplied by the caller.

    May I ask what's the output of nvm ls when run from the master branch? Asking because for me I got identical output:

    image
  • When I run nvm ls-remote --min=4 locally with this PR, all versions are displayed.

    As a comparison, here's what I got with the same command:

    image

    Is it possible that the shell was still running the code from another branch, rather than that of the PR? Maybe check the output of type nvm_print_versions?

ryenus avatar Jul 28 '24 11:07 ryenus

I'm certain that I'm using the code from this PR, yes.

I'm running this on a Mac; if you're not, then perhaps there's some platform-specific difference here?

ljharb avatar Jul 28 '24 19:07 ljharb

hm, ok i did just figure out that i had a $NVM_DIR/versions/node/empty dir for some reason, so at least that's a red herring. Let me test again.

ljharb avatar Jul 28 '24 19:07 ljharb

ok, testing again, and nvm ls-remote --min=4 (or 20, or whatever) still has no effect for me on my Mac.

What OS are you using exactly?

ljharb avatar Jul 29 '24 00:07 ljharb

ok, testing again, and nvm ls-remote --min=4 (or 20, or whatever) still has no effect for me on my Mac.

What OS are you using exactly?

Actually I'm also on mac :-) How about NVM_MIN=4 nvm ls-remote? Can you also try to enable set -x within nvm_print_versions?

ryenus avatar Jul 29 '24 00:07 ryenus

ohhh wait, lol i think this is the actual feature i asked for - i have tons of node versions installed, going back a very long way:

$ NVM_MIN=4 nvm ls-remote | wc -l
     711
$ nvm ls-remote --min=4 | wc -l
     711
$ nvm ls-remote | wc -l
     790

in other words, it's filtering some things out, but with the additional logic i requested, it's still showing me a lot of things i didn't expect. 🤔

ljharb avatar Jul 29 '24 00:07 ljharb

ohhh wait, lol i think this is the actual feature i asked for - i have tons of node versions installed, going back a very long way:

in other words, it's filtering some things out, but with the additional logic i requested, it's still showing me a lot of things i didn't expect. 🤔

Ahh, so it's because you have just too many versions installed? Or would you change mind? Hopefully not :P

BTW, typically I just run something like NVM_MIN=18 nvm ls-remote --lts.

ryenus avatar Jul 29 '24 00:07 ryenus

@ljharb FYI, just rebased with some minor fix in tests.

ryenus avatar Jul 30 '24 00:07 ryenus

@ljharb may I ask how it is going? anything to note?

ryenus avatar Aug 13 '24 02:08 ryenus

No progress yet; I've been traveling. I'm prioritizing a bug fix on nvm before this PR, but it'll be next in line after that.

ljharb avatar Aug 13 '24 22:08 ljharb