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 version switcher script for std docs #101855

Closed

Conversation

GuillaumeGomez
Copy link
Member

The goal of this PR is to add a UI which allows to switch between the different versions of the hosted libraries docs on doc.rust-lang.org.

It comes from this discussion.

r? @Mark-Simulacrum

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 15, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2022
let content = fs::read_to_string(builder.src.join("src/doc/version-switcher.js")).unwrap();
fs::write(
out.join("version-switcher.js"),
content.replace("/* VERSION TO BE REPLACED */", &builder.version),
Copy link
Member Author

Choose a reason for hiding this comment

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

To be noted that this builder.version is "broken" because it returns 1.65.0 whereas the current version hosted is 1.63.0. Not sure how to get around that either...

I'm thinking about doing it like this: if it's nightly, I reduce the number of version by 2, if it's beta by 1 and it's stable, I don't change it. Does it seem like the right way to do it?

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused -- when we're shipping nightly, the current version is 1.65.0? Why would this need to get offset?

My expectation is that new versions of the script are/will be picked up in doc.rust-lang.org/nightly/ after each nightly release, in doc.rust-lang.org/beta/ after each beta release, and both (doc.rust-lang.org/stable/ and doc.rust-lang.org/1.x.y/) after each stable release.

We can always have a toggle for switching to beta + nightly and have a separate replace here which puts the current channel (so that the script knows that the current version and that channel are the same).

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that there is no 1.65.0/std link currently. So the version number, even if accurate, is wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Right, we would need to use the channel and the version number to compute the right link. I think we can assume that if the channel is nightly the current and current-1 versions should link to /nightly/ and /beta/ respectively, and prior versions can use the 'named/versioned' (1.x.0) URLs.

@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

@bors try so we can produce a sample set of documentation on dev-doc.

@bors
Copy link
Contributor

bors commented Sep 17, 2022

⌛ Trying commit dab8165d6a1a8935ad102abccdf9cd2199d34a74 with merge f129e33cb1c13b17c19290f4903b011c07edb0ce...

@bors
Copy link
Contributor

bors commented Sep 17, 2022

☀️ Try build successful - checks-actions
Build commit: f129e33cb1c13b17c19290f4903b011c07edb0ce (f129e33cb1c13b17c19290f4903b011c07edb0ce)

@Mark-Simulacrum
Copy link
Member

@bors try -- looks like we need a slight adjustment to be able to build a release, cherry picked the commit from #101952 here.

@bors
Copy link
Contributor

bors commented Sep 17, 2022

⌛ Trying commit f1140894115b8e306e5ed8e84df8a4bbabbf6f1b with merge 83e89a338d2e9e4adf115a978fed92380bc53424...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 17, 2022

☀️ Try build successful - checks-actions
Build commit: 83e89a338d2e9e4adf115a978fed92380bc53424 (83e89a338d2e9e4adf115a978fed92380bc53424)

@@ -0,0 +1 @@
<script defer src='https://doc.rust-lang.org/version-switcher.js'></script>
Copy link
Member

Choose a reason for hiding this comment

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

Per https://github.com/rust-lang/rust/pull/101855/files#r973608362, I think this file needs to be slightly templated as wel, ideally a relative path so we support dev-doc well -- basically, I'd like it if we just land this PR we get the right behavior on nightly immediately, and in all future Rust releases. We still want something like a trivial include mechanism so that it's easy to insert it into past HTML, but this PR isn't there yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

The include on rustdoc side is unfortunately limited. For example, we can't use a relative path for this JS file as it wouldn't work on file:///. To work for all previous versions, as we discussed, we either need to regenerate the documentation or to make an insertion when serving the files. We could also add this JS directly into the main.js script in all versions and that would work too. Might be ever simpler.

Copy link
Member

Choose a reason for hiding this comment

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

If appending to main.js works consistently (and identifying that file is easy, which is probably true), then I think that seems like a good option to me, since it side-steps some of the difficulty here.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is one down-side putting this script into main.js though: starting 1.63 (with this commit), the main.js script import is defer, meaning it doesn't block loading the page, meaning the top navbar will appear after the page is loaded, making a weird update. Not a blocker but worth mentioning I think.

So do you want me to simply insert it at the start of the main.js file or do you want to keep it into another file?

@Mark-Simulacrum
Copy link
Member

@bors try

Adjusted to use dev-doc + nightly for the source of the version switcher file.

@bors
Copy link
Contributor

bors commented Sep 18, 2022

⌛ Trying commit 970af13a317a9be8c4556b8da9b89bc8b8be8a58 with merge 8aa7abbffd996766c55d3aa9861074906ba7ddcd...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 18, 2022

☀️ Try build successful - checks-actions
Build commit: 8aa7abbffd996766c55d3aa9861074906ba7ddcd (8aa7abbffd996766c55d3aa9861074906ba7ddcd)

@Mark-Simulacrum
Copy link
Member

OK, this is now live in its current form at https://dev-doc.rust-lang.org/nightly/std/.

Some initial thoughts:

  • We should have the current version pre-selected
  • The numbered version and the channel should probably share the 'slot' (i.e., nightly docs built for 1.66 should default to showing something like '1.66.0 - nightly').

@Mark-Simulacrum Mark-Simulacrum 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2022
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Sep 18, 2022

We should have the current version pre-selected

Good idea!

The numbered version and the channel should probably share the 'slot' (i.e., nightly docs built for 1.66 should default to showing something like '1.66.0 - nightly').

How would it work when we generate docs for stable and beta? This is really where my big confusion is coming from at the moment.

@GuillaumeGomez
Copy link
Member Author

I fixed formatting, applied your suggestions. Now remains to clarify about the version.

@Mark-Simulacrum
Copy link
Member

@bors try

@Mark-Simulacrum
Copy link
Member

@bors try

@Mark-Simulacrum
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Sep 27, 2022

⌛ Trying commit 415fa7e57e0bbe18b95cd40aea37cd02f56540fd with merge 47571822b519c0511afd228bac8d84c62ad200dd...

@bors
Copy link
Contributor

bors commented Sep 27, 2022

☀️ Try build successful - checks-actions
Build commit: 47571822b519c0511afd228bac8d84c62ad200dd (47571822b519c0511afd228bac8d84c62ad200dd)

@Mark-Simulacrum
Copy link
Member

Published to dev-doc the latest contents.

image

I think we should flip nightly/beta/stable so that we have them in version order and include the version number by each name (e.g., 1.66.0 - nightly).

I will try and look at the injection of this into past releases but IMO it would be good to get some style/design eyes on the general concept of this bar from @rust-lang/rustdoc, which can be done in parallel with that. Once T-rustdoc is happy with this I would suggest landing this PR so we're gaining experience and testing on nightly, and then we (I) can in parallel help with rolling out to the bar to older releases.

@GuillaumeGomez
Copy link
Member Author

I switched the order as suggested. For older releases, I think the simplest way to do it is to insert this JS directly at the beginning of main.js.

@notriddle
Copy link
Contributor

notriddle commented Sep 27, 2022

Layout breaks when the text wraps.

Screenshot 2022-09-27 at 2 04 57 PM

@GuillaumeGomez
Copy link
Member Author

Fixed it. We also had a weird case with the 1.59 where I think the sidebar was in a "middle state" between updates.

@camelid
Copy link
Member

camelid commented Sep 28, 2022

I really like the idea! My only concern is that the visual design doesn't feel right. The banner's a little intrusive, and the text preceding feels too long. What about something like this instead?

image

@@ -0,0 +1,269 @@
(function() {

let CURRENT_VERSION = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Feels like this should be null to be a better indication that it's a sentinel value.

Comment on lines +6 to +17
if (CURRENT_VERSION !== -1) {
return CURRENT_VERSION;
}
const now = Date.now();
// Month is 0-indexed.
// First release of Rust, 15 may 2015.
const first_release = new Date(2015, 4, 15);
const diff_time = Math.abs(now - first_release);
const nb_days = Math.ceil(diff_time / (1000 * 60 * 60 * 24));
const nb_weeks = nb_days / 7;
CURRENT_VERSION = Math.floor(nb_weeks / 6);
return CURRENT_VERSION;
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't account for patch versions. Also, it seems fragile to compute the version from the date. E.g., what if for some reason Rust had to skip or delay a release?

I think a decent solution would be to use https://blog.rust-lang.org/releases.json and extract the number from the title or URL.

11.346h48.546c6.373 0 11.635-4.982 11.982-11.346l7.418-136c.375-6.874-5.098-12.654-\
11.982-12.654h-63.383c-6.884 0-12.356 5.78-11.981 12.654z"></path></svg>&nbsp;`;
return warning_img + "You are seeing an outdated version of this documentation. " +
"Click on the dropdown to go to the latest stable version:&nbsp;";
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a screenshot of what this warning looks like? I can't get it to show with the dev-doc since it's only for old versions.

@jsha
Copy link
Contributor

jsha commented Sep 28, 2022

FYI I started a Zulip thread to talk about the UI, and more importantly the expected use cases and tasks: https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Version.20switching.20for.20std.20docs

@jsha
Copy link
Contributor

jsha commented Sep 28, 2022

After some discussion in the chat, I propose to close this PR:

I would like to have a better path to learning about the different documentation channels, but I don't think adding UI like this is the right path.

In theory I like Pietro's idea [in chat] about turning the version number into a dropdown. In practice, we would be adding significant complexity and potential for breakage, because of the problem of rewriting rustdoc output for old versions, and in general having UI that is the same across versions. We already spend a lot of maintenance effort making this work for docs.rs; doing something similar on doc.rust-lang.org would add more maintenance burden.

If the need was strong, it would be worth taking that maintenance burden, but since the need is weak let's skip it. Maybe we can solve the problem another way, like documenting the URL structure. For instance, https://doc.rust-lang.org/std/ doesn't mention the beta and nightly docs at all.

@GuillaumeGomez
Copy link
Member Author

Since there is no clear path for this, we will close it.

@GuillaumeGomez GuillaumeGomez deleted the doc-version-switcher branch September 28, 2022 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants