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 rustdoc version into the help popup #88964

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Sep 15, 2021

After a discussion with a rustdoc user about a specific behaviour, we realized we were not talking about the same version. To add on top of it, it was actually not that simple to find out the version since it was hosted documentation.

So to simplify things, I added the version into the help popup:

Screenshot from 2021-09-16 10-45-52

Does the version format looks or would you prefer that I add more information? We can also add the commit hash, commit date, host and release.

cc @rust-lang/rustdoc
r? @jyn514

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) A-rustdoc-js Area: Rustdoc's JS front-end labels Sep 15, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2021
@Nemo157
Copy link
Member

Nemo157 commented Sep 15, 2021

I feel like the full short-form like the CLI would be enough (rustdoc 1.53.0 (53cb7b09b 2021-06-17)), host shouldn't affect the output.

@GuillaumeGomez
Copy link
Member Author

So commit hash and commit date as well, sounds good to me!

@GuillaumeGomez
Copy link
Member Author

Actually, they're already supposed to be in. Funny:

$ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustdoc -V
rustdoc 1.57.0-dev
$ rustdoc -V
rustdoc 1.56.0-nightly (af140757b 2021-08-22)

@jyn514
Copy link
Member

jyn514 commented Sep 15, 2021

r? @Nemo157

@rust-highfive rust-highfive assigned Nemo157 and unassigned jyn514 Sep 15, 2021
@camelid
Copy link
Member

camelid commented Sep 15, 2021

(Weird, I didn't see the @rust-lang/rustdoc ping... GitHub notifications have been acting up for me recently.)

I like this change a lot; there have been many times in the past that I've wanted to know what rustdoc version some docs were generated with. However, I'm wondering whether the help popup is the best place to put it. It seems fine to put it there for now, but I wanted to bring up the topic.

Does the version format looks or would you prefer that I add more information? We can also add the commit hash, commit date, host and release.

I agree with Nemo that having the regular rustdoc --version output would be good. I also have a styling comment: I feel like it would be better to have it with inline code style.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Sep 15, 2021

I agree with Nemo that having the regular rustdoc --version output would be good. I also have a styling comment: I feel like it would be better to have it with inline code style.

I use exactly the same function used for the -V option. It's that apparently, the two missing parameters (commit hash and date) are not passed for the dev, only for a "release" (including all nightly versions). Take a look at my explanations just above.

I like this change a lot; there have been many times in the past that I've wanted to know what rustdoc version some docs were generated with. However, I'm wondering whether the help popup is the best place to put it. It seems fine to put it there for now, but I wanted to bring up the topic.

I actually see a few advantages to this:

  • It doesn't take more space that needed (the JS file is generated once, allowing to have this information to not be duplicated into few places).
  • It's easy to tell to people where to look in case we need this information.

But if you have suggestion for another place, I'm all for it. My second choice was on the crate page (but not sure where to put it though), my third one was in the setting page (but don't like this one much...).

@camelid
Copy link
Member

camelid commented Sep 15, 2021

  • the JS file is generated once, allowing to have this information duplicated into few places

Does that mean that if different crates in the docs were built with different rustdoc versions, it would only show one of the versions? It seems like we should make sure it shows the version used to build the crate the user is looking it, to avoid any possibility for confusion.

@jyn514
Copy link
Member

jyn514 commented Sep 15, 2021

FWIW, the version thing shouldn't affect docs.rs, since it uses a --resource-suffix that includes the version. But I suppose it could make a difference when documenting locally.

@jyn514
Copy link
Member

jyn514 commented Sep 15, 2021

Actually, cargo since recently deletes files in doc/ when rerunning rustdoc, so it shouldn't make a difference even locally.

@GuillaumeGomez
Copy link
Member Author

Does that mean that if different crates in the docs were built with different rustdoc versions, it would only show one of the versions? It seems like we should make sure it shows the version used to build the crate the user is looking it, to avoid any possibility for confusion.

It'll have the last version yes (which is why I thought about putting it into the crate doc page). But like @jyn514 said, unless you are doing something strange, it shouldn't be a concern.

@camelid
Copy link
Member

camelid commented Sep 16, 2021

Actually, cargo since recently deletes files in doc/ when rerunning rustdoc, so it shouldn't make a difference even locally.

Although that doesn't help for using rustdoc ... manually. But I guess it'd be pretty rare to have putting it in the help popup cause an issue so it seems fine.

@Nemo157
Copy link
Member

Nemo157 commented Sep 16, 2021

I feel like it would be better to have it with inline code style.

I also agree with this.

@GuillaumeGomez
Copy link
Member Author

Done! (updated the screenshot as well)

@Nemo157
Copy link
Member

Nemo157 commented Sep 16, 2021

Sounds like we're happy with the slight chance for inconsistency on manual cross-version rustdoc usage.

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2021

📌 Commit 26eb5bb has been approved by Nemo157

@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 Sep 16, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 16, 2021
Add rustdoc version into the help popup

After a discussion with a rustdoc user about a specific behaviour, we realized we were not talking about the same version. To add on top of it, it was actually not that simple to find out the version since it was hosted documentation.

So to simplify things, I added the version into the help popup:

![Screenshot from 2021-09-16 10-45-52](https://user-images.githubusercontent.com/3050060/133581128-b93b460a-e1cb-4a31-9f2f-97c7a916cfcc.png)

Does the version format looks or would you prefer that I add more information? We can also add the commit hash, commit date, host and release.

cc `@rust-lang/rustdoc`
r? `@jyn514`
@bors
Copy link
Contributor

bors commented Sep 16, 2021

☔ The latest upstream changes (presumably #88219) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 16, 2021
@GuillaumeGomez
Copy link
Member Author

@bors: r=Nemo157

@bors
Copy link
Contributor

bors commented Sep 17, 2021

📌 Commit cd3f4da has been approved by Nemo157

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 17, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 17, 2021
…laumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#86422 (Emit clearer diagnostics for parens around `for` loop heads)
 - rust-lang#87460 (Point to closure when emitting 'cannot move out' for captured variable)
 - rust-lang#87566 (Recover invalid assoc type bounds using `==`)
 - rust-lang#88666 (Improve build command for compiler docs)
 - rust-lang#88899 (Do not issue E0071 if a type error has already been reported)
 - rust-lang#88949 (Fix handling of `hir::GenericArg::Infer` in `wrong_number_of_generic_args.rs`)
 - rust-lang#88953 (Add chown functions to std::os::unix::fs to change the owner and group of files)
 - rust-lang#88954 (Allow `panic!("{}", computed_str)` in const fn.)
 - rust-lang#88964 (Add rustdoc version into the help popup)
 - rust-lang#89012 (Suggest removing `#![feature]` for library features that have been stabilized)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 833358b into rust-lang:master Sep 17, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 17, 2021
@GuillaumeGomez GuillaumeGomez deleted the version-help branch September 17, 2021 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-js Area: Rustdoc's JS front-end A-rustdoc-ui Area: Rustdoc UI (generated HTML) 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.

7 participants