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.

Sign up for GitHub

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

Jump to bottom

Add version switcher #58603

Closed
wants to merge 2 commits into from
Closed

Add version switcher #58603

wants to merge 2 commits into from

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 20, 2019
@QuietMisdreavus
Copy link
Member

cc #44687 @rust-lang/docs

@GuillaumeGomez
Copy link
Member Author

It's not complete yet. I'm not really satisfied on how I handle the raw HTML. I think that I'll create "temporary" files instead and reference to them.

@GuillaumeGomez GuillaumeGomez force-pushed the version-switch branch 2 times, most recently from 8883159 to 555a07b Compare February 21, 2019 00:55
@rust-highfive

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member Author

It's now ready for review.

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@GuillaumeGomez
Copy link
Member Author

Finally fixed! \o/

@QuietMisdreavus QuietMisdreavus mentioned this pull request Feb 25, 2019
Open
2 tasks
@GuillaumeGomez
Copy link
Member Author

ping @rust-lang/docs

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2019
frewsxcv
frewsxcv approved these changes Feb 27, 2019
View reviewed changes
Copy link
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

🥇

src/bootstrap/doc.rs
let mut middle_version = i32::from_str_radix(version_parts[1], 10).expect("unknown number");

while middle_version >= 0 {
versions.push(format!("\"1.{}.0\"", middle_version));
Copy link
Member

Choose a reason for hiding this comment

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

would be cool if we could also add bugfix releases in here, but that might be hard

Copy link
Member Author

Choose a reason for hiding this comment

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

Bugfixes don't change API normally so I'm not sure it's really useful to add them in here.

@GuillaumeGomez
Copy link
Member Author

ping @frewsxcv

@frewsxcv
Copy link
Member

frewsxcv commented Mar 1, 2019

I haven't approved a rustdoc PR before and am not familiar with the source so I don't feel comfortable approving this

@QuietMisdreavus
Copy link
Member

I'll need to give this PR an actual read-through, but i wanted to make sure that this didn't derail the discussion that was happening on #44687 last year. I don't remember the specifics of that discussion (it's been a while), but this seems like a different approach than the people in that thread were considering. If it achieves the same goals, then we can fold it into that effort. But this came up independently, so i want to make sure it's not getting in the way.

@GuillaumeGomez
Copy link
Member Author

From my point of view, it doesn't prevent the #44687 to happen if they think it's not solving the issue but I'm always glad to wait for reviews coming from you. :)

QuietMisdreavus
QuietMisdreavus reviewed Mar 5, 2019
View reviewed changes
src/librustdoc/lib.rs Outdated
@@ -201,6 +201,18 @@ fn opts() -> Vec<RustcOptGroup> {
Markdown file or generated documentation",
"FILES")
}),
unstable("raw-js-in-header", |o| {
o.optopt("", "raw-js-in-header",
Copy link
Member

Choose a reason for hiding this comment

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

Bikeshed warning: Since the included file eventually gets named additional.js/additional.css, i feel like these flags would be better named --additional-js/--additional-css.

I also feel like the flag description is a little misleading. Maybe something like:

(JS|CSS) file to include as a static file to be linked from generated documentation

(I feel like this feature would be much better served if it allowed multiple files, but that can be added in a separate PR. Since the flag will be unstable, we can edit it after it lands.)

@ollie27
Copy link
Member

ollie27 commented Mar 5, 2019

I would've expected the version selector to be injected by the server running on doc.rust-lang.org because it needs to be injected into the older docs as well as all of the books. It will also need to be kept up to date with the list of released stable versions.

@GuillaumeGomez
Copy link
Member Author

The files are just generated then copied into the server. There is nothing dynamic in there. The only thing we can do is injecting the JS/CSS content required to have it.

@bors
Copy link
Contributor

bors commented Mar 27, 2019

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

@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

ping from triage @rust-lang/docs

@jonas-schievink jonas-schievink added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools I-nominated labels Jun 22, 2019
@jonas-schievink
Copy link
Contributor

Nominating to move this forward

@GuillaumeGomez
Copy link
Member Author

I don't think it'll. It needs to be added directly to the website so it has nothing to do in rustdoc directly. Closing it.

@GuillaumeGomez GuillaumeGomez deleted the version-switch branch August 19, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuietMisdreavus QuietMisdreavus QuietMisdreavus left review comments

@frewsxcv frewsxcv frewsxcv approved these changes

Assignees

@QuietMisdreavus QuietMisdreavus

Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add button to switch between std docs version on doc.rust-lang.org
9 participants
@GuillaumeGomez @QuietMisdreavus @rust-highfive @frewsxcv @ollie27 @bors @Centril @jonas-schievink @Mark-Simulacrum