feat: add filtering options to `ls-remote` #1063

ryanccn · 2023-10-25T15:53:56Z

This PR adds the following options to fnm ls-remote:

--lts: only show LTS versions
--lts <name>: only show LTS versions for a specific codename
--sort <asc|desc>: sort versions by ascending or descending
--filter: filter versions by specifying a UserVersion
--latest: only show the latest version matching all of the criteria above

changeset-bot · 2023-10-25T15:53:59Z

🦋 Changeset detected

Latest commit: a436254

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
fnm	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

polarathene · 2023-11-11T07:03:46Z

Might be nice if it can omit earlier releases, most of the time you may only be interested in seeing a list of the latest versions sorted by major (or just LTS).

For example, this would probably make for a nicer fnm list-remote --lts sort of filter (related is this --lts filter option PR):

# Filters to only lines with an LTS name,
# flips the order (to prefer latest version),
# sorts by the right-side (LTS name) as unique field
# Flips output of sort so the latest LTS is at the top:

$ fnm list-remote | grep '(' | tac | sort -k 2 -u | tac
v20.9.0 (Iron)
v18.18.2 (Hydrogen)
v16.20.2 (Gallium)
v14.21.3 (Fermium)
v12.22.12 (Erbium)
v10.24.1 (Dubnium)
v8.17.0 (Carbon)
v6.17.1 (Boron)
v4.9.1 (Argon)

Schniz · 2023-11-11T08:35:38Z

Cool idea! I think we can receive a semver range like “install” and “use”. Should not be hard to implement too. Are you interested in doing so?

Schniz · 2023-11-11T08:35:49Z

(A nice way of learning Rust)

Schniz · 2023-11-11T08:36:50Z

Ah now I see you know Rust haha

ryanccn · 2023-11-11T10:43:58Z

Cool idea! I think we can receive a semver range like “install” and “use”. Should not be hard to implement too. Are you interested in doing so?

@Schniz I've refactored it to take UserVersion, so now it accepts the same version specifications as install and use 👍

ryanccn · 2023-11-11T10:54:41Z

Might be nice if it can omit earlier releases, most of the time you may only be interested in seeing a list of the latest versions sorted by major (or just LTS).

@polarathene I've added support for filtering LTSes with --lts and optionally specifying the LTS name, I think adding major version support to the option won't be necessary because just using the filter argument would work.

Edit: Your example picks out the latest versions from each LTS, should I make that the behavior here as well?

polarathene

I'm not too familiar with the project but hopefully the review helps :)

src/remote_node_index.rs

polarathene · 2023-11-13T05:15:24Z

src/commands/ls_remote.rs

+ /// Only show latest LTS versions
+ #[arg(long)]
+ lts: Option<Option<String>>,


Does it need to be two layers of Option here?

EDIT: Yes, technically since it appears to be relying on:

None for false (no option / flag provided)

Inner None for true (aka --lts), no value provided

Some(Some(String)) to represent a version codename to filter against (eg: --lts iron).

It can technically be worked around with a variable arg number, or using an enum similar to LtsStatus.

From the related functionality with retain() I didn't see something that indicated it also filters to "latest LTS versions", is that doc comment accurate? If so a bit more context for the relevant logic in the apply() method might be worthwhile?

EDIT: Or was it meant to be only the current LTS release series as default for --lts, rather than all LTS series?

@njzydark in #944 actually took the "bool or string" enum approach I had in mind.

They also contributed improvements to docs and tests that should be adapted here for the --lts feature? I kind of thought both PR would be reviewed/merged separately 😅

@ryanccn I think you can take a look at my pr #944

I think you can take a look at my pr #944

That is what I suggested 😝

My last review provides a checklist with two items referring to your PR as good additions (docs + tests).

src/commands/ls_remote.rs

polarathene · 2023-11-13T07:57:24Z

.changeset/fifty-emus-type.md

+"fnm": minor
+---
+
+feat: add remote version sorting and filtering


I'm not too familiar with how this changelog entry is handled, but might be relevant to include the actual flags / examples here to appear in the release notes?

Not sure either, seems like this was added fairly recently

src/commands/ls_remote.rs

polarathene · 2023-11-13T09:37:30Z

@polarathene I've added support for filtering LTSes with --lts and optionally specifying the LTS name

That's great, thanks! ❤️

I think adding major version support to the option won't be necessary because just using the filter argument would work.

Edit: Your example picks out the latest versions from each LTS, should I make that the behavior here as well?

That might be nice to have too. I suppose it should be an additional flag like --latest?

I assume the current PR would output something like:

$ fnm list-remote --filter 20
v20.0.0
v20.1.0
v20.2.0
v20.3.0
v20.3.1
v20.4.0
v20.5.0
v20.5.1
v20.6.0
v20.6.1
v20.7.0
v20.8.0
v20.8.1
v20.9.0 (Iron)


$ fnm remote-list --lts iron
v20.9.0 (Iron)


$ fnm remote-list --filter 20 --lts
v20.9.0 (Iron)


$ fnm remote-list --lts hydrogen
v18.12.0 (Hydrogen)
v18.12.1 (Hydrogen)
v18.13.0 (Hydrogen)
v18.14.0 (Hydrogen)
v18.14.1 (Hydrogen)
v18.14.2 (Hydrogen)
v18.15.0 (Hydrogen)
v18.16.0 (Hydrogen)
v18.16.1 (Hydrogen)
v18.17.0 (Hydrogen)
v18.17.1 (Hydrogen)
v18.18.0 (Hydrogen)
v18.18.1 (Hydrogen)
v18.18.2 (Hydrogen)


$ fnm remote-list --lts
// ... many more
v18.17.1 (Hydrogen)
v18.18.0 (Hydrogen)
v18.18.1 (Hydrogen)
v18.18.2 (Hydrogen)
v20.9.0 (Iron)

Where --latest logic would be fairly straight-forward (take the last item from all_versions after sorting, but before reverse()), except for the final one (latest of each LTS):

$ fnm list-remote --filter 20 --latest
v20.9.0 (Iron)

$ fnm remote-list --lts iron --latest
v20.9.0 (Iron)

$ fnm remote-list --filter 20 --lts --latest
v20.9.0 (Iron)

$ fnm remote-list --lts hydrogen --latest
v18.18.2 (Hydrogen)

$ fnm remote-list --lts --latest
// ... same with earlier LTS
v18.18.2 (Hydrogen)
v20.9.0 (Iron)

Differs from UX of fnm install --lts / fnm install --latest / fnm install 20 if that's a concern?

I think you can implement it this way though with dedup_by_key? Sort order may matter as context:

// Now defaulting to descending:
// Descending order is necessary for selecting latest
value.sort_by_key(|v| std::cmp::Reverse(v.version));

// `--latest` Needs to be done after sort
// Then can flip sort order for ascending afterwards
if latest {
    // Technically should filter by major version
    // or if only for `--lts` by `LtsType::CodeName()`?
    value.dedup_by_key(|v| v.version);
}

// Presentational:
if let SortingMethod::Ascending = sort {
    value.reverse();
}

Technically that's only relevant for --lts (Some(None)), so if it needed to be optimized you could complicate it a little with two separate implementations.

Or I'm over thinking it :)

If it's not minimal to add, perhaps defer to a follow-up PR?

polarathene

Use truncate() for --latest.
Update the project docs file (like was done here).
- @Schniz may chime in if they want the new options included in the changeset file too 🤷‍♂️ (your PR description listing each addition seems like good content for that 👍 )
Optional: Better clarify --lts help text (some possible choices suggested).
Optional: Consider adding some tests (like was done here).

After that should be good for @Schniz to handle final review 🎉

src/commands/ls_remote.rs

Schniz · 2023-11-14T13:07:52Z

just see how stupid I am: I was saying that "I will accept a PR implementing this request", on a PR, that implements this feature. Sorry :lol:

Co-authored-by: Brennan Kinney <[email protected]>

vercel · 2024-02-29T07:28:52Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fnm	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 27, 2024 9:36am

vercel bot deployed to Preview October 25, 2023 15:54 View deployment

ryanccn force-pushed the list-filter branch from 7bc998d to 1f397e8 Compare October 25, 2023 15:54

vercel bot deployed to Preview October 25, 2023 15:54 View deployment

ryanccn force-pushed the list-filter branch from 1f397e8 to 1a0777d Compare October 25, 2023 15:56

vercel bot deployed to Preview October 25, 2023 15:56 View deployment

polarathene mentioned this pull request Nov 11, 2023

fnm install usability #1035

Open

Schniz added the good first issue Good for newcomers label Nov 11, 2023

vercel bot deployed to Preview November 11, 2023 10:43 View deployment

vercel bot deployed to Preview November 11, 2023 10:53 View deployment

vercel bot deployed to Preview November 11, 2023 11:06 View deployment

polarathene suggested changes Nov 13, 2023

View reviewed changes

vercel bot deployed to Preview November 13, 2023 12:32 View deployment

vercel bot deployed to Preview November 13, 2023 12:38 View deployment

ryanccn changed the title ~~feat: add remote version sorting and filtering~~ feat: add filtering options to ls-remote Nov 13, 2023

vercel bot deployed to Preview November 13, 2023 12:44 View deployment

vercel bot deployed to Preview November 13, 2023 12:45 View deployment

polarathene suggested changes Nov 14, 2023

View reviewed changes

src/commands/ls_remote.rs Outdated Show resolved Hide resolved

src/commands/ls_remote.rs Outdated Show resolved Hide resolved

src/commands/ls_remote.rs Outdated Show resolved Hide resolved

vercel bot deployed to Preview November 15, 2023 13:10 View deployment

ryanccn added 3 commits February 29, 2024 15:28

feat: add remote version sorting and filtering

934ec4f

use UserVersion for filtering

1e2f09a

add additional lts filter

38a0780

ryanccn and others added 10 commits February 29, 2024 15:28

refactor: use Vec::retain instead of filtering and collecting

3abd284

fix docstring for sort option

c82e79e

change docstring for filter argument

5079ea8

refactor vec filtering

d29c77b

Co-authored-by: Brennan Kinney <[email protected]>

refactor to use sort_by_key and reverse

84523ef

Co-authored-by: Brennan Kinney <[email protected]>

add latest flag

9d8f809

make filter an option as well

cb179dc

move sort into command as it is presentational

86191ba

fix getting latest version

76bd090

refactors

81e6532

ryanccn force-pushed the list-filter branch from c88a84e to 81e6532 Compare February 29, 2024 07:28

vercel bot deployed to Preview February 29, 2024 07:29 View deployment

Merge branch 'master' into list-filter

d587f61

vercel bot deployed to Preview May 27, 2024 08:12 View deployment

Schniz added 5 commits May 27, 2024 11:33

Merge remote-tracking branch 'origin/main' into list-filter

5674423

update docs/commands.md

c019f1c

sort upon installation

769b96e

fix clippy

ab5ea61

don't mention rust structs in the cli docs

b0903b5

Schniz added the automerge label May 27, 2024

Schniz added 2 commits May 27, 2024 12:35

sort by default

516c6d8

remove leftover temp dir from test

a436254

vercel bot deployed to Preview May 27, 2024 09:36 View deployment

kodiakhq bot merged commit 121128f into Schniz:master May 27, 2024
17 checks passed

github-actions bot mentioned this pull request May 27, 2024

Version Packages #1052

Merged

BrewTestBot mentioned this pull request May 28, 2024

fnm 1.36.0 Homebrew/homebrew-core#172993

Merged

Schniz mentioned this pull request May 29, 2024

feat(ls-remote): add lts option to show only lts version #944

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add filtering options to `ls-remote` #1063

feat: add filtering options to `ls-remote` #1063

ryanccn commented Oct 25, 2023 •

edited

changeset-bot bot commented Oct 25, 2023 •

edited

polarathene commented Nov 11, 2023 •

edited

Schniz commented Nov 11, 2023

Schniz commented Nov 11, 2023

Schniz commented Nov 11, 2023

ryanccn commented Nov 11, 2023

ryanccn commented Nov 11, 2023 •

edited

polarathene left a comment

polarathene Nov 13, 2023

polarathene Nov 13, 2023 •

edited

polarathene Nov 13, 2023

njzydark Nov 16, 2023 •

edited

polarathene Nov 16, 2023

polarathene Nov 13, 2023

ryanccn Nov 13, 2023

polarathene commented Nov 13, 2023

polarathene left a comment •

edited

Schniz commented Nov 14, 2023

vercel bot commented Feb 29, 2024 •

edited

feat: add filtering options to ls-remote #1063

feat: add filtering options to ls-remote #1063

Conversation

ryanccn commented Oct 25, 2023 • edited

changeset-bot bot commented Oct 25, 2023 • edited

🦋 Changeset detected

polarathene commented Nov 11, 2023 • edited

Schniz commented Nov 11, 2023

Schniz commented Nov 11, 2023

Schniz commented Nov 11, 2023

ryanccn commented Nov 11, 2023

ryanccn commented Nov 11, 2023 • edited

polarathene left a comment

Choose a reason for hiding this comment

polarathene Nov 13, 2023

Choose a reason for hiding this comment

polarathene Nov 13, 2023 • edited

Choose a reason for hiding this comment

polarathene Nov 13, 2023

Choose a reason for hiding this comment

njzydark Nov 16, 2023 • edited

Choose a reason for hiding this comment

polarathene Nov 16, 2023

Choose a reason for hiding this comment

polarathene Nov 13, 2023

Choose a reason for hiding this comment

ryanccn Nov 13, 2023

Choose a reason for hiding this comment

polarathene commented Nov 13, 2023

polarathene left a comment • edited

Choose a reason for hiding this comment

Schniz commented Nov 14, 2023

vercel bot commented Feb 29, 2024 • edited

feat: add filtering options to `ls-remote` #1063

feat: add filtering options to `ls-remote` #1063

ryanccn commented Oct 25, 2023 •

edited

changeset-bot bot commented Oct 25, 2023 •

edited

polarathene commented Nov 11, 2023 •

edited

ryanccn commented Nov 11, 2023 •

edited

polarathene Nov 13, 2023 •

edited

njzydark Nov 16, 2023 •

edited

polarathene left a comment •

edited

vercel bot commented Feb 29, 2024 •

edited