-
Notifications
You must be signed in to change notification settings - Fork 13k
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 spacing of links in inline code. #88343
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,7 +79,7 @@ use crate::vec::Vec; | |
/// | ||
/// # Examples | ||
/// | ||
/// You can create a `String` from [a literal string][`str`] with [`String::from`]: | ||
/// You can create a `String` from [a literal string][`&str`] with [`String::from`]: | ||
/// | ||
/// [`String::from`]: From::from | ||
/// | ||
|
@@ -128,7 +128,7 @@ use crate::vec::Vec; | |
/// println!("The first letter of s is {}", s[0]); // ERROR!!! | ||
/// ``` | ||
/// | ||
/// [`OsString`]: ../../std/ffi/struct.OsString.html | ||
/// [`OsString`]: ../../std/ffi/struct.OsString.html "ffi::OsString" | ||
/// | ||
/// Indexing is intended to be a constant-time operation, but UTF-8 encoding | ||
/// does not allow us to do this. Furthermore, it's not clear what sort of | ||
|
@@ -141,7 +141,7 @@ use crate::vec::Vec; | |
/// | ||
/// # Deref | ||
/// | ||
/// `String`s implement [`Deref`]`<Target=str>`, and so inherit all of [`str`]'s | ||
/// `String` implements <code>[Deref]<Target = [str]></code>, and so inherits all of [`str`]'s | ||
/// methods. In addition, this means that you can pass a `String` to a | ||
/// function which takes a [`&str`] by using an ampersand (`&`): | ||
/// | ||
|
@@ -182,7 +182,7 @@ use crate::vec::Vec; | |
/// to explicitly extract the string slice containing the string. The second | ||
/// way changes `example_func(&example_string);` to | ||
/// `example_func(&*example_string);`. In this case we are dereferencing a | ||
/// `String` to a [`str`][`&str`], then referencing the [`str`][`&str`] back to | ||
/// `String` to a [`str`], then referencing the [`str`] back to | ||
/// [`&str`]. The second way is more idiomatic, however both work to do the | ||
/// conversion explicitly rather than relying on the implicit conversion. | ||
/// | ||
|
@@ -282,9 +282,11 @@ use crate::vec::Vec; | |
/// | ||
/// Here, there's no need to allocate more memory inside the loop. | ||
/// | ||
/// [`str`]: prim@str | ||
/// [`&str`]: prim@str | ||
/// [`Deref`]: core::ops::Deref | ||
/// [str]: prim@str "str" | ||
/// [`str`]: prim@str "str" | ||
/// [`&str`]: prim@str "&str" | ||
/// [Deref]: core::ops::Deref "ops::Deref" | ||
/// [`Deref`]: core::ops::Deref "ops::Deref" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: you can avoid needing a second reference link by using |
||
/// [`as_str()`]: String::as_str | ||
#[derive(PartialOrd, Eq, Ord)] | ||
#[cfg_attr(not(test), rustc_diagnostic_item = "string_type")] | ||
|
@@ -308,10 +310,10 @@ pub struct String { | |
/// an analogue to `FromUtf8Error`, and you can get one from a `FromUtf8Error` | ||
/// through the [`utf8_error`] method. | ||
/// | ||
/// [`Utf8Error`]: core::str::Utf8Error | ||
/// [`std::str`]: core::str | ||
/// [`&str`]: prim@str | ||
/// [`utf8_error`]: Self::utf8_error | ||
/// [`Utf8Error`]: str::Utf8Error "std::str::Utf8Error" | ||
/// [`std::str`]: core::str "std::str" | ||
/// [`&str`]: prim@str "&str" | ||
/// [`utf8_error`]: FromUtf8Error::utf8_error | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -487,8 +489,8 @@ impl String { | |
/// with this error. | ||
/// | ||
/// [`from_utf8_unchecked`]: String::from_utf8_unchecked | ||
/// [`Vec<u8>`]: crate::vec::Vec | ||
/// [`&str`]: prim@str | ||
/// [`Vec<u8>`]: crate::vec::Vec "Vec" | ||
/// [`&str`]: prim@str "&str" | ||
/// [`into_bytes`]: String::into_bytes | ||
#[inline] | ||
#[stable(feature = "rust1", since = "1.0.0")] | ||
|
@@ -524,7 +526,7 @@ impl String { | |
/// it's already valid UTF-8, we don't need a new allocation. This return | ||
/// type allows us to handle both cases. | ||
/// | ||
/// [`Cow<'a, str>`]: crate::borrow::Cow | ||
/// [`Cow<'a, str>`]: crate::borrow::Cow "borrow::Cow" | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -625,7 +627,7 @@ impl String { | |
/// conversion requires a memory allocation. | ||
/// | ||
/// [`from_utf8_lossy`]: String::from_utf8_lossy | ||
/// [`Cow<'a, str>`]: crate::borrow::Cow | ||
/// [`Cow<'a, str>`]: crate::borrow::Cow "borrow::Cow" | ||
/// [U+FFFD]: core::char::REPLACEMENT_CHARACTER | ||
/// | ||
/// # Examples | ||
|
@@ -1721,11 +1723,11 @@ impl String { | |
unsafe { self.as_mut_vec() }.splice((start, end), replace_with.bytes()); | ||
} | ||
|
||
/// Converts this `String` into a [`Box`]`<`[`str`]`>`. | ||
/// Converts this `String` into a <code>[Box]<[str]></code>. | ||
/// | ||
/// This will drop any excess capacity. | ||
/// | ||
/// [`str`]: prim@str | ||
/// [str]: prim@str "str" | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -1795,8 +1797,8 @@ impl FromUtf8Error { | |
/// an analogue to `FromUtf8Error`. See its documentation for more details | ||
/// on using it. | ||
/// | ||
/// [`std::str`]: core::str | ||
/// [`&str`]: prim@str | ||
/// [`std::str`]: core::str "std::str" | ||
/// [`&str`]: prim@str "&str" | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -2319,7 +2321,7 @@ impl ops::DerefMut for String { | |
/// | ||
/// This alias exists for backwards compatibility, and may be eventually deprecated. | ||
/// | ||
/// [`Infallible`]: core::convert::Infallible | ||
/// [`Infallible`]: core::convert::Infallible "convert::Infallible" | ||
#[stable(feature = "str_parse_error", since = "1.5.0")] | ||
pub type ParseError = core::convert::Infallible; | ||
|
||
|
@@ -2606,7 +2608,7 @@ impl<'a> From<&'a str> for Cow<'a, str> { | |
/// assert_eq!(Cow::from("eggplant"), Cow::Borrowed("eggplant")); | ||
/// ``` | ||
/// | ||
/// [`Borrowed`]: crate::borrow::Cow::Borrowed | ||
/// [`Borrowed`]: crate::borrow::Cow::Borrowed "borrow::Cow::Borrowed" | ||
#[inline] | ||
fn from(s: &'a str) -> Cow<'a, str> { | ||
Cow::Borrowed(s) | ||
|
@@ -2629,7 +2631,7 @@ impl<'a> From<String> for Cow<'a, str> { | |
/// assert_eq!(Cow::from(s), Cow::<'static, str>::Owned(s2)); | ||
/// ``` | ||
/// | ||
/// [`Owned`]: crate::borrow::Cow::Owned | ||
/// [`Owned`]: crate::borrow::Cow::Owned "borrow::Cow::Owned" | ||
#[inline] | ||
fn from(s: String) -> Cow<'a, str> { | ||
Cow::Owned(s) | ||
|
@@ -2651,7 +2653,7 @@ impl<'a> From<&'a String> for Cow<'a, str> { | |
/// assert_eq!(Cow::from(&s), Cow::Borrowed("eggplant")); | ||
/// ``` | ||
/// | ||
/// [`Borrowed`]: crate::borrow::Cow::Borrowed | ||
/// [`Borrowed`]: crate::borrow::Cow::Borrowed "borrow::Cow::Borrowed" | ||
#[inline] | ||
fn from(s: &'a String) -> Cow<'a, str> { | ||
Cow::Borrowed(s.as_str()) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,8 +99,8 @@ macro_rules! acquire { | |
/// first: after all, isn't the point of `Arc<T>` thread safety? The key is | ||
/// this: `Arc<T>` makes it thread safe to have multiple ownership of the same | ||
/// data, but it doesn't add thread safety to its data. Consider | ||
/// `Arc<`[`RefCell<T>`]`>`. [`RefCell<T>`] isn't [`Sync`], and if `Arc<T>` was always | ||
/// [`Send`], `Arc<`[`RefCell<T>`]`>` would be as well. But then we'd have a problem: | ||
/// <code>Arc<[RefCell\<T>]></code>. [`RefCell<T>`] isn't [`Sync`], and if `Arc<T>` was always | ||
/// [`Send`], <code>Arc<[RefCell\<T>]></code> would be as well. But then we'd have a problem: | ||
/// [`RefCell<T>`] is not thread safe; it keeps track of the borrowing count using | ||
/// non-atomic operations. | ||
/// | ||
|
@@ -176,6 +176,7 @@ macro_rules! acquire { | |
/// [deref]: core::ops::Deref | ||
/// [downgrade]: Arc::downgrade | ||
/// [upgrade]: Weak::upgrade | ||
/// [RefCell\<T>]: core::cell::RefCell | ||
/// [`RefCell<T>`]: core::cell::RefCell | ||
/// [`std::sync`]: ../../std/sync/index.html | ||
/// [`Arc::clone(&from)`]: Arc::clone | ||
|
@@ -206,7 +207,7 @@ macro_rules! acquire { | |
/// | ||
/// Sharing a mutable [`AtomicUsize`]: | ||
/// | ||
/// [`AtomicUsize`]: core::sync::atomic::AtomicUsize | ||
/// [`AtomicUsize`]: core::sync::atomic::AtomicUsize "sync::atomic::AtomicUsize" | ||
/// | ||
/// ```no_run | ||
/// use std::sync::Arc; | ||
|
@@ -262,7 +263,7 @@ impl<T: ?Sized> Arc<T> { | |
|
||
/// `Weak` is a version of [`Arc`] that holds a non-owning reference to the | ||
/// managed allocation. The allocation is accessed by calling [`upgrade`] on the `Weak` | ||
/// pointer, which returns an [`Option`]`<`[`Arc`]`<T>>`. | ||
/// pointer, which returns an <code>[Option]<[Arc]\<T>></code>. | ||
/// | ||
/// Since a `Weak` reference does not count towards ownership, it will not | ||
/// prevent the value stored in the allocation from being dropped, and `Weak` itself makes no | ||
|
@@ -476,7 +477,7 @@ impl<T> Arc<T> { | |
/// assert_eq!(*zero, 0) | ||
/// ``` | ||
/// | ||
/// [zeroed]: ../../std/mem/union.MaybeUninit.html#method.zeroed | ||
/// [zeroed]: mem::MaybeUninit::zeroed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, did this move into core recently or something? Or was it just missed when converting links to intra-doc links? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea :-) |
||
#[cfg(not(no_global_oom_handling))] | ||
#[unstable(feature = "new_uninit", issue = "63291")] | ||
pub fn new_zeroed() -> Arc<mem::MaybeUninit<T>> { | ||
|
@@ -684,7 +685,7 @@ impl<T> Arc<[T]> { | |
/// assert_eq!(*values, [0, 0, 0]) | ||
/// ``` | ||
/// | ||
/// [zeroed]: ../../std/mem/union.MaybeUninit.html#method.zeroed | ||
/// [zeroed]: mem::MaybeUninit::zeroed | ||
#[cfg(not(no_global_oom_handling))] | ||
#[unstable(feature = "new_uninit", issue = "63291")] | ||
pub fn new_zeroed_slice(len: usize) -> Arc<[mem::MaybeUninit<T>]> { | ||
|
@@ -712,7 +713,7 @@ impl<T> Arc<mem::MaybeUninit<T>> { | |
/// Calling this when the content is not yet fully initialized | ||
/// causes immediate undefined behavior. | ||
/// | ||
/// [`MaybeUninit::assume_init`]: ../../std/mem/union.MaybeUninit.html#method.assume_init | ||
/// [`MaybeUninit::assume_init`]: mem::MaybeUninit::assume_init | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -751,7 +752,7 @@ impl<T> Arc<[mem::MaybeUninit<T>]> { | |
/// Calling this when the content is not yet fully initialized | ||
/// causes immediate undefined behavior. | ||
/// | ||
/// [`MaybeUninit::assume_init`]: ../../std/mem/union.MaybeUninit.html#method.assume_init | ||
/// [`MaybeUninit::assume_init`]: mem::MaybeUninit::assume_init | ||
/// | ||
/// # Examples | ||
/// | ||
|
@@ -1086,7 +1087,7 @@ impl<T: ?Sized> Arc<T> { | |
/// assert!(!Arc::ptr_eq(&five, &other_five)); | ||
/// ``` | ||
/// | ||
/// [`ptr::eq`]: core::ptr::eq | ||
/// [`ptr::eq`]: core::ptr::eq "ptr::eq" | ||
pub fn ptr_eq(this: &Self, other: &Self) -> bool { | ||
this.ptr.as_ptr() == other.ptr.as_ptr() | ||
} | ||
|
@@ -1714,7 +1715,7 @@ impl<T: ?Sized> Weak<T> { | |
/// // assert_eq!("hello", unsafe { &*weak.as_ptr() }); | ||
/// ``` | ||
/// | ||
/// [`null`]: core::ptr::null | ||
/// [`null`]: core::ptr::null "ptr::null" | ||
#[stable(feature = "weak_into_raw", since = "1.45.0")] | ||
pub fn as_ptr(&self) -> *const T { | ||
let ptr: *mut ArcInner<T> = NonNull::as_ptr(self.ptr); | ||
|
@@ -1806,7 +1807,6 @@ impl<T: ?Sized> Weak<T> { | |
/// [`new`]: Weak::new | ||
/// [`into_raw`]: Weak::into_raw | ||
/// [`upgrade`]: Weak::upgrade | ||
/// [`forget`]: std::mem::forget | ||
#[stable(feature = "weak_into_raw", since = "1.45.0")] | ||
pub unsafe fn from_raw(ptr: *const T) -> Self { | ||
// See Weak::as_ptr for context on how the input pointer is derived. | ||
|
@@ -1982,7 +1982,7 @@ impl<T: ?Sized> Weak<T> { | |
/// assert!(!first.ptr_eq(&third)); | ||
/// ``` | ||
/// | ||
/// [`ptr::eq`]: core::ptr::eq | ||
/// [`ptr::eq`]: core::ptr::eq "ptr::eq" | ||
#[inline] | ||
#[stable(feature = "weak_ptr_eq", since = "1.39.0")] | ||
pub fn ptr_eq(&self, other: &Self) -> bool { | ||
|
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.
By default this won't have a tooltip, right? Why did you add one?
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.
In general, links generated by rustdoc do have a tooltip.
E.g. something like
will come with a tooltip
Whereas (unfortunately) the following alternative
creates almost the same documentation, just lacking the tooltip. That’s the general reason why I’ve converted many instances of
into
and in some cases, I’ve also adjusted the tooltip slightly to make it more easy to understand. This includes the case here because
Result
is part of the prelude, a simple"Result"
tooltip seems confusing to me, thestd::
prefix is something I’ve often avoided for tooltips OTOH (not least because the paths themselves also often are relative so some importedstd::foo
module).Also, as mentioned in the description of this PR, the addition of tooltips isn’t systematic and only covers places that I happened to come across. (Furthermore, the amount of tooltips added declines from earlier to later commits.)
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.
Ok. I think this would be better to fix systematically in rustdoc rather than adding it piecemeal to individual links. Could you open an issue and revert the tooltips you've added this PR?
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.
"revert" as in “add another commit to revert them” or as a “rebase / edit / force push”. Because the former could easily happen in a later PR, too; these tooltips aren't particularly hard to find (and remove) with a regex search.
Either way, the revert would only remove those cases where the path and tooltip look identical, right? E.g. the
fmt::Result
should better stay IMO, because I don't like aResult
tooltip.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, no answer. I’m assuming that an additional commit is okay, and that only the ones where tooltip and path are identical need to be removed. This is more easily accomplished by a systematic find-and-replace that by looking through all my commits here. This could, as noted, easily be a separate PR, done after the relevant change to rustdoc was made, but I’m including a commit here now to get this PR going.
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.
Sorry for the delay, I haven't had much time for rust lately.
Well, ideally I wouldn't want any changes, because it makes them inconsistent and it will hopefully be done by rustdoc automatically eventually ... but if you feel strongly it seems fine to keep, it's just more work for me to review.I misunderstood - you're talking about showingcore::result::Result
links asstd::result::Result
and that sort of thing. I guess that's fine? I think showing the link used in the source is more clear in a way but I don't feel strongly.Yup, that's fine - I don't have strong opinions about git commits.
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.
The problem I see is that the link used in the source code depends on what the imports are, and the imports depends on what happens to be used in the source code. If e.g. I’m writing documentation mentioning, say,
mem::transmute
, then I’d use the pathmen::transmute
(resulting in amen::transmute
tooltip) if the module I’m in doesuse core::mem
(oruse crate::mem
). But ifmem
is only used in docs, AFAIK I can’t just add an import for that because (I think) it’ll trigger an unused imports lint and fail CI. But it’s IMO better to concistently have it have the tooltipmem::transmute
and not sometimescore::mem::transmute
or andcrate::mem::transmute
, etc…, depending on the implementation details (i.e. imports) of the module your documentation comes from (in particular, I don’t think thecrate::
prefix is a good thing showing up in standard library tooltips).