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

Broken parameter links in some of the stdlib documentation #121106

Closed
RossSmyth opened this issue Feb 14, 2024 · 6 comments · Fixed by #121490
Closed

Broken parameter links in some of the stdlib documentation #121106

RossSmyth opened this issue Feb 14, 2024 · 6 comments · Fixed by #121490
Assignees
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@RossSmyth
Copy link
Contributor

RossSmyth commented Feb 14, 2024

  1. Go to https://doc.rust-lang.org/std/simd/type.u8x1.html#method.len
  2. Click on the usize return parameter
  3. The link goes here: https://doc.rust-lang.org/primitive.usize.html

This happens with other parameters. But if you go to the SIMD page through the prelude the link is not broken. Nor is it broken on the portable simd docs. There are other things broken, such as slices on the SIMD page as well. It is also on other SIMD types' documentation pages. I don't know the exact pattern though.

I expected to see this happen:

The link would be to this page: https://doc.rust-lang.org/std/primitive.usize.html

Instead, this happened:

The link is to this page: https://doc.rust-lang.org/primitive.usize.html

Basically it seems to not be inserting the "std" directory in the link.

@RossSmyth RossSmyth added the C-bug Category: This is a bug. label Feb 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 14, 2024
@jieyouxu
Copy link
Member

@rustbot label +A-docs +A-rustdoc-ui +T-rustdoc -needs-triage

@rustbot rustbot added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-rustdoc-ui Area: rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 14, 2024
@fmease fmease removed the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Feb 15, 2024
@gurry
Copy link
Contributor

gurry commented Feb 15, 2024

@rustbot claim

@gurry
Copy link
Contributor

gurry commented Feb 16, 2024

Regressed in one of these two commits: db3e2ba 74cf505. Not sure which because the second one, which is the predecessor of the first one, doesn't build.

@gurry
Copy link
Contributor

gurry commented Feb 21, 2024

Ignore the above comment. It was a wrong bisection.

The regression most likely occurred in PR #116471. I say "most likely" because it was impossible to bisect around this part of the git log because builds kept failing with ICE's.

Anyway, the cause of the bug is that in type alias docs links to primitives are being rendered relative to the target type not the alias. For example for u8x1 the link is rendered as ../../primitive.<prim>.html instead of ../primitive.<prim>.html because the target type Simd is two levels deep from the root even though u8x1 is just one level deep.

This problem seems limited only type aliases.

Internally this is how the rendering of the link works:

  • write_shared() generates the primitive path as just primitive.<prim>.html in the HTML fragment held in the JSONP format.
  • window.register_type_impls in main.js then prepends this string with the value of window.rootPath which seems to be relative to the target type (i.e. ../../ for our above example of u8x1)

The end result is that we get a link relative to the type not the alias.

@gurry
Copy link
Contributor

gurry commented Feb 21, 2024

The fix could either be in main.js wherein we could compute the correct path to prepend depending on the path of the alias instead of just using window.rootPath. Or it could be in write_shared() /primitive_link_fragment() methods whereby we could generate the path std/primitive.<prim>.html instead of primitive.<prim>.html for the primitive which will ensure that just prepending window.rootPath in front of it will work.

What are your thoughts @notriddle since you where the original author? This is the first time I've ventured into rustdoc so any pointers will be helpful.

@notriddle
Copy link
Contributor

I think the second option, generating the link as std/primitive…, is the better choice.

@bors bors closed this as completed in dbe3398 Feb 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 24, 2024
Rollup merge of rust-lang#121490 - gurry:121106-broken-usize-link-in-docs, r=notriddle

Rustdoc: include crate name in links for local primitives

Fixes rust-lang#121106.

This change makes links to primitives easier to use when the path of the page where they will be embedded is not known beforehand such as when we generate impls dynamically from the `register_type_impls` method in `main.js`, which is exactly what is happening in rust-lang#121106.

An example to show the effect of this change: earlier, if the current page in `cx.current` inside `primitive_link_fragment()` was `std::simd::prelude::Simd` the generated path would be `../../primitive.<prim>.html`. Now it would be `../../../std/primitive.<prim>.html` instead.

A side effect of the change is that local primitive links _everywhere_ will now contain the crate name, even outside of the dynamic situation mentioned above. I'm not sure if there are any major downsides of that other than making the links a bit longer. Ideally I wanted to restrict this behaviour change to only the dynamic cases. We could have achieved that by passing an additional bool arg to `primitive_link_fragment()`, but it felt awkward to do so. Any alternative suggestions are welcome.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 27, 2024
…06, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 27, 2024
…06, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 27, 2024
…06, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 28, 2024
…06, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 28, 2024
…06, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 28, 2024
Rollup merge of rust-lang#121572 - notriddle:notriddle/test-case-121106, r=GuillaumeGomez

Add test case for primitive links in alias js

Follow up rust-lang#121490

CC rust-lang#121106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants