rustdoc: use more precise relative URLs #83237

notriddle · 2021-03-17T17:28:21Z

This is a fairly large diff, and will probably conflict with #82815 since it reduces (but does not eliminate) the use of the old depth variable.

Instead of using a depth counter and adding "../" to get to the top, this commit makes rustdoc actually compare the path of what it's linking from to the path that it's linking to. This makes the resulting HTML shorter.

Here's a comparison of one of the largest (non-source) files in the Rust standard library docs (about 4% improvement before gzipping).

$ wc -c struct.Wrapping.old.html struct.Wrapping.new.html
2387389 struct.Wrapping.old.html
2298538 struct.Wrapping.new.html

Most if it can be efficiently gzipped away.

$ wc -c struct.Wrapping.old.html.gz struct.Wrapping.new.html.gz
70679 struct.Wrapping.old.html.gz
70050 struct.Wrapping.new.html.gz

But it also makes a difference in the final DOM size, reducing it from 91MiB to 82MiB.

rust-highfive · 2021-03-17T17:28:24Z

r? @jyn514

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

src/librustdoc/clean/types.rs

jyn514

But it also makes a difference in the final DOM size, reducing it from 91MiB to 82MiB.

What is the "DOM size"? Is that like the memory the browser uses the render the page?

I guess before I go much further I should ask: what happens if relative_to is empty? Will it just use links that are relative to the output root? If so I don't feel as bad about bugs, it just means the links are a little more verbose.

src/librustdoc/clean/types.rs

src/librustdoc/clean/mod.rs

src/librustdoc/clean/utils.rs

src/librustdoc/html/format.rs

notriddle · 2021-04-05T19:34:36Z

@jyn514

What is the "DOM size"? Is that like the memory the browser uses the render the page?

Yes, that's exactly what it is. In Firefox, go into the Dev Tools, then pick the Memory tab, then click the Take Snapshot button (camera icon).

what happens if relative_to is empty? Will it just use links that are relative to the output root?

Yes, that's exactly what it does. I've added a test case for this in src/librustdoc/html/tests.rs.

notriddle · 2021-04-05T20:27:13Z

@jyn514

Can you add a test case for that to make sure it doesn't regress?

Done in 53df7fc507bb6e48e1aa6588300f29c0ee4d0437

s.trim_end_matches

Done in 25bc0c0d1364a0b0cd7b8ba4742d91e3f7158b15

jyn514

Fixed in d85b42a

Could you add a test for linking to decl macros?

In some places you use cx.current; in others you choose a custom relative_to. I think the only difference is that some places are in clean, so cx.current isn't set yet. Rather than threading relative_to through everywhere, do you think it makes sense to set cx.current instead, and remember to unset it when you're done? That would make the diff a lot smaller and also reduce the number of parameters everywhere. You could add a with_current_path function or something to make sure you don't forget to reset it.

src/test/rustdoc/cross-crate-primitive-doc.rs

src/librustdoc/html/format.rs

src/librustdoc/html/tests.rs

src/librustdoc/html/render/mod.rs

src/librustdoc/html/render/context.rs

src/librustdoc/html/format.rs

src/librustdoc/html/render/write_shared.rs

jyn514 · 2021-04-16T17:27:58Z

This looks good to me with the nits addressed :) I think I want to skim through it one last time when you fix the comments and then I'm ready to approve.

It looks like the only places CURRENT_DEPTH is still used is for primitive_link? Are you interested in changing that to use shorter URLs too (in a follow-up)? I think we could get rid of CURRENT_DEPTH completely after that :)

notriddle · 2021-04-16T17:32:34Z

I'd like to finish getting rid of CURRENT_DEPTH as a follow-up. This thing has been in rebase-hell for a month now.

jyn514 · 2021-04-16T17:33:52Z

Oh definitely - this PR is fine as-is, I meant in a follow-up.

… r=jyn514 rustdoc: clean up and test macro visibility print This fixes the overly-complex invariant mentioned in <rust-lang#83237 (comment)>, where the macro source can't have any links in it only because the cache hasn't been populated yet.

jyn514 · 2021-04-16T20:17:15Z

I opened #84258 to get #84074 and #84246 merged; please wait for that to land (it should hopefully be the last rebase you have to do). I'll be offline the rest of the day and I didn't want you to hit more conflicts overnight :)

Thanks so much for working on this!

@bors delegate=notriddle

bors · 2021-04-16T20:17:17Z

✌️ @notriddle can now approve this pull request

…=jyn514 rustdoc: clean up and test macro visibility print This fixes the overly-complex invariant mentioned in <rust-lang#83237 (comment)>, where the macro source can't have any links in it only because the cache hasn't been populated yet.

Instead of using a depth counter and adding "../" to get to the top, this commit makes rustdoc actually compare the path of what it's linking from to the path that it's linking to. This makes the resulting HTML shorter. Here's a comparison of one of the largest (non-source) files in the Rust standard library docs (about 4% improvement before gzipping). $ wc -c struct.Wrapping.old.html struct.Wrapping.new.html 2387389 struct.Wrapping.old.html 2298538 struct.Wrapping.new.html Most if it can be efficiently gzipped away. $ wc -c struct.Wrapping.old.html.gz struct.Wrapping.new.html.gz 70679 struct.Wrapping.old.html.gz 70050 struct.Wrapping.new.html.gz But it also makes a difference in the final DOM size, reducing it from 91MiB to 82MiB.

The function `to_src_with_space` doesn't even accept the cache as a parameter, so it doesn't matter any more what's in it.

This should help make things consistent.

notriddle · 2021-04-17T18:39:49Z

@bors r=jyn514

bors · 2021-04-17T18:39:50Z

📌 Commit 755b4fb has been approved by jyn514

Rollup of 4 pull requests Successful merges: - rust-lang#83237 (rustdoc: use more precise relative URLs) - rust-lang#84150 (rustdoc: move some search code into search.js) - rust-lang#84203 (rustdoc: Give a more accurate span for anchor failures) - rust-lang#84257 (Add documentation to help people find `Ipv4Addr::UNSPECIFIED`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

jyn514 · 2021-04-18T03:48:23Z

🎉 🎉 🎉

…mp-to-definition, r=GuillaumeGomez rustdoc: use more precise URLs for jump-to-definition links As an example, this cuts down <https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_middle/ty/mod.rs.html> by about 11%. $ du -h new_mod.rs.html old_mod.rs.html 296K new_mod.rs.html 332K old_mod.rs.html Like rust-lang#83237, but separate code since source links have a different URL structure. Related to [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/RFC.20for.20.22jump.20to.20definition.22.20feature/near/299029786) and [the jump-to-definition pre-RFC](GuillaumeGomez/rfcs#1).

rust-highfive assigned jyn514 Mar 17, 2021

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

This comment has been minimized.

Sign in to view

jyn514 reviewed Mar 17, 2021

View reviewed changes

src/librustdoc/clean/types.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from 732830c to be6c7e5 Compare March 17, 2021 19:24

camelid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 21, 2021

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from be6c7e5 to f7f072f Compare March 22, 2021 18:55

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from f7f072f to 2b1b5d4 Compare March 22, 2021 21:51

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch 3 times, most recently from e734e40 to 93c2e3e Compare March 26, 2021 08:00

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from 93c2e3e to 4065363 Compare April 4, 2021 05:33

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from 4065363 to 21a21d4 Compare April 5, 2021 04:18

jyn514 reviewed Apr 5, 2021

View reviewed changes

jyn514 mentioned this pull request Apr 5, 2021

Remove CURRENT_DEPTH thread-local variable #82815

Closed

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch 2 times, most recently from 4b48b32 to 32d6e4c Compare April 5, 2021 23:59

This comment has been minimized.

Sign in to view

notriddle force-pushed the short-links branch from 32d6e4c to f3b76aa Compare April 6, 2021 04:45

jyn514 reviewed Apr 7, 2021

View reviewed changes

jyn514 reviewed Apr 16, 2021

View reviewed changes

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

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

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

jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 16, 2021

jyn514 marked this pull request as ready for review April 16, 2021 20:17

This comment has been minimized.

Sign in to view

notriddle added 4 commits April 17, 2021 09:21

Remove assert that no longer matters

21bf8af

The function `to_src_with_space` doesn't even accept the cache as a parameter, so it doesn't matter any more what's in it.

rustdoc: change 'cx to 'tcx, to match the struct itself

bb02048

rustdoc: move the cx argument to the end of the list

755b4fb

This should help make things consistent.

notriddle force-pushed the short-links branch from bfce2bf to 755b4fb Compare April 17, 2021 16:55

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-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Apr 17, 2021

Dylan-DPC-zz mentioned this pull request Apr 17, 2021

Rollup of 4 pull requests #84279

Merged

bors merged commit 5dae27a into rust-lang:master Apr 17, 2021

rustbot added this to the 1.53.0 milestone Apr 17, 2021

notriddle deleted the short-links branch April 17, 2021 23:32

notriddle mentioned this pull request Sep 15, 2022

rustdoc: use more precise URLs for jump-to-definition links #101868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: use more precise relative URLs #83237

rustdoc: use more precise relative URLs #83237

notriddle commented Mar 17, 2021 •

edited

Loading

rust-highfive commented Mar 17, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment

notriddle commented Apr 5, 2021

notriddle commented Apr 5, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment •

edited

Loading

jyn514 commented Apr 16, 2021 •

edited

Loading

notriddle commented Apr 16, 2021

jyn514 commented Apr 16, 2021

jyn514 commented Apr 16, 2021

bors commented Apr 16, 2021

This comment has been minimized.

notriddle commented Apr 17, 2021

bors commented Apr 17, 2021

jyn514 commented Apr 18, 2021

rustdoc: use more precise relative URLs #83237

rustdoc: use more precise relative URLs #83237

Conversation

notriddle commented Mar 17, 2021 • edited Loading

rust-highfive commented Mar 17, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment

Choose a reason for hiding this comment

notriddle commented Apr 5, 2021

notriddle commented Apr 5, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

jyn514 left a comment • edited Loading

Choose a reason for hiding this comment

jyn514 commented Apr 16, 2021 • edited Loading

notriddle commented Apr 16, 2021

jyn514 commented Apr 16, 2021

jyn514 commented Apr 16, 2021

bors commented Apr 16, 2021

This comment has been minimized.

notriddle commented Apr 17, 2021

bors commented Apr 17, 2021

jyn514 commented Apr 18, 2021

notriddle commented Mar 17, 2021 •

edited

Loading

notriddle commented Apr 5, 2021 •

edited

Loading

jyn514 left a comment •

edited

Loading

jyn514 commented Apr 16, 2021 •

edited

Loading