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

Fix links pointing to std_detect #96519

Closed

Conversation

GuillaumeGomez
Copy link
Member

This PR is one way to fix #96506. Another one would be to extend html_root_url attribute to allow it to work like the unstable --html-root-url option.

Any opinion maybe?

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2022
@GuillaumeGomez
Copy link
Member Author

cc @rust-lang/rustdoc

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 28, 2022
@rust-log-analyzer

This comment has been minimized.

.arg(&builder.src.join("src/doc/index.md"));
.arg(&builder.src.join("src/doc/index.md"))
.arg("--extern-html-root-url")
.arg("std_detect=https://docs.rs/std_detect/0.1.5/")
Copy link
Contributor

Choose a reason for hiding this comment

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

Version will eventually be changed, and this can lead to some hard to detect errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is exactly why I specify a version: otherwise older std docs version will always point to the latest std_detect version and that will very likely be completely wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or we can check version of crate via metadata and fail if it not equal hardcoded one.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can extract the version from the libstd Cargo.toml file I guess...

Copy link
Contributor

Choose a reason for hiding this comment

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

cargo_metadata crate should work, i guess.

Copy link
Member

Choose a reason for hiding this comment

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

Why is std_detect not added to the list of crates we document?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea. Should it be part of the crates documented?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe - it seems like a likely easy and correct way to expose it's documentation/source. There's a policy question of whether we should do that - it would probably be a reasonable stance that it's not part of the stable surface area so showing search results (for example) from it isn't appropriate. Perhaps rustdoc shouldn't render the src link in this case.

But I think this PR is almost certainly the wrong approach - if we want to not include it in search results but include links to it, there's probably a way to order the builds and later combine them or something like that to accomplish that.

@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 May 1, 2022
@Mark-Simulacrum
Copy link
Member

I'm going to close this PR as I don't think it's the right direction (as noted in my last comment), and if we want to pursue this further my recommendation is to first define what behavior we want, answering these questions:

  • Should std_detect results be available in rustdoc search on doc.rust-lang.org? (I'd probably say no, since it's not intentionally available for end-user consumption)
  • How should rustdoc be informed to generate (or not generate) source links for re-exported macros?

One alternative to answering these questions might be to import the std_detect crate into libstd proper, which doesn't seem like it should be that hard to me (it's already part of the dependency closure and doesn't receive that much traffic in terms of contributions). I'm not sure why it lives separately right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link for source of is_x86_feature_detected is broken
5 participants