Feedback on "Important Traits" rustdoc feature

Yeah, I think this approach is really good. I guess the next step will be to highlight Iterator on https://doc.rust-lang.org/nightly/std/iter/struct.Enumerate.html somehow? :smiley:

I feel like that's already done by mentioning it in the docs. The reason it needs to be highlighted on the other page is so you know "this is just an iterator, and it's an iterator over XYZ".

Great feature, very useful imo. The placement is also good and obvious. It doesn't particularly matter to me whether "Important" is the right word, but I wish that the respective line would be formatted less conspicuously, to bring the real content (the traits themselves) front-and-center, but it's more of nitpick than anything else.

I find the feature useful, but I think it should pop out more. The tiny "(i)" button can easily be overlooked, to the point where it took me quite some time to learn about the existence of the original feature.

I really like this version, but I think it can be improved even more by replacing the leading semi-colon by impl:

  • impl Iterator<Item=(usize, Self::Item)>

This way it would match what a user would write (when impl Trait in Trait will be supported).

I have been thinking about a similar feature for inherent methods and trait methods.

The problem is that some types and traits have a huge amount of methods which are sometimes very useful, but tend to bury the most important/common methods in the documentation. (Looking at you, Option and Iterator!)

I think it would be useful if you could highlight some methods in the documentation, putting them at the front and maybe in a separate subsection, by marking them as #[doc(important)]

I know you can get similar results by using the option to sort methods by appearance rather than alphabetically, but I feel that has several disadvantages:

  • it's still unstable
  • it's a global switch, requiring you to reorganize all your source code to make best use of it
  • you need to establish a total order of all methods, even when you only want to highlight a few particular ones. Alternatively you need to sort the others alphabetically by hand
  • I think it can worsen the readability of your code: say you have (among others) a get_mut_unchecked() method and a get_mut() method implemented by a call to the former. Logically they should be grouped together, but if you want to emphasize the safe variant you need to put them in different places in your module.

Therefore I think a way to highlight methods with #[doc(important)] would be useful. IDEs could also use this information to improve their suggestions.

Alternatively we could have an attribute setting the priority of the method in the documentation like #[doc(priority=42)]. That would allow sorting all methods by priority first and then within one priority either alphabetically or by source code order. Items without an annotation would have priority zero and items with a positive priority could go into a special subsection.

You could also have negative priorities for items that should be collapsed.

2 Likes

The default for methods is to sort by source order (including partitioning by separate impl blocks); --sort-modules-by-appearance only appears to be for affecting the order of items on modules documentation pages.

If we use the -> Type impl SomeTrait, I think it can be always displayed (instead of being a pop-up).

1 Like

It looks really good for a single trait, but if for some reason the returned types implements two important trait, how do you display it? impl Iterator<(usize, T)> and SomeOtherTrait as Enumerate<T> doesn't feels right. I would propose instead -> Enumerate<T>: impl Iterator<(usize, T)> + SomeOtherTrait

1 Like

Am I the only one who a priori couldn't grok that "important traits for T" meant "important traits that T implements"? The "for" there seems awfully generic even if it does appear in the impl for syntax. Maybe "Key implemented traits" or somesuch?

3 Likes

FWIW, another possible syntax, which is currently legal rust, would be:

fn ... -> Enumerate<Self>
where
    Enumerate<Self> : Iterator<Item = (usize, Self::Item)>,
3 Likes

No, this is wrong. : impl is two steps of the “:” relation where you only want one.

x : impl Trait means: there exists a type T such that x: T and T: Trait.


I suppose a non-hidden : Trait does work quite nicely though:


(Note: if something like this was to be generated automatically there would need to be a lot of special logic in place to keep the information as concise as possible, especially around types like Flatten or Zip)

It also works with multiple traits (see fuse above).

You are totally right, thanks for the correction.


I think that it's going in the right direction, but it still feels (at least for me) awfully too verbose.

Taking the example of zip, a more detalis button should give all the details that @steffahn's mock-up displays, but the minimal documentation we need is just

fn zip(self, other: Iterator) -> impl Iterator<Item=(Self::Item, other::Item)

However I don't know how to reach this level of conciseness automatically (especially since I replaced U::item by other::Item).

Yeah so that's why I moved it right next to the return type. I'm hoping that's sufficient.

A concern I have here is that this misses out where clauses and also won't work for multiple traits, if we care about that.

I think that's easy to do by reordering the methods in source.


I'm against the non-hidden : impl Trait, I think it gets too noisy and will be confusing, especially if multiple traits are involved or if where clauses are involved.

FWIW my current plan is to land @XAMPPRocky's CSS suggestions (Guillaume made a pull request, and change the wording to "Notable traits for Type". We can move on from there.

Note that currently it doesn't actually check if the type implements the trait, e.g. it erroneously says Box<str> implements Iterator because there's an impl on Box (https://github.com/rust-lang/rust/issues/74417). Before making it pop out further I'd like that to be fixed.

How could third-party crates add key traits?

@pickfire #[doc(spotlight)]

3 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.