Feedback on "Important Traits" rustdoc feature

Rustdoc has for a while had an "important traits" feature wherein functions returning structs that implement notable traits (like Iterator, Future, Read, and Write) have those traits mentioned in a popup.

In February it was removed for being confusing, I recently improved and re added it with the goal of solving many of the problems the original feature had.

This is what it currently looks like on hover (you can try it out here):

image

The basic idea behind this feature is that there are some traits which are typically the most important thing about a type implementing them. When you call enumerate(), you don't really care about Enumerate, you care about impl Iterator, and you care what the Iterator iterates over. This feature brings this information front and center, without needing to click through to the page for Enumerate.

Some questions:

  • What do y'all think of this feature? Do you think it will be useful?
  • What do you think of the label "important traits"? Is there something that could be used here that is more descriptive?
  • What do you think of the UI/placement of this feature? Is it obvious enough?
  • Do you think the choice of traits to mark "important" is good?
12 Likes

An alternative name would be "key traits". They're both key as in important/central to using the type, and in the sense of "unlocking" the useful functionality.

As a side note, it might be useful to also be able to mark "key traits" of a type on the type rather than the trait, if a type is only/primarily used through a trait but the trait itself doesn't usually fit that category.

5 Likes

My personal experience, never really understanding this ā€œimportant traitsā€ business when I used to see it (which meant I just ignored it after a while): The naming ā€œimportant traitā€ sounds/sounded, to me, like the trait inherently being important. Which then was confusing because it seemed that a lot of traits that seemed important to me werenā€™t mentioned. In this case what it really is is just a trait that is important for or perhaps in other words relevant for a specific type.

In other words: I think naming should be improved so that it does become obvious that it is only for this specific type that the trait is considered important.

I donā€™t remember what it used to be like, in the screenshot you provide here, I do see that it tries to communicate being about the importance of the specific type-and-trait-pair, but it does so in an ambiguous way: One could just as well think that Iterator is just, globally, one of a few ā€œimportant traitsā€ and thereā€™s some info here indicating that Enumerate implements it.

Nonetheless the placement is great. Even if you misinterpret the whole ā€œimportant traitsā€ rationale, the main point stands that you can just hover over the ā“˜ in the signature and immediately understand the signature a lot better.

Now that @CAD97 has also posted an answer, I like the idea of ā€œkey traitsā€. I couldā€™ve also imagined ā€œfeaturedā€ or ā€œnoteworthyā€, all these alternatives seem, to me, to better emphasize that their importance appears only in the context of the particular type in question.


Edit: Nevermind, on a second read of your post, Iā€™m back to thinking that you actually meant that traits being ā€œimportantā€ is an absolute property of the trait. So back to my concern that ā€œimportanceā€ of traits like ā€œIteratorā€ seems confusing when basic (IMO) important traits like Copy or Sync are somehow ā€œunimportantā€. Perhaps changing this model is an option. And the screenshot are still ambiguous about how this works either way.

2 Likes

I personally liked it, a lot of times I keep on what Item a type that implements Iterator iterates over, so I do wish I had that on docs generated and stored on my machine

1 Like

Yeah an alternative naming I was considering was "Notable traits"

I like "Key traits". I wonder if it could become "Key traits for Enumerate", to build on @steffahn's suggestion?

I have thought about marking impls instead of marking the traits: This would solve Spotlight shows traits with generic impls that do not apply to the specific type Ā· Issue #74417 Ā· rust-lang/rust Ā· GitHub immediately. However, this puts library authors on the hook for marking their impls; I'm not so sure we want to do that.

The core issue here is that Iterator/Future/Read/Write are traits which basically universally behave this way: If there is an impl of this trait on a type, it is very likely for that impl to be the most important thing about the type!

But it's possible this feature will be more useful if we ask for specific impls to be spotlit instead. It just means that people don't get it "for free" by implementing a trait.

A third option is to do both, you can tag impls and you can tag traits.

I donā€™t remember what it used to be like,

It used to be on the side

1 Like

I like the idea of highlighting important/notable/key traits of a type, but I don't like: tiny buttons, information that is hidden by default and needs interaction to be revealed, popups that cover the page.

I'm ok clicking through to type's own page if I don't know what it does. And then on that page I'd prefer these important traits to be highlighted.

I wish rustdoc could explain traits in prose: "This type is an Iterator of Item. It can be Read from."

11 Likes

On a second thought, this makes a lot of sense. Iā€™m not a practical Rust user (yet) at all, so I kind-of forgot for a moment that e.g. collections are never themselves iterators.

Thereā€™s another granularity, that is: individual functions. For example if you use the Either type from the either crate, you might want to return it for its Iterator impl in one place and for something else in another place.

I tend to think of some of these types like Enumerate of just a glorified impl Iterator return type with a + X, i.e. some extra trait implementations and also methods, so it seems particularly important to me that you can see those traits that the author of the function/method made sure his/her return type implements are visible in the documentation of that function/method.

Staying with iterators, for example the fuse method returns iter::Fuse and it is very noteworthy IMO in this particular case that Fuse implements FusedIterator. But for other iterators, especially those kind of adaptors that only propagate properties like FusedIterator, like iter::Map for example, it is not too important.

Going into the on-the-individual-function level granularity, thereā€™s also situations where you mostly only care about having this information on display very visibly on the documentation for the type, but with no particular function. I remember being puzzled for several minutes as to how to use tokio::sync:oneshot. There was the oneshot::channel function giving me the Sender and Receiver but the Receiver API seemed very confusing. I think I needed to google it to realize that the receiver implements Future and I could receive using .await.

2 Likes

A great example of some of my points above on granularity is AsMut which informs me about Iterator, Future, Read, and Write instances of &mut T, where I care about none of these. Compared to Iterator::by_ref where I care about Iterator, but still donā€™t carre about Future, Read, and Write. Also the AsMut page shows some rendering problems in the declaration where the semicolon is wayy off to the right after the ā“˜.

In a maximally complex tagging system, there could be a default for having impls for Iterator marked noteworthy (that is, on the trait level), with an opt-out for the particular impl for &mut T with a re-opt-in for the particular method by_ref.


Also the current approach seems to (AFAICT) not cover nested occurrences. This is unfortunate because, for comparison, something like Result<impl Future<...>> is a thing that works. If I replace this (e.g. to add a method or allow users to name the type) with a custom ā€œglorified impl Future typeā€ like so: fn foobar(...) -> Result<Foobar>, I would care a lot to still have the information that Foobar implements Future visible in the documentation of foobar.

2 Likes

Yeah that's mostly a matter of https://github.com/rust-lang/rust/issues/74417 , but that might be tricky to fix.

Tagging functions is an easy way to do this, tagging impls is another. But i'd be sad to lose the "you get it for free" that downstream crates get.

On the bikeshed of the "Important", I think I would have a preferennce for something like "useful" that conveys their utility to the user.

On mobile this popover should probably always be the full width of the viewport otherwise for some methods it can be quite difficult to read.

3 Likes

I think the noun form of "key" could muddle the interpretation of "key traits" up without more context, because the intended meaning relies a little too heavily on metaphor to jump from "thing that unlocks something" and there's also less likely interpretations like "the things on keyboards" as background noise.

Aside from "featured", "noteworthy", or "notable", "significant" would also work.

When it's unclear what that qualifications for noteworthiness or significance are, it doesn't lead to any confusion about what those terms mean.

1 Like

Another point I would like to make: While I personally find trait impls to be presented in a way too verbose manner throughout rustdoc, this kind of tool-tip situation is particularly bad in that it still tries to replicate the whole source code of a trait impl.

Just to note a few points in particular: declaring variables on trait impls in rustdoc is redundant since varaibles are distinguished from types by color-coding anyways. Another thing is the verbosity around things like <T as Iterator>::Item which no-one would even write in actual source-code. Iā€™ve created an example image of how I would envision an improved, more concise alternative:

Currently:

Improved:

Or perhaps, in different wording:

Edit: An even more minimalistic format (would need further analysis to determine Self: Iterator):

2 Likes

Currently on my phone, but making it easier to discover that Iterator<T>::enumerate() -> impl Iterator<(usize, T)> is definitely something that's on my "high impact" list of things for making rust more accessible. I hadn't realised that it had been there but removed. I'm glad you've been giving it some love.

When thinking about it, I was thinking of putting the annotation in the return signature of the trait methods, like fn enumerate(self) -> impl Iterator<(usize, T)> as Enumerate<T>. My thinking was that the std traits predate impl Trait but if they didn't then they would probably have just returned impl Trait and kept the concrete type hidden. The return type could then be read as "I return impl Iterator, but you can also rely on the return type being the concrete type Enumerate for historical reasons" even when using "go to definition and reading the trait's source code".

This is not true, they could not just return impl Trait. As I mentioned in a comment above (where I call this pattern a ā€œglorified impl Trait + Xā€ (X stands for extra)), there is more to a type like Enumerate than that it implements Iterator.

There are additional methods and trait impls that matter. For example Enumerate<I> also implements Clone, Debug, DoubleEndedIterator, ExactSizeIterator, and FusedIterator, depending on which ones I implements. Other types such as Peekable offer extra methods while also being an Iterator.

2 Likes

"Key" also collides with terms common in APIs where this information is likely to be presented, while "important" is less likely to have such collisions. For example, seeing "Key traits" on the return value of BTreeMap::entry would be very confusing. This makes "important" a clearer term than "key". "Pertinent" is more correct, but may be too uncommon to be accessible. On those criteria "useful traits" seems the best suggestion so far.

1 Like

I would just call it "required trait" or "related trait" based on the actual criteria used to select it. There's no need to invent new terminology here.

Yeah I tried to improve it and couldn't get anything good, would you be able to try some CSS and see what you get?

(admittedly when I tried this there were other issues that needed fixing)

This kinda works for Iterator but less so for things like Read and Write where the return type also matters outside of those traits.

You'll want to wrap this in an appropiate media query for phone sizes only but adding this worked for me.

.important-traits .important-traits-tooltiptext {
    left: 0;
    top: 100%;
}

1 Like

@XAMPPRocky Mind making a PR? I think we already have some media queries there, if not, feel free to add one.

Unfortunately I don't have the time right now, I tested this through web inspector. If anyone else here wants to make a PR in my place feel free.

1 Like