Add an item-info for non-public visibility, and remove redundant pub
#90242
Conversation
Visibility::print_with_space and Visibility::to_src_with_space had some duplicated code. This extracts the code into a shared enum, with a Display impl that will make reuse elsewhere easier.
For most documentation, the `pub` in front of each method is redundant: you know the method is public because you are reading the documentation. However, when running with `--document-private`, it's important to see which items are public and which are private. This adds an item-info tag for private items showing their visibility, and removes the `pub` from methods and fields. This doesn't currently touch the summary code block at the top of the page, which will still have `pub` or `pub(crate)` or whatever is appropriate.
jsha
added
T-rustdoc
|
Some changes occurred in HTML/CSS/JS. |
rust-highfive
added
the
S-waiting-on-review
|
FWIW, I'd personally rather avoid having rustdoc invent a new way of describing privacy -- and the lack of a visibility label does not imply public in most Rust code, so it seems surprising to take that approach here. Perhaps we can instead only emit visibility annotations if --document-private-items is passed, but not modify their style? These feel like orthogonal changes to me. I'd definitely be in favor of removing pub in places where it can't be written in normal Rust code, like the blanket impls section for example, and it seems OK to remove it from non-
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Using item-infos to display visibility feels like a much worse way to communicate the information. It's distracting and takes up significant visual space on the page. When documenting a crate with private items, tons of items are going to have these markers, which would clutter the UI even further. Making the visibility display dependent on whether As discussed in the PR from a few months ago (#86283), I think it's good for rustdoc to be as close as possible to Rust source code. Perhaps what Mark said above in #90242 (comment) about hiding IMHO, having an extra |
This doesn't make the visibility display dependent on the --document-private-items flag. It removes The core problem I'm trying to solve is this: Multiple users have complained about the "scannability" of Rust docs; that's one of the core accessibility complaints in #59829. Users tend to scan web pages in an "F-Pattern", that is, after reading a summary at the top, they scan down the left side of the page. That means we should place the information they are looking for - the method name, for most structs and traits - as close to the left side of the page as possible, in the path of their scan. There are several things that prevent the method name from being leftmost - One way to look at this is in terms of tasks: Task 1: As a user reading public documentation, I need to know which methods and types are public. This is trivially achieved: everything in the documentation is public. Task 2: As a developer working on a crate, I need to read documentation generated with Are we in agreement that Task 1 is as easily accomplished with this PR? My argument is that Task 2 is actually better accomplished by marking the "private" case than the non-private case:
@Mark-Simulacrum said:
This is not really a new way to describe privacy: It's how privacy is described at https://doc.rust-lang.org/reference/visibility-and-privacy.html:
The difference is that privacy is expressed one way in the code, and one way in the reference documentation. I think it makes sense to align rustdoc with the reference documentation rather than the code. The code expression is geared towards one use case: privacy should be the default. The documentation expression should be geared towards another use case: public visibility is the default. @camelid said:
Personally, I think the scannability of method names is the most significant clutter problem in the UI right now, and I think removing
Agreed! Would this idea improve things? If a struct is private, we would mark that struct private at the top of its page, but we would not separately annotate its fields and methods as private, since that is implied by the struct being private. Also, I'd be really interested to hear more details about your workflow - what crates do you use |
Sorry, I wasn't clear. I was referring to Mark's suggestion from above:
What do you mean by the way privacy is expressed in the Reference documentation? The Rust Reference should be describing how Rust source code works. Where does the Reference say that public visibility is the default? I want to make sure we're not miscommunicating :)
It's hard for me to know without experiencing it first-hand. I wonder if we could consider changing the UI behind a feature flag so we could get practical experience with it before landing it for all users?
Mostly, I'm referring to the rustc and rustdoc API docs (/nightly/nightly-rustc), which I use frequently while working on Rust. I also sometimes use For the most part, I use Overall, part of the issue is that I and others, like Mark, are used to the docs the way they are. Thus, it's hard for me to tell whether changing the format would be an improvement or regression, and either way, I'd have to get used to the new UI. Like I mentioned above, maybe we could figure out a way to land this behind a feature flag (or, rather, an unstable CLI flag)? Otherwise, it feels like we have to commit to the new UI before even trying it out "in the wild". |
I agree that it's valuable to frame the situation as two distinct ways to use rustdoc (what you call tasks): the "user" and the "developer" use case. Rustdoc is by default meant to be used in "user mode", and passing cc @rust-lang/rustdoc to see what other people think |
To clarify, what I meant by quoting this is that putting "Visibility: private" in Rustdoc output is not a new way to describe privacy, but that those words ("visibility" and "private") are already used to describe the semantics expressed in code by
Ah, yes, I'm not saying that public visibility is the default when you write code. I'm saying: As a user, when I read Rust docs, 99.99% of the items I read are public. I therefore assume that most things I read are public - that's what I mean by default. And that's a property one would like from one's public documentation. I think what I'm gathering from your use cases is: when you use |
|
Maybe this is an unusual mindset, but I usually don't read docs - regardless of how they were generated - with a fixation on privacy. Most or all of the code I deal with typically can, if necessary, make things public or expose a new public wrapper if necessary, so it's not a particularly meaningful attribute. In that context, I'd prefer that the way rustdoc exposes privacy matches how I expect to see the function in source code (sort of a form of inline docs): that is, pub is rendered as pub, lack thereof is similarly rendered as nothing - just like in the code I write or source code I read. In practice, I think that my lack of interest drives a desire to make the information (which is useful when I actually decide to use an API) as streamlined as possible. Since I'm already used to scanning Rust code in it's .rs form, pub/pub(crate) or lack thereof don't jump out at me, my eyes skim them easily, but any other indicator is "unusual" and so a distraction. I think my "goal" for the function signatures in rustdoc is that they're as close as possible to matching what I'd see in the source, and so having different rendering then the source does not seem like a good idea. I'd rather see "pub" a bunch rather than not and need to think "oh this is not a .rs signature, so rustdoc is removing visibility markers, so pub is not necessary". I think the signatures having the same syntax also helps as a teaching tool, since it gives a very clear mapping. But I have no actual proof for this. |
|
Just to voice my opinion here: the "scannability" issue of the rust docs came up quite a lot of time. I had quite a lot discussions about it but could never find out a way to improve it without making the output "heavier". Removing the However, I'm not completely convinced by the current approach. It's fine, but heavier. Which seems to be the opposite of what we're trying to achieve here. |
I don't think this is true, I've been against deviating too much from how it looks in the source so that people don't have to learn a new set of conventions. If the stated goal is scannability it might be worth having the alignment axis be the method name and have the modifiers appear before it (probably with a smaller font) I tend to agree with Mark on my goals of using rust documentation |
My situation is essentially what Mark described:
|
JohnCSimon
added
S-waiting-on-review
|
☔ The latest upstream changes (presumably #91203) made this pull request unmergeable. Please resolve the merge conflicts. |
rustbot
added
S-waiting-on-author
JohnCSimon
added
S-waiting-on-author
|
@jsha any updates on this? |
|
I still prefer the current approach of trying to match source code syntax. And, if we were to switch to a less-close-to-syntax approach, I quite dislike using "item-infos" since they're very visually noisy. |
|
Yeah, I agree item-info is not the way to go here. I hadn't really grokked how much people use rustdoc to navigate private aspects of a crate. Closing for now; I'll try again with a different approach later. |
For most documentation, the
pubin front of each method is redundant: you know the method is public because you are reading the documentation.However, when running with
--document-private, it's important to see which items are public and which are private. This adds an item-info tag for private items showing their visibility, and removes thepubfrom methods and fields.This doesn't currently touch the summary code block at the top of the page, which will still have
puborpub(crate)or whatever is appropriate.This refactors some code in Visibility to allow better reuse.
Part of #59829. Note: one of the pieces of feedback there was that the item-infos (experimental, deprecated, platform-specific, etc) are too prominent. That may be the case, but for this change, I think the important thing is matching the other item-infos, and we can change the style of item-infos later if we want to.
r? @GuillaumeGomez
/cc @camelid
Demo:
https://jacob.hoffman-andrews.com/rust/document-private/foo/outer_mod/inner_mod/inmost_mod/struct.Boo.html
https://jacob.hoffman-andrews.com/rust/document-private/foo/outmost_mod/outer_mod/inner_mod/inmost_mod/struct.Foo.html
https://jacob.hoffman-andrews.com/rust/document-private/foo/outmost_mod/outer_mod/inner_mod/inmost_mod/enum.Bar.html