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

[rustdoc] Allow to link to current crate #137935

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GuillaumeGomez
Copy link
Member

Part of #137857.

Seems surprising that we cannot link to the current crate, so this PR adds it.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Mar 3, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

This test failed because you created a breaking change.

To merge this, either make it non-breaking or prove that it's extremely rare. I'm skeptical that it's extremely rare, because thin_vec and log both have items with the same name as the crate, and that's just what I know about.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum... Making it non-breaking seems simpler. I'll add it only if there are no other item matching.

Comment on lines 459 to 465
.or_else(|| {
if self.cx.tcx.crate_name(LOCAL_CRATE).as_str() == path_str {
Some(Res::from_def_id(self.cx.tcx, CRATE_DEF_ID.to_def_id()))
} else {
None
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is the right place to put it. This runs in the context of the crate that's being documented, not in the context of the item having its names resolved, which seems like it would behave wrong when cross-crate inlining happens.

Shouldn't the doc_link_resolutions (in rustc_resolve) query be updated instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum, fair enough. Gonna move it there then.

@GuillaumeGomez
Copy link
Member Author

Moved it into rustc_resolve.


//@ has 'bar/index.html'
//@ has - '//*[@class="docblock"]//a[@href="index.html"]' 'bar'
//! [`bar`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Test case for [crate@bar]?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no crate disambiguator. Can be added in a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, an integration test for cross-crate inlining?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!

@GuillaumeGomez GuillaumeGomez force-pushed the current-crate-intra-doc branch from 5a0eb4f to ad8a77a Compare March 4, 2025 14:30
@GuillaumeGomez
Copy link
Member Author

Added the test for inlined crate link.

@ollie27
Copy link
Member

ollie27 commented Mar 10, 2025

Seems surprising that we cannot link to the current crate, so this PR adds it.

Can you not use crate to link to the current crate?

@GuillaumeGomez
Copy link
Member Author

There is no crate disambiguator. Might be worth adding but it will be in a follow-up.

@ollie27
Copy link
Member

ollie27 commented Mar 11, 2025

I meant using:

//! [`bar`](crate)

which already works.

It would be surprising if you could refer to the current crate using its name in intra-doc links when you can't in code.

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Mar 11, 2025

Oh I see. Well it does, but it's also nice to be able to link to the current crate with its name too.

@notriddle
Copy link
Contributor

notriddle commented Mar 21, 2025

I'm not convinced that it's a good idea to support linking to the current crate by its name, since Rust can't use the current crate by its name. Intra-doc links usually act like import paths (with special disambiguation powers), so this wouldn't have precedent.

#![crate_name = "tarte"]
use crate as myself; // works
use tarte as myself; //~ERROR no external crate `tarte`

@notriddle
Copy link
Contributor

@rfcbot fcp close

@rfcbot
Copy link

rfcbot commented Mar 21, 2025

Team member @notriddle has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Mar 21, 2025
@Nemo157
Copy link
Member

Nemo157 commented Mar 21, 2025

It's also possible for users to do this crate-wide if they want with extern crate self as ...;.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants