Limit the number of crate features that are displayed as result of cargo add

[ranile]

Problem

Some crates have way too many features (such as web-sys). cargo add lists every feature when adding a crate and could potentially even overflow terminal buffer (it just did for me with web-sys using console)

Proposed Solution

Limit the number of features that are displayed. The limit should preferably be configurable but a hard-coded one is also fine.

If the feature list is truncated, cargo should link to docs.rs feature list for that crate

Notes

No response

[weihanglo]

Sounds interesting. How bad is it now, could you share some screenshots?

[epage]

Following the link for web-sys

This version has 1566 feature flags, 0 of them enabled by default.

1566 is nuts.

I think, at minimum, we should show the enabled features. If so, we should make sure we indicate that it isn't all.

The question is how we should deal with this

1. First X entries with a "... X more entries"
2. First X entries, "... Y more entries", last Z entries
3. "... X more entries"

When the list is long like this, I feel like there is little value to first/last. The chance that any of those features matter is probabilistically small. I would lean towards Option 3 (only showing ...)

If the feature list is truncated, cargo should link to docs.rs feature list for that crate

1. We'd need to be careful that we detect for a crates.io package and decide on a fallback when its from a different source
2. Its still not guaranteed that the package will be there. crates.io first checks the if it exists before showing the link

We could generically link out to package.documentation, making the user figure out where to find the information.

Also, a link might be fairly long for putting in output mixed with text.

My proposal is that we use ANSI escape codes for embedding a link to package.documentation in the "X more entries" text. For terminals that support it, they get a helpful link. For everyone else, they will have to manually get the link but thats no worse off than if we don't provide it.

Short term, we should probably do this via fwdansi so we make sure it gets processed correctly (if fwdansi can handle it). Otherwise, we should wait until we switch to anstream (which I'm working on) so it will be handled automatically without any extra work.

If need be, we could decouple this link work from the rest of the work so we don't block the whole thing.

[epage]

If there aren't any concerns, my proposal is

• Hard coded limit of <50 features
• Always show enabled features
• If past hard limit, collapse all remaining features into "... X more features"
• Defer out including a link until anstream is in (ie create a separate issue to track that once this is closed as fixed)

I'm willing to help mentor someone on this so I've marked it as Accepted. You can reach me here, on zulip, or during office hours

[Angelin01]

@epage I think I could tackle this throughout this week and next week given some extra information and some style/convention guidance. I just have a few questions regarding your proposal:

• Always show enabled features

I understood this as "Show all enabled features regardless of the hard coded limit", but won't this lead to the same problem as this issue describes if someone enables a lot of features? If that's acceptable, then so be it.

Otherwise we could do something similar to what I propose below, just with a different message.

• If past hard limit, collapse all remaining features into "... X more features"

If I understood the above correctly, may I suggest renaming it to "... X more deactivated features"?
Also, do we print on the line with the - or after it?

      Adding example v0.1.2 to dependencies.
             Features:
             + activated1
             + activated2
             - deactivated1
             - deactivated2
             - ... 123 more deactivated features

vs

      Adding example v0.1.2 to dependencies.
             Features:
             + activated1
             + activated2
             - deactivated1
             - deactivated2
             ... 123 more deactivated features

We can also print "... 123 more active features" if we wanna collapse active features too.

I won't claim this just yet in case someone else can get to it before I do, but I'll try to get to it soon enough.

[epage]

I like explicitly stating "deactivating".

My assumption is someone won't have hundreds of activated features but I'm fine protecting against it by applying the limit to those as well and showing "X activated features\nX deactivated features".

I would assume the ... would not have a leading -. The - is a bullet and an indicator and it reads weird for ... to appear after the bullet

For myself, I don't see what benefit we'd be providing the user by listing some of the category that is collapsed (- deactivated 1). It takes up space and is just a fraction of the information so the chance of it being meaningful is relatively low. We're showing features to communicate what they activated and what they could activate and not a general feature viewer (that'd be more for a cargo info) .

[Angelin01]

@rustbot claim

[epage]

@Angelin01 the difference is in

For myself, I don't see what benefit we'd be providing the user by listing some of the category that is collapsed (- deactivated 1). It takes up space and is just a fraction of the information so the chance of it being meaningful is relatively low. We're showing features to communicate what they activated and what they could activate and not a general feature viewer (that'd be more for a cargo info) .

Showing the the partial feature lists in these long lists seems to have little utility and I was wanting it to not be there.

As an implementation note, for feat iin deactivated should be for (deactivated_printed, feat) in deactivated.iter().enumerate.

[Angelin01]

I think understand.

Should we just go with something like

      Adding example v0.1.2 to dependencies.
             Features:
             27 activated features
             123 deactivated features

Or would you rather we aggregate it just on one message? 150 total features, 27 activated, 123 deactivated?

My issue with that is that I feel like it changes the behavior of the output considerably based on some arbitrary threshold. How useful are those 5 ~ 10 features that we displayed as enabled in the original output versus the possibly 50 or more we collapsed?

[epage]

When activated features is above the limit, yes.

There are a couple of states

• Activated features below the limit, total below the limit: show all
• Activated features below the limit, total features above the limit: show activated, summarize deactivated
• Activated features above the limit meaning total features is above the limit: summarize both

My issue with that is that I feel like it changes the behavior of the output considerably based on some arbitrary threshold. How useful are those 5 ~ 10 features that we displayed as enabled in the original output versus the possibly 50 or more we collapsed?

If the threshold is high enough, then there is no concern because the size is so large that its unusable.

Keep in mind, cargo add is about adding dependencies. The feature list is a helper for people doing that and isn't meant to be feature viewer on its own.

[Angelin01]

• Activated features below the limit, total below the limit: show all

This remains the same as before:

      Adding example v0.1.2 to dependencies.
             Features:
             + foo
             + buzz
             - bar
             - zum

• Activated features below the limit, total features above the limit: show activated, summarize deactivated

Should look like this:

      Adding example v0.1.2 to dependencies.
             Features:
             + foo
             + buzz
             123 deactivated features

• Activated features above the limit meaning total features is above the limit: summarize both
  Should look like this:

      Adding example v0.1.2 to dependencies.
             Features:
             57 activated features
             123 deactivated features

I feel like these look fine.

Also, Github doesn't let us show this through comments, but both + - were colored green and red respectively. Would we like to add any color indication to the summaries? I find it unnecessary, but I'm also not into UX.

[Angelin01]

Hard coded limit of <50 features

Since we are on the topic of "useful information", I noticed during the previous development that 50, visually, still feels like a lot. On a 1080p monitor with a size 10pt font, that fills about 80% of my screen with a maximized terminal. I feel like most people running cargo add are also going to be using small, editor-embedded terminals or similar size reducing situations. Maybe a reduction to 20 ~ 30 maximum would be better. How do you feel?

Edit:
As a reference for anyone, the output of $LINES for my terminals are as such:

• Intellij embedded: 12
• Dropdown terminal: 23
• Full main screen (1080p): 50
• Full Laptop screen (768p): 34

[epage]

Let's go with 30. We can always tweak it in the future

(for me echo $LINES reports 65 but I have vertical monitors)

[ranile]

Why not try to read $LINES and use half of that? If that fails, then use 30 or whatever

[epage]

I'd recommend terminal_size crate for it, but yes, that could be an option.

[Angelin01]

It seems Cargo already depends on that crate, so it should be simple enough to add. I'd need to see how much complexity it adds during testing though.

[Angelin01]

Any opinion on the formats exemplified here? If no one is opposed, I'll consider it settled. I'll also assume to stick to no color on the summarized messages.

I'll open a draft PR with the new changes soon if we are OK. I'll try to use terminal_size if I can easily hook into it on the tests, otherwise I'll stick to the hard coded 30 for a first version.

[epage]

Yes, those examples are what I am envisioning.

I'll try to use terminal_size if I can easily hook into it on the tests, otherwise I'll stick to the hard coded 30 for a first version.

I wouldn't consider terminal_size as required for the initial implementation.