-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
More intra doc links #77875
More intra doc links #77875
Conversation
library/core/src/char/methods.rs
Outdated
/// # Safety | ||
/// | ||
/// This function is unsafe, as it may construct invalid `char` values. | ||
/// | ||
/// For a safe version of this function, see the [`from_u32`] function. | ||
/// | ||
/// [`from_u32`]: #method.from_u32 | ||
/// [`from_u32`]: prim@char::from_u32() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, this is a bug in intra-doc links. prim
should apply to the whole item, not to the type.
warning: incompatible link kind for `char::from_u32`
--> tmp.rs:1:6
|
1 | /// [char::from_u32()]
| ^^^^^^^^^^^^^^^^ help: to link to the builtin type, prefix with `prim@`: `prim@char::from_u32`
= note: this link resolved to a builtin type, which is not a function
That said, I guess it works as-is, it's just inconsistent with the rest of intra-doc links ... @Manishearth what do you think? I think if we made it apply to the whole item there'd be no way to disambiguate between primitives and modules for associated items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think it could still use core::primitive::char::from_u32
actually. That seems better anyway to remove the ambiguity.
I didn't know core::primitive
existed when I added prim@
... maybe we should just get rid of that disambiguator altogether to be consistent with rustc_resolve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you end up using core::primitive
please ping me so I can update intraconv
:)
Oh sorry - the current failures are another bug that was recently fixed: #77267. I guess this has to wait until the beta bump then :/ |
4609ddf
to
d7fc2f8
Compare
Now that #77267 has been closed, I rebased on |
This fails on the
|
☔ The latest upstream changes (presumably #78874) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
d7fc2f8
to
4f50d0a
Compare
I couldn't transform links like those so I simply let them as they are for now so that this PR can be merged - /// [`len_utf8()`]: #method.len_utf8
+ /// [`len_utf8()`]: crate::primitive::char::len_utf8()
OR
+ /// [`len_utf8()`]: prim@char::len_utf8()
OR
+ /// [`len_utf8()`]: char::len_utf8() |
It looks like this is still broken unfortunately.
|
This comment has been minimized.
This comment has been minimized.
d558e60
to
1735a42
Compare
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me with Cargo.lock updated
I think this might be fixed by #76467. I'll add a test case. |
1735a42
to
5bdd640
Compare
@bors r+ Thanks for sticking with this! |
📌 Commit 5bdd640 has been approved by |
⌛ Testing commit 5bdd640 with merge 796a2a9bbe7614610bd67d4cd0cf0dfff0468778... |
💥 Test timed out |
@bors retry |
☀️ Test successful - checks-actions |
Helps with #75080.
I did a commit by group of file, I can squash if wanted.
@rustbot modify labels: T-doc, A-intra-doc-links
r? @jyn514