Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Group navigation items in the docs by source crate/module #105307

Open
Tracked by #130358
cmpute opened this issue Dec 5, 2022 · 20 comments
Open
Tracked by #130358

Group navigation items in the docs by source crate/module #105307

cmpute opened this issue Dec 5, 2022 · 20 comments
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@cmpute
Copy link

cmpute commented Dec 5, 2022

I posted a thread in IRLO for this feature request, and it seems nobody is against it. Therefore, I come to post an issue here.

For the documentation generated for a type, it seems great to me if the trait implmentations can be grouped by the name of source crate (even source module) in the left navigation panel. Usually, a type won't have too many methods but it can have numerous trait implmentations (especially for numberic related types such as BigUint and rug::Integer).

People usually don't browse the trait implementations by the alphabetic order, they do directly by searching. But sometimes if I don't know which trait I'm looking for, but just want to know what functionalities the type support, it will be great if I can find the implementations by the crate / module name. For example, the implementation of all the arithmetic traits (Add, Sub, etc) can be grouped by the core::ops module name.

Current behavior

(This documentation comes from dashu-int::UBig)

image

Proposed behavior

The style for the module names is to be defined.

image

@fmease
Copy link
Member

fmease commented Dec 5, 2022

@rustbot label C-enhancement T-rustdoc A-rustdoc-ui

@rustbot rustbot added A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 5, 2022
@cmpute
Copy link
Author

cmpute commented Dec 6, 2022

I just tried to implement this feature and got it basically working, right now the rendered page looks like:

image

There are two questions I'm not sure about:

  1. Should the module names be embedded in the title like the picture above, or they should be formatted as subtitles under the "Trait Implementation" section?
  2. Is it useful to have some rules to simplify the groups to prevent generating too many groups (such as don't create a group if there is only one trait in that group)
  3. Do we also group the traits in the sections for auto and blanket trait implementations?

@cmpute
Copy link
Author

cmpute commented Dec 7, 2022

Here is a new trial: page1, page2. In these examples, I combine all groups (modules) that only contains one trait into a single group called (ungrouped), and I also grouped the traits in sections of auto and blanket trait implementations.

@scottmcm
Copy link
Member

scottmcm commented Dec 7, 2022

Pondering: does this need the module information? Would crate headers be sufficient?

@cmpute
Copy link
Author

cmpute commented Dec 7, 2022

Pondering: does this need the module information? Would crate headers be sufficient?

What's the drawback of having module information? Group with modules can provide fine-grained categorization, but it does take more space in the side bar.

The benefit is especially noticable for traits in std / core, because there are various traits for different purposes in them.

@jsha
Copy link
Contributor

jsha commented Dec 9, 2022

Thanks for thinking about this problem @cmpute, and prototyping a solution!

I think this is not the right solution, though. You mention that most types have few methods and many traits, but I think that's not universally true. Plenty of types (particularly in std) have many methods and few traits.

And there are some traits that are extremely common, like Clone, Debug, and Eq. Your approach adds a lot of visual noise in the sidebar to give each of those traits its own heading, because they happen to be in separate modules. But the current status quo, where the traits are children of a single heading, allows convenient quick skimming so long as the type doesn't have too many trait implementations. For example:

You're right that for numeric types, the sidebar is incredibly noisy. This is particularly due to the nature of the mathematical traits, where each trait (e.g. Add) needs N implementations for the N numeric types it can interact with. Then multiply by another 2x since you need separate implementations for winding up on the left or right side of the +, and then another 2x if you want to implement for both T and &T, as dashu-int does. And that's just for a single trait! It explodes combinatorically.

Probably the core::ops traits are special enough that we should give them their own treatment in rustdoc. For instance: normally, the sidebar includes the trait parameters because that's an important part of the impl. However, for numeric types this is not so useful, since the type probably implements each trait for a predictable set of type parameters. Also, normally, the sidebar is presented in alphabetic order while the main documentation is presented in order of implementation.

What if we introduced a "Numeric Trait Implementations" section, in both the sidebar and the main document. It would handle all the traits from core::ops. For the sidebar, instead of listing Add<u8>, Add<u16>, etc., it would just list Add. In the main document, rather than a repetitive listing of all the impls, it would present a summary:

Add

UBig can be added to: u8, u16, u32, u64, u128, usize, and UBig, resulting in a UBig.
UBig can be added to IBig, resulting in an IBig.
&UBig can be added to: &u8, &u16, &u32, &u64, &u128, &usize, and &UBig, resulting in a &UBig.

Rustdoc could also detect when the operations are not symmetrically defined, and say e.g. "IBig can be on the left hand side of an add operation with UBig".

This would happen to solve another problem: In the std docs, the numeric primitive types are some of our largest pages, and the slowest to load. The massive number of implementations is not very informative to the reader. A summary would be much more informative. And while rustdoc tends to mimic the presentation of the source code, this is a place where it diverges: the numeric traits are almost never implemented by hand because it would be so tedious and error prone. Instead they are usually implemented by a macro. So there's no reason for the documentation to be so much more verbose than the source code.

@jsha
Copy link
Contributor

jsha commented Dec 9, 2022

Or perhaps a more useful format to present the summary would be a table:

UBig + u8 -> UBig commutative
UBig + u16 -> UBig
UBig + IBig -> IBig
IBig + UBig -> IBig

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

Thanks for your reply! I think it's a good idea to only document operator traits! But one concern come to mind is what about user defined "operator" traits? Such as num_traits::Pow, dashu_base::DivEuclid. Maybe it's better to create an attribute like #[doc(trait_collapse)] (naming is so hard lol), that can be applied to a trait, and then we group all the trait implementations with this mark into a compact form.

For combining the docs in the main document, I'm guessing it's not a perfect solution to combine the trait docs. Maybe it's okay for core/std docs, but I'm hoping for a more generalizable solution for other traits (including user defined ones). And what if the user indeed has specialized documentation for the implementation (though in this situation maybe it's okay to just not combine them)

I also want to mention another point I have when I proposed this change. One benefit of grouping the traits by module is that, the relevant traits are closer in the sidebar. For example, serde::Serialize and serde::Deserialize will be next to each other rather than pretty far away in an alphabetical order.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

Or perhaps a more useful format to present the summary would be a table:

UBig + u8 -> UBig commutative
UBig + u16 -> UBig ✅
UBig + IBig -> IBig ✅
IBig + UBig -> IBig ✅

This is awesome! But how to support user defined traits is still a question

@jsha
Copy link
Contributor

jsha commented Dec 9, 2022

one concern come to mind is what about user defined "operator" traits?

The way I think of this is: rustdoc is a tool to help the doc author write docs. Sometimes it's much harder to make the tool write the docs than for the crate author to write them.

So in this case, the crate author could write a section with a summary like the one I provided, listing all the types for which Pow and DivEuclid are defined. And then to prevent impls for those traits from cluttering up the sidebar, they could use #[doc(hidden)] on the impl.

This has the downside that the doc author could make a mistake when hand-writing their summary. However, the author could improve on this by automating creation of the summary inside the macro they use to generate all the various trait implementations.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

I'm not aware of a way to automate generation of this kind of table in rust macro, do you mean using a proc-macro or something? How about using an attribute item as I proposed? By this way we can help the automate this process for everyone.

@jsha
Copy link
Contributor

jsha commented Dec 9, 2022

So for instance with Pow, the implementation is currently like:

pow_impl!(u8, u8, u32, u8::pow);
pow_impl!(u8, u16, u32, u8::pow);
pow_impl!(u8, u32, u32, u8::pow);
pow_impl!(u8, usize);
pow_impl!(i8, u8, u32, i8::pow);
pow_impl!(i8, u16, u32, i8::pow);
pow_impl!(i8, u32, u32, i8::pow);
pow_impl!(i8, usize);

You could rearrange things so these are all inside one big macro call:

pow_impl! {
  impl_one(u8, u8, u32, u8::pow);
  ...
}

That should allow the macro to have a high enough view to be able to generate a summary table. But I haven't tried it there may be obstacles I haven't foreseen.

How about using an attribute item as I proposed? By this way we can help the automate this process for everyone.

It's best to avoid adding new features until we have exhausted the possibility of a solution that uses existing features.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

With that macro, the documentation will still be generated on a specific (or each) implementation, right? I guess this kind of work still has to be done in the rustdoc side.

If a new feature is not desired, then I think the best way is to come up with a heuristic that automatically combine this kind of traits, for example here is one I quickly come up: we could combine the implementations if

  1. the trait has only one method
  2. the trait is generic
  3. the trait is implemented with multiple generic arguments

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

Furthermore, I think it's better to focus on changing the behavior of the sidebar first. For the main documentation, the ultimate solution is to generate a good summary of all the items, and the summary of trait implementations can be combined into that. I would like to implement the generating a table of contents for the docs, but I'll need some time to come up with a good style and maybe I will first post a prototype as zulip. 😄

@jsha
Copy link
Contributor

jsha commented Dec 9, 2022

the documentation will still be generated on a specific (or each) implementation, right? I guess this kind of work still has to be done in the rustdoc side.

No, I believe it should be possible to generate one set of documentation per macro invocation. Note that there is something else missing from my sketch: you might also need to encompass the type definition in your macro invocation so that you can add the appropriate #[doc(..)] attribute where you need it. Or you could call a separate macro for generating the summary, which takes the same arguments. Or you could write the summary by hand, since it's unlikely to change frequently. Or you could write a one-off program to generate the summary.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

the documentation will still be generated on a specific (or each) implementation, right? I guess this kind of work still has to be done in the rustdoc side.

No, I believe it should be possible to generate one set of documentation per macro invocation. Note that there is something else missing from my sketch: you might also need to encompass the type definition in your macro invocation so that you can add the appropriate #[doc(..)] attribute where you need it. Or you could call a separate macro for generating the summary, which takes the same arguments. Or you could write the summary by hand, since it's unlikely to change frequently. Or you could write a one-off program to generate the summary.

Well, that's true. But one point I heard a lot is that how rustdoc can help people write good documentation. If it's too bothersome, I guess most people (including me lol) will not bother to do this.

Anyway, I think it can be mitigated by a great ToC and I'm more willing to put effort into that.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

A simple solution based on your suggestion is that we just collapse all the implementations of the same trait into folded-by-default lists (in the sidebar), either always or when some criteria are met.

@willcrichton
Copy link
Contributor

Btw just to toss an idea into the pile -- I made a prototype a while ago of a pop-out table of methods:

Screen Shot 2022-12-09 at 1 00 15 PM

(I even wrote a little paper about it: https://arxiv.org/pdf/2011.05600.pdf)

Honestly I'm not a huge fan of the sidebar in the first place, since a linear column isn't a great way to organize a large amount of information. The prototype above was explicitly designed to use a large amount of screen space.

@cmpute
Copy link
Author

cmpute commented Dec 9, 2022

@willcrichton That looks really nice! Do you happen to have the implementation opened?

For the sidebar, although it's not as good as a full table in terms of information, it's still pretty useful for navigation across the page. So ideally we should have the both.

@willcrichton
Copy link
Contributor

Yep, it's sitting around in a branch on my Rust fork: willcrichton@d825c8a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants