-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustdoc: Add item types to all page titles #34345
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Do you have an online doc with this change please? |
Sure, I've uploaded this change to https://ollie27.github.io/rust_doc_test/std/. See before, after for example. |
Like I thought, I don't like this change. :-/ cc @steveklabnik |
The item type is already in the title for most items: crates, modules, structs, traits, primitives etc... with inconsistencies like functions but not foreign functions. What is it about this change you don't like? Is it the the titles in general or just the types I've adding them to? |
I don't like "Type definition". I don't find this information interesting. For a struct or a function, it is "easy" to read. But in this case, not really. We're a bit lost in the title trying to get the useful information of what this page is about. However, this isn't a big "anti" opinion. |
I am split myself. It is in some places already... @rust-lang/tools what do you think? |
I think the inconsistency should be fixed, but I too think the extra verbosity of the "Type Definition" example doesn't add much. Immediately below the title is the declaration, so it's obvious it's a typedef. On the other hand there are pages where the extra title does contribute: modules, because the page doesn't show the module definition, crates for the same reason and to distinguish from modules, primitives because they are special pages. One problem with the example is just that "Type Definition" is a mouthful - two whole words, whereas the existing examples are mostly short single words. That specific case might look more visually appealing (though less understandable) abbreviated to "Typedef". Alternative suggestions:
|
clean::StaticItem(..) | clean::ForeignStaticItem(..) => | ||
write!(fmt, "Static ")?, | ||
clean::ConstantItem(..) => write!(fmt, "Constant ")?, | ||
_ => unreachable!(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it may be a bit of a refactoring hazard, could this explicitly mention the remaining cases and redirect them to unreachable!()
? Perhaps with a comment as well indicating why they are indeed unreachable.
I kinda like the idea of just removing them from everything that isn't clearly disambiguated by looking at the definition inlined right afterwards. For example structs and functions don't really need the extra word because it's clear from reading the definition. Along those lines, I'd be fine leaving just modules/crates with these words and eliding all the others. |
Closing due to inactivity, but feel free to resubmit with comments addressed! |
…bnik rustdoc: Add missing item types to page titles Most pages include the item type in the title such as "Struct std::vec::Vec". However it is missing from the pages for foreign functions, type definitions, macros, statics and constants. This adds them so for example, instead of a title of "std::u32::MAX" it is "Constant std::u32::MAX" to match the others. [before](https://doc.rust-lang.org/nightly/std/u32/constant.MAX.html) [after](https://ollie27.github.io/rust_doc_test/std/u32/constant.MAX.html) [before](https://doc.rust-lang.org/nightly/std/io/type.Result.html) [after](https://ollie27.github.io/rust_doc_test/std/io/type.Result.html) Previous discussions: rust-lang#34345, rust-lang#35003
…bnik rustdoc: Add missing item types to page titles Most pages include the item type in the title such as "Struct std::vec::Vec". However it is missing from the pages for foreign functions, type definitions, macros, statics and constants. This adds them so for example, instead of a title of "std::u32::MAX" it is "Constant std::u32::MAX" to match the others. [before](https://doc.rust-lang.org/nightly/std/u32/constant.MAX.html) [after](https://ollie27.github.io/rust_doc_test/std/u32/constant.MAX.html) [before](https://doc.rust-lang.org/nightly/std/io/type.Result.html) [after](https://ollie27.github.io/rust_doc_test/std/io/type.Result.html) Previous discussions: rust-lang#34345, rust-lang#35003
No description provided.