[rustdoc] Generate redirect map file #81223

GuillaumeGomez · 2021-01-20T20:08:28Z

So with this code:

#![crate_name = "foo"]

pub use private::Quz;
pub use hidden::Bar;

mod private {
    pub struct Quz;
}

#[doc(hidden)]
pub mod hidden {
    pub struct Bar;
}

#[macro_export]
macro_rules! foo {
() => {}
}

It generates:

{
  "foo/macro.foo!.html": "foo/macro.foo.html",
  "foo/private/struct.Quz.html": "foo/struct.Quz.html",
  "foo/hidden/struct.Bar.html": "foo/struct.Bar.html"
}

Do the pathes look as you expected @pietroalbini?

r? @jyn514

src/librustdoc/html/render/mod.rs

src/test/rustdoc/redirect-map.rs

jyn514 · 2021-01-21T00:31:51Z

The implementation looks mostly fine to me, but I'm not 100% convinced this is necessary - @pietroalbini the main issue in #81134 was that rustdoc is generating inline <script>s, right? I think we could just remove that and be fine, since there's also a <meta http-equiv="refresh">.

pietroalbini · 2021-01-21T12:31:18Z

It generates:

{
    "macro.foo!.html":"macro.foo.html",
    "doc/foo/private/struct.Quz.html":"../../foo/struct.Quz.html",
    "doc/foo/hidden/struct.Bar.html":"../../foo/struct.Bar.html"
}

Do the pathes look as you expected @pietroalbini?

@GuillaumeGomez I have some bits of feedback:

macro.foo!.html should be foo/macro.foo!.html.
The doc/ prefix shouldn't be there: if you call rustdoc -o otheroutputdir ... I guess the code would include otheroutputdir in the redirect map instead of doc, which I'm not sure is right.
I guess relative paths as the destination are ok, but I think it would make more sense to avoid ../s and make all the redirects relative from the documentation root.

the main issue in #81134 was that rustdoc is generating inline <script>s, right? I think we could just remove that and be fine, since there's also a <meta http-equiv="refresh">.

@jyn514 technically yes, but even the <meta http-equiv="refresh"> provides a worse UX than server-side redirects. It's inevitable when using a simple static server or loading docs from the filesystem, but an application like docs.rs can use a feature like this to greatly improve the redirect UX.

bors · 2021-01-21T16:32:30Z

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

pietroalbini · 2021-01-25T10:58:32Z

The output you posted on the PR body looks good @GuillaumeGomez, but there should be a test for it.

GuillaumeGomez · 2021-01-25T11:47:10Z

I'm waiting for #81197 to add a run-make test.

bors · 2021-01-27T13:47:39Z

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

jyn514

I'm waiting for #81197 to add a run-make test.

@Mark-Simulacrum asked that I drop that change from the PR, so I would just make it run-make-fulldeps for now.

src/librustdoc/html/render/mod.rs

jyn514 · 2021-02-15T02:12:58Z

Also, please remember to update the labels so it's easy to tell the state of the PR. I didn't realize you thought this was blocked.

GuillaumeGomez · 2021-02-15T20:55:18Z

I added the run-make-fulldeps test. I had to add a lot of print in there to know what I did wrong and I decided to keep them in case they're needed in the future. That was a fun ride!

src/test/run-make-fulldeps/rustdoc-map-file/expected.json

src/test/run-make-fulldeps/rustdoc-map-file/validate_json.py

GuillaumeGomez · 2021-02-16T11:00:40Z

@jyn514 Thanks a lot for your great catch! I fixed it.

jyn514

This looks good to me (with nits fixed), at least good enough to be on nightly. @pietroalbini may want to take another look before this is merged though.

src/librustdoc/html/render/mod.rs

…ith a unique JSON map

GuillaumeGomez · 2021-02-23T10:27:55Z

Updated! Waiting for @pietroalbini now. :)

pietroalbini · 2021-03-02T15:06:16Z

The format looks good on my end!

jyn514 · 2021-03-02T15:08:59Z

@bors r+

bors · 2021-03-02T15:09:00Z

📌 Commit 41fc58b has been approved by jyn514

…p, r=jyn514 [rustdoc] Generate redirect map file Fixes rust-lang#81134. So with this code: ```rust #![crate_name = "foo"] pub use private::Quz; pub use hidden::Bar; mod private { pub struct Quz; } #[doc(hidden)] pub mod hidden { pub struct Bar; } #[macro_export] macro_rules! foo { () => {} } ``` It generates: ```json { "foo/macro.foo!.html": "foo/macro.foo.html", "foo/private/struct.Quz.html": "foo/struct.Quz.html", "foo/hidden/struct.Bar.html": "foo/struct.Bar.html" } ``` Do the pathes look as you expected `@pietroalbini?` r? `@jyn514`

…p, r=jyn514 [rustdoc] Generate redirect map file Fixes rust-lang#81134. So with this code: ```rust #![crate_name = "foo"] pub use private::Quz; pub use hidden::Bar; mod private { pub struct Quz; } #[doc(hidden)] pub mod hidden { pub struct Bar; } #[macro_export] macro_rules! foo { () => {} } ``` It generates: ```json { "foo/macro.foo!.html": "foo/macro.foo.html", "foo/private/struct.Quz.html": "foo/struct.Quz.html", "foo/hidden/struct.Bar.html": "foo/struct.Bar.html" } ``` Do the pathes look as you expected ``@pietroalbini?`` r? ``@jyn514``

…p, r=jyn514 [rustdoc] Generate redirect map file Fixes rust-lang#81134. So with this code: ```rust #![crate_name = "foo"] pub use private::Quz; pub use hidden::Bar; mod private { pub struct Quz; } #[doc(hidden)] pub mod hidden { pub struct Bar; } #[macro_export] macro_rules! foo { () => {} } ``` It generates: ```json { "foo/macro.foo!.html": "foo/macro.foo.html", "foo/private/struct.Quz.html": "foo/struct.Quz.html", "foo/hidden/struct.Bar.html": "foo/struct.Bar.html" } ``` Do the pathes look as you expected ```@pietroalbini?``` r? ```@jyn514```

Rollup of 10 pull requests Successful merges: - rust-lang#81223 ([rustdoc] Generate redirect map file) - rust-lang#82439 (BTree: fix untrue safety) - rust-lang#82469 (Use a crate to produce rustdoc tree comparisons instead of the `diff` command) - rust-lang#82589 (unix: Non-mutable bufs in send_vectored_with_ancillary_to) - rust-lang#82689 (meta: Notify Zulip for rustdoc nominated issues) - rust-lang#82695 (Revert non-power-of-two vector restriction) - rust-lang#82706 (use outer_expn_data() instead of outer_expn().expn_data()) - rust-lang#82710 (FloatToInit: Replacing round_unchecked_to --> to_int_unchecked) - rust-lang#82712 (Remove unnecessary conditional `cfg(target_os)` for `redox` and `vxworks`) - rust-lang#82713 (Update cargo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 20, 2021

rust-highfive assigned jyn514 Jan 20, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2021

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the generate-redirect-map branch from 0784868 to d69e1f1 Compare January 20, 2021 20:23

jyn514 reviewed Jan 21, 2021

View reviewed changes

src/librustdoc/html/render/mod.rs Show resolved Hide resolved

src/test/rustdoc/redirect-map.rs Show resolved Hide resolved

jyn514 assigned pietroalbini Jan 21, 2021

jyn514 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 Jan 21, 2021

GuillaumeGomez force-pushed the generate-redirect-map branch from d69e1f1 to d9868a0 Compare January 21, 2021 20:18

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the generate-redirect-map branch from d9868a0 to 78f50d2 Compare January 21, 2021 20:59

jyn514 mentioned this pull request Jan 23, 2021

Fix flaky test #81197

Closed

JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 14, 2021

jyn514 reviewed Feb 15, 2021

View reviewed changes

src/librustdoc/html/render/mod.rs Show resolved Hide resolved

GuillaumeGomez force-pushed the generate-redirect-map branch from 78f50d2 to 0a01acf Compare February 15, 2021 20:53

GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 15, 2021

jyn514 reviewed Feb 15, 2021

View reviewed changes

src/test/run-make-fulldeps/rustdoc-map-file/expected.json Outdated Show resolved Hide resolved

jyn514 reviewed Feb 15, 2021

View reviewed changes

src/test/run-make-fulldeps/rustdoc-map-file/validate_json.py Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the generate-redirect-map branch from 0a01acf to 735320e Compare February 16, 2021 09:59

GuillaumeGomez force-pushed the generate-redirect-map branch from 735320e to c73b1a1 Compare February 16, 2021 10:59

jyn514 approved these changes Feb 23, 2021

View reviewed changes

src/librustdoc/html/render/mod.rs Outdated Show resolved Hide resolved

src/librustdoc/html/render/mod.rs Outdated Show resolved Hide resolved

jyn514 removed their assignment Feb 23, 2021

jyn514 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 23, 2021

GuillaumeGomez added 3 commits February 23, 2021 11:13

Add --generate-redirect-map option to replace HTML redirection file w…

3a92f77

…ith a unique JSON map

Add tests for --generate-redirect-map option

2b59e76

Add full test for rustdoc map file

1a7126b

GuillaumeGomez force-pushed the generate-redirect-map branch from c73b1a1 to d1c223d Compare February 23, 2021 10:26

Fix nits

41fc58b

GuillaumeGomez force-pushed the generate-redirect-map branch from d1c223d to 41fc58b Compare February 23, 2021 10:26

GuillaumeGomez mentioned this pull request Mar 2, 2021

Rollup of 5 pull requests #82705

Closed

JohnTitor mentioned this pull request Mar 3, 2021

Rollup of 10 pull requests #82718

Merged

bors merged commit 9a827d9 into rust-lang:master Mar 3, 2021

rustbot added this to the 1.52.0 milestone Mar 3, 2021

GuillaumeGomez deleted the generate-redirect-map branch March 3, 2021 11:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rustdoc] Generate redirect map file #81223

[rustdoc] Generate redirect map file #81223

GuillaumeGomez commented Jan 20, 2021 •

edited

Loading

This comment has been minimized.

jyn514 commented Jan 21, 2021

pietroalbini commented Jan 21, 2021 •

edited by jyn514

Loading

bors commented Jan 21, 2021

This comment has been minimized.

pietroalbini commented Jan 25, 2021

GuillaumeGomez commented Jan 25, 2021

bors commented Jan 27, 2021

jyn514 left a comment

jyn514 commented Feb 15, 2021 •

edited

Loading

GuillaumeGomez commented Feb 15, 2021

GuillaumeGomez commented Feb 16, 2021

jyn514 left a comment

GuillaumeGomez commented Feb 23, 2021

pietroalbini commented Mar 2, 2021

jyn514 commented Mar 2, 2021

bors commented Mar 2, 2021

[rustdoc] Generate redirect map file #81223

[rustdoc] Generate redirect map file #81223

Conversation

GuillaumeGomez commented Jan 20, 2021 • edited Loading

This comment has been minimized.

jyn514 commented Jan 21, 2021

pietroalbini commented Jan 21, 2021 • edited by jyn514 Loading

bors commented Jan 21, 2021

This comment has been minimized.

pietroalbini commented Jan 25, 2021

GuillaumeGomez commented Jan 25, 2021

bors commented Jan 27, 2021

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 commented Feb 15, 2021 • edited Loading

GuillaumeGomez commented Feb 15, 2021

GuillaumeGomez commented Feb 16, 2021

jyn514 left a comment

Choose a reason for hiding this comment

GuillaumeGomez commented Feb 23, 2021

pietroalbini commented Mar 2, 2021

jyn514 commented Mar 2, 2021

bors commented Mar 2, 2021

GuillaumeGomez commented Jan 20, 2021 •

edited

Loading

pietroalbini commented Jan 21, 2021 •

edited by jyn514

Loading

jyn514 commented Feb 15, 2021 •

edited

Loading