Tracking issue for `#[doc(spotlight)]`

[QuietMisdreavus]

Initially implemented in #45039, this attribute puts your own trait on every function doc if its return type implements it.

[jethrogb]

Couple of observations:

1. The exclamation point is to the left of the function signature, but it relates to the return type on the right. This can be confusing, especially if the trait implementations are not exactly the return type of the function (&[T] -> &[u8], see next comment).
2. It seems pretty strange to me that https://doc.rust-lang.org/nightly/std/slice/struct.Iter.html#impl shows Read/Write for &[u8]. I'm probably not that interested in that.
3. I think it would be more useful to me if I could force an ordering of impls on a doc page, so that the type author can decide what impls are relevant. For example, in https://docs.rs/hyper/0.10/hyper/client/request/struct.Request.html there's all this stuff which is mostly irrelevant and then all the way at the bottom you see theres impl Write which is what you actually need to put data in the request. (Although this particular example might just have been improved by adding a doc example at the top.)

[kennytm]

I think it is more intuitive if the dialog is a popover that shows those "important traits" directly next to the ⓘ (mouseover will show it temporarily, click to make it stay).

[jonhoo]

I think it'd be handy to write up guidelines for when spotlight should be added to a trait. I had to search pretty deep through the history of this feature until I found out what the original use-case was. There's also another question of why this is a whitelist instead of a blacklist? Aren't trait implementations usually important, except those that are extremely common (like Clone, PartialEq, etc.)?

The '🛈' sign being to the left of the function signature was confusing to me. I expected it to provide extra information about the function but it's actually related to the return type Iter<'a, T>. Perhaps putting it next to Iter<'a, T> would be better-aligned with its purpose?

My intuition is that the spotlight feature should really be just a simple balloon (tooltip) that shows up when hovering over '🛈' or Iter<'a, T>. So when you ask yourself "what's this return type all about?", you don't have to follow the link and scroll through the type's trait implementations, but instead you see a balloon that gives you a quick overview summarizing its trait implementations.

Maybe we don't even need the '🛈' sign - maybe it's enough to just show the balloon when hovering over the type. Also, clicking on '🛈' is a bit cumbersome - it feels like too much UI interaction for a quick peek into the return type. Finally, I'd probably prefer the balloon showing up when hovering over any occurence of the type Iter<'a, T> in the documentation, rather than in return type positions only.

[durka]

FWIW I don't like the mouseover effect. It's surprising when I trigger it unintentionally, making things worse by covering the function signature I was trying to read. And I agree it should be somehow visually associated with the return type, instead of the current confusing location.

[QuietMisdreavus]

@jonhoo The reason it was a whitelist was because i originally wanted this feature to be for "fundamental" traits, those that can effectively define an entire type. Things that implement Iterator are typically just there to be an Iterator. When i first got started on the implementation, i only put the spotlight attribute on Iterator, until i got a recommendation to also put it on Read and Write as well. The reason i didn't just put a static listing inside rustdoc was so that i could apply this to Future as well, since it has the same "fundamental" nature as Iterator.

However, putting every trait on there would blow out the window and possibly make it even worse than not having it there in the first place. Note that the idea came from #25928, and the fact that i wrote a generalized solution was mainly for Future, as mentioned earlier. (And also so i wouldn't have to make rustdoc specially dig out the Iterator trait, or some other static whitelist.)

@jethrogb Oh wow, i didn't even notice that &[T] -> &[u8] thing. I'll take a look at that and see what's up.

---

I initially wanted the circle-i icon to be next to the return type as well, but when @GuillaumeGomez put this UI together he said that we couldn't guarantee them appearing next to each other, in situations like line-wrapping and the like. There may have been issues getting everything together for the print as well. I wonder how much of a problem it would be to try to stick them next to each other.

[jonhoo]

@QuietMisdreavus ah, I see. I wonder if the name spotlight should be changed to better hint at that being the target use-case, rather than just "show this as special". Essentially, the name should communicate the semantic implications, more so than the desired visual outcome. It's too verbose, but something along the lines of #[doc(is_likely_primary_impl)]. Maybe #[doc(fundamental)] or #[doc(dominant)]? #[doc(notable)] also isn't terrible, but loses the "this is likely the primary behavior of implementors. An argument could also be made for #[doc(essential)], in that these traits are likely the essence of any implementing type, but unfortunately "essential" also carries may other connotations (along these lines, "constitutive" is great, but too obscure, and "intrinsic" is already used elsewhere).

Anyway, that's a lot of bikeshedding. I do think picking a more semantic name will lead to much less confusion for users though!

[durka]

Maybe put a box around the return type or something? The current design strongly suggests something is special about the function itself, which is misleading.

…

On Fri, Jan 12, 2018 at 4:25 PM, Jon Gjengset ***@***.***> wrote: @QuietMisdreavus <https://github.com/quietmisdreavus> ah, I see. I wonder if the name spotlight should be changed to better hint at that being the target use-case, rather than just "show this as special". Essentially, the name should communicate the semantic implications, more so than the desired visual outcome. It's too verbose, but something along the lines of #[doc(is_likely_primary_impl)]. Maybe #[doc(fundamental)] or #[doc(dominant)]? #[doc(notable)] also isn't terrible, but loses the "this is likely the primary behavior of implementors. An argument *could* also be made for #[doc(essential)], in that these traits are likely the *essence* of any implementing type, but unfortunately "essential" also carries may other connotations (along these lines, "constitutive" is great, but too obscure, and "intrinsic" is already used elsewhere). Anyway, that's a lot of bikeshedding. I do think picking a more semantic name will lead to much less confusion for users though! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#45040 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n87eb4793wKRoq8BKLMoZYQhTQwuks5tJ83SgaJpZM4Pugq7> .

[QuietMisdreavus]

I like #[doc(fundamental)], or maybe #[doc(important)] to match the current text of the pop-up window. (Though i wonder whether we should rename the feature gate at the same time, and whether there's any kind of funny business that needs to happen in that case. >_>) The current #[doc(spotlight)] was just a random name that came to mind as i was dreaming up how the feature would be implemented, so i'm not that attached to it, heh.

[GuillaumeGomez]

Maybe we should open a survey to pick a name?

[jonhoo]

I don't think renaming the feature should be too problematic. It's a nightly unstable feature for a reason! #[doc(important)] is problematic in the same way as #[doc(spotlight)] to me -- why is it important? Plenty of things are important (e.g., Sync, Clone, Debug, arguably most of the deriveable traits), but we don't want to highlight all of them. We want to highlight this one because we believe that the primary way you'll interact with an implementing type is through the marked trait. And the name should communicate that. #[doc(primary)] might even be a good fit, though "primary" already has so many other meanings...

@GuillaumeGomez I don't think it's outrageous to spend some time getting this name right before stabilization. I do agree with your sentiment that we shouldn't lament too much over this decision, but spotlight to me is too vague to be discoverable.

[GuillaumeGomez]

Yes, that's why I suggested to open a survey. I don't have any personal preference on this topic and I have the feeling that's the case for everyone. So unless someone has a proposition, let's just open a survey or something along the lines. :)