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

Add a chapter on reading Rustdoc output #90487

Merged
merged 1 commit into from
Nov 7, 2021

Conversation

NoraCodes
Copy link
Contributor

Includes documentation for:

  • general page structure
  • navigation
  • searching
  • themes
  • deep-linking

Doesn't include docs on the settings page.

Per #90309

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2021
The `rustdoc` output is divided into three sections.
Along the left side of each page is a quick navigation bar,
which shows contextual information about the current entry.
The rest of the is taken up by the search interface at the top
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"of the page" I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, yes, thanks!


## The Item Documentation

The majority of the screen is taken up with the main event:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the main event"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English (American?) idiom meaning "the point" - think what data is to metadata. If it confused you, though, it'll confuse others, so I'll change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds a bit funny to me too. I'd suggest something like "The majority of the screen is taken up by the main focus:" or, even simpler, "The majority of the screen is taken up by the documentation text[...]".

The main title is the type and name of the item,
such as "Struct `std::time::Duration`",
along with a button to collapse or expand the top-level documentation for that
item (`[+]` or `[-]`),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsha If we merge this PR with this doc, we'll need to not forget to update it in case we change the collapse/expand buttons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, makes sense! We would also need to update https://doc.rust-lang.org/std/#how-to-read-this-documentation.

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 1, 2021

Below this is the main documentation for the item,
including a definition or function signature if appropriate,
followed by a list of fields or variants for structs, unions, and enums.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of listing the different types, wouldn't it be better to simply say "for rust types"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable!


Typing in the search bar instantly searches the available documentation for
the string entered, with a fuzzy matching algorithm that is tolerant of omitted
letters but not extra ones.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I look for "stringa" in the std docs, it returns String. So simply saying "is tolerant to a given amount of typos" or something along the line is fine I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Reduced verbosity.

By clicking an item, you will navigate to its particular documentation.

There are two other modes, shown as tabs in the search results pane.
"In Parameters" searches for the string in the types of parameters to functions,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't search when clicking on another tab, it's simply how they're classified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, important distinction to make. Thanks!


There are two other modes, shown as tabs in the search results pane.
"In Parameters" searches for the string in the types of parameters to functions,
and "In Return Types" searches in the return types of functions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.


Pressing `S` while focused elsewhere on the page will move focus to the
search bar, and pressing `?` shows a help screen with many shortcuts.
Pressing `T` focuses the theme picker.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe talk about the help popup to see the full list (the one appearing when clicking on the ? button or pressing the ? key).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I not include the full list here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to keep them. Maybe add a note that they are listed directly in the help popup too.

@GuillaumeGomez
Copy link
Member

So overall, it's really nice. I wonder: could we add screenshots to make it easier for readers to follow? Or would it be too difficult to keep it up-to-date? (cc @rust-lang/rustdoc)

@NoraCodes
Copy link
Contributor Author

I considered that, but first of all it would make this page inaccessible, or at least it would be very verbose if we wanted to keep it accessible to people who can't see images for whatever reason - and, yeah, I suspect it would be quite annoying to keep up to date.

When typing in the search bar, you can prefix your search term with a type
followed by a colon (such as `mod:`) to restrict the results to just that
kind of item - one of `fn`, `mod`, `struct`, `enum`, `trait`, `type`, `macro`,
or `const`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are actually other things that the search input supports. I'm working on fixing the search input, so better to keep this and not go into deeper explanations and instead mention that more possibilities are listed in the help popup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotchya!

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one concern I have with this guide is that it will fall out of date as we improve the UI. Hopefully we can try to keep it up to date. Let's just make sure that the guide doesn't have scope creep, which would make it harder to keep current :)

Comment on lines 20 to 27
The main title is the type and name of the item,
such as "Struct `std::time::Duration`",
along with a button to collapse or expand the top-level documentation for that
item (`[+]` or `[-]`),
a link to the source code (`[src]`),
if [configured](the-doc-attribute.html#html_no_source),
and the version in which the item became stable,
if it's a stable item in the standard library.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is long; maybe use bullet points instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, done!

@camelid
Copy link
Member

camelid commented Nov 1, 2021

I wonder: could we add screenshots to make it easier for readers to follow? Or would it be too difficult to keep it up-to-date?

Yeah, I think it'd be too difficult to keep up-to-date. People can compare side-by-side with a docs page if they want. Also, like NoraCodes said, it'd not be as accessible.

@camelid
Copy link
Member

camelid commented Nov 1, 2021

The one concern I have with this guide is that it will fall out of date as we improve the UI.

A related thing we should keep in mind is that docs generated with older versions of rustdoc (e.g., many crates on docs.rs) won't match up to the description in the guide. (Side note: @jyn514 would it be feasible for docs.rs to regenerate docs, say once a year, for crates over a certain download rate even if they haven't been updated?)

@jyn514
Copy link
Member

jyn514 commented Nov 1, 2021

. (Side note: @jyn514 would it be feasible for docs.rs to regenerate docs, say once a year, for crates over a certain download rate even if they haven't been updated?)

rust-lang/docs.rs#464

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2021

cc @pierwill

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thank you for working on this :)

- a button to collapse or expand the top-level documentation for that item
(`[+]` or `[-]`),
- a link to the source code (`[src]`),
if [configured](the-doc-attribute.html#html_no_source),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [configured](the-doc-attribute.html#html_no_source),
if [configured](the-doc-attribute.html#html_no_source) and present. The source may not be available if the documentation was created with `cargo doc --no-deps`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @NoraCodes - did you see this comment? Was it intentional to not add this sentence?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry! I thought there was a discussion about this but now I can't find it - did I hallucinate that? lol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, yeah, still can't find it. I don't think we should mention this here because it's intended to be a guide for docs readers - we document --no-deps elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so the reason I thought to mention it was because it's pretty common for people to be confused by reading docs and having missing links. But I don't feel super strongly about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry! I thought there was a discussion about this but now I can't find it - did I hallucinate that? lol

@NoraCodes I think you're thinking of #90487 (comment), which was a discussion about another command-line flag.

src/doc/rustdoc/src/how-to-read-rustdoc.md Show resolved Hide resolved
Comment on lines +85 to +88
Both are very useful when looking for a function whose name you can't quite
bring to mind when you know the type you have or want.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also useful if you don't know whether the function exists, but you think it should (e.g. Option<T> -> T or String -> PathBuf).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that neither of those examples work as search queries currently. I assume you didn't mean using those as queries verbatim?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The results include functions that don't have String in their parameters (like Path::join). String -> PathBuf implies that it will show only functions that takes Strings and return PathBufs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ I don't think there exist any useful type-based search queries for which rustdoc doesn't have bugs. I don't think we should block documenting how to use the feature on fixing the bugs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those aren't useful searches though. We should give an example of why you would want to use the feature.

Copy link
Member

@camelid camelid Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? String -> PathBuf isn't useful either, because it returns the same results as -> PathBuf. Personally, I think we should avoid documenting type-based search here until the dust settles a bit (since Guillaume is working on an overhaul of it right now). After Guillaume finishes his overhaul, IIUC String -> PathBuf should work correctly.

We should give an example of why you would want to use the feature.

Giving a broken example of why you'd want to use the feature seems worse than giving no example at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we should wait on this until it's updated, if that's already in progress.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can take a look here if you want to see what it looks like. ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I opened the PR here).

Includes documentation for:
 - general page structure
 - navigation
 - searching
 - themes
 - deep-linking

Doesn't include docs on the settings page.
@NoraCodes
Copy link
Contributor Author

NoraCodes commented Nov 5, 2021

Okay, I believe I've addressed all PR feedback here.

@GuillaumeGomez
Copy link
Member

Thanks! I'll review it tomorrow.

Copy link
Member

@GuillaumeGomez GuillaumeGomez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me other than one nit :) thanks for working on this!

- a button to collapse or expand the top-level documentation for that item
(`[+]` or `[-]`),
- a link to the source code (`[src]`),
if [configured](the-doc-attribute.html#html_no_source),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @NoraCodes - did you see this comment? Was it intentional to not add this sentence?

@jyn514
Copy link
Member

jyn514 commented Nov 6, 2021

@bors r+ rollup

Thanks again!

@bors
Copy link
Contributor

bors commented Nov 6, 2021

📌 Commit 768554a has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 6, 2021
… r=jyn514

Add a chapter on reading Rustdoc output

Includes documentation for:
 - general page structure
 - navigation
 - searching
 - themes
 - deep-linking

Doesn't include docs on the settings page.

Per rust-lang#90309
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 6, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#90487 (Add a chapter on reading Rustdoc output)
 - rust-lang#90508 (Apply adjustments for field expression even if inaccessible)
 - rust-lang#90627 (Suggest dereference of `Box` when inner type is expected)
 - rust-lang#90642 (use matches!() macro in more places)
 - rust-lang#90646 (type error go brrrrrrrr)
 - rust-lang#90649 (Run reveal_all on MIR when inlining is activated.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit de332b5 into rust-lang:master Nov 7, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants