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 <5098581+polarathene@users.noreply.github.com>

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

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 5 commits February 29, 2024 15:28

feat: add remote version sorting and filtering

934ec4f

use UserVersion for filtering

1e2f09a

add additional lts filter

38a0780

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

3abd284

fix docstring for sort option

c82e79e

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

change docstring for filter argument

5079ea8

refactor vec filtering

d29c77b

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

refactor to use sort_by_key and reverse

84523ef

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

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

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

sdavids mentioned this pull request Aug 21, 2024

Add --lts flag for ls-remote #886

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

Loading

changeset-bot bot commented Oct 25, 2023 •

edited

Loading

polarathene commented Nov 11, 2023 •

edited

Loading

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

Loading

polarathene left a comment

polarathene Nov 13, 2023

polarathene Nov 13, 2023 •

edited

Loading

polarathene Nov 13, 2023

njzydark Nov 16, 2023 •

edited

Loading

polarathene Nov 16, 2023

polarathene Nov 13, 2023

ryanccn Nov 13, 2023

polarathene commented Nov 13, 2023

polarathene left a comment •

edited

Loading

Schniz commented Nov 14, 2023

vercel bot commented Feb 29, 2024 •

edited

Loading

feat: add filtering options to ls-remote #1063

feat: add filtering options to ls-remote #1063

Conversation

ryanccn commented Oct 25, 2023 • edited Loading

changeset-bot bot commented Oct 25, 2023 • edited Loading

🦋 Changeset detected

polarathene commented Nov 11, 2023 • edited Loading

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 Loading

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 Loading

Choose a reason for hiding this comment

polarathene Nov 13, 2023

Choose a reason for hiding this comment

njzydark Nov 16, 2023 • edited Loading

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 Loading

Choose a reason for hiding this comment

Schniz commented Nov 14, 2023

vercel bot commented Feb 29, 2024 • edited Loading

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

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

ryanccn commented Oct 25, 2023 •

edited

Loading

changeset-bot bot commented Oct 25, 2023 •

edited

Loading

polarathene commented Nov 11, 2023 •

edited

Loading

ryanccn commented Nov 11, 2023 •

edited

Loading

polarathene Nov 13, 2023 •

edited

Loading

njzydark Nov 16, 2023 •

edited

Loading

polarathene left a comment •

edited

Loading

vercel bot commented Feb 29, 2024 •

edited

Loading