Skip to content

Commit

Permalink
Rollup merge of #108025 - notriddle:notriddle/intra-doc-link-tooltips…
Browse files Browse the repository at this point in the history
…, r=GuillaumeGomez

rustdoc: add more tooltips to intra-doc links

This commit makes intra-doc link tooltips consistent with generated links in function signatures and item tables, with the format `itemtype foo::bar::baz`. This way, you can tell if a link points at a trait or a type (for example) by mousing over it.

See also #39697

Partially solves https://internals.rust-lang.org/t/rustdoc-suggestion-highlight-links-fn-s-mod-s-type-s-etc-appropriately-within-and-documentation/17931 (though the Internals thread asks for color-coding, while this PR adds a tooltip instead, it's accomplishing the same thing).

Before:

<img width="950" alt="image" src="https://user-images.githubusercontent.com/1593513/218653059-911cea01-7231-438a-ad98-be98ab73783f.png">

After:

<img width="432" alt="image" src="https://user-images.githubusercontent.com/1593513/218653201-34ca3aa7-18f1-4cb1-be68-a1411bbe797e.png">
  • Loading branch information
matthiaskrgr authored Feb 14, 2023
2 parents 43b42c5 + ba4b026 commit 8804e7f
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 9 deletions.
12 changes: 8 additions & 4 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,23 +482,24 @@ impl Item {
}

pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
use crate::html::format::href;
use crate::html::format::{href, link_tooltip};

cx.cache()
.intra_doc_links
.get(&self.item_id)
.map_or(&[][..], |v| v.as_slice())
.iter()
.filter_map(|ItemLink { link: s, link_text, page_id: did, ref fragment }| {
debug!(?did);
if let Ok((mut href, ..)) = href(*did, cx) {
.filter_map(|ItemLink { link: s, link_text, page_id: id, ref fragment }| {
debug!(?id);
if let Ok((mut href, ..)) = href(*id, cx) {
debug!(?href);
if let Some(ref fragment) = *fragment {
fragment.render(&mut href, cx.tcx())
}
Some(RenderedLink {
original_text: s.clone(),
new_text: link_text.clone(),
tooltip: link_tooltip(*id, fragment, cx),
href,
})
} else {
Expand All @@ -523,6 +524,7 @@ impl Item {
original_text: s.clone(),
new_text: link_text.clone(),
href: String::new(),
tooltip: String::new(),
})
.collect()
}
Expand Down Expand Up @@ -1040,6 +1042,8 @@ pub struct RenderedLink {
pub(crate) new_text: String,
/// The URL to put in the `href`
pub(crate) href: String,
/// The tooltip.
pub(crate) tooltip: String,
}

/// The attributes on an [`Item`], including attributes like `#[derive(...)]` and `#[inline]`,
Expand Down
16 changes: 16 additions & 0 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::clean::{
use crate::formats::item_type::ItemType;
use crate::html::escape::Escape;
use crate::html::render::Context;
use crate::passes::collect_intra_doc_links::UrlFragment;

use super::url_parts_builder::estimate_item_path_byte_length;
use super::url_parts_builder::UrlPartsBuilder;
Expand Down Expand Up @@ -768,6 +769,21 @@ pub(crate) fn href_relative_parts<'fqp>(
}
}

pub(crate) fn link_tooltip(did: DefId, fragment: &Option<UrlFragment>, cx: &Context<'_>) -> String {
let cache = cx.cache();
let Some((fqp, shortty)) = cache.paths.get(&did)
.or_else(|| cache.external_paths.get(&did))
else { return String::new() };
let fqp = fqp.iter().map(|sym| sym.as_str()).join("::");
if let &Some(UrlFragment::Item(id)) = fragment {
let name = cx.tcx().item_name(id);
let descr = cx.tcx().def_kind(id).descr(id);
format!("{descr} {fqp}::{name}")
} else {
format!("{shortty} {fqp}")
}
}

/// Used to render a [`clean::Path`].
fn resolved_path<'cx>(
w: &mut fmt::Formatter<'_>,
Expand Down
16 changes: 11 additions & 5 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
trace!("it matched");
assert!(self.shortcut_link.is_none(), "shortcut links cannot be nested");
self.shortcut_link = Some(link);
if title.is_empty() && !link.tooltip.is_empty() {
*title = CowStr::Borrowed(link.tooltip.as_ref());
}
}
}
// Now that we're done with the shortcut link, don't replace any more text.
Expand Down Expand Up @@ -410,9 +413,12 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
}
// If this is a link, but not a shortcut link,
// replace the URL, since the broken_link_callback was not called.
Some(Event::Start(Tag::Link(_, dest, _))) => {
Some(Event::Start(Tag::Link(_, dest, title))) => {
if let Some(link) = self.links.iter().find(|&link| *link.original_text == **dest) {
*dest = CowStr::Borrowed(link.href.as_ref());
if title.is_empty() && !link.tooltip.is_empty() {
*title = CowStr::Borrowed(link.tooltip.as_ref());
}
}
}
// Anything else couldn't have been a valid Rust path, so no need to replace the text.
Expand Down Expand Up @@ -976,7 +982,7 @@ impl Markdown<'_> {
links
.iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.new_text.as_str().into()))
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
};

let p = Parser::new_with_broken_link_callback(md, main_body_opts(), Some(&mut replacer));
Expand Down Expand Up @@ -1059,7 +1065,7 @@ impl MarkdownSummaryLine<'_> {
links
.iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.new_text.as_str().into()))
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
};

let p = Parser::new_with_broken_link_callback(md, summary_opts(), Some(&mut replacer))
Expand Down Expand Up @@ -1106,7 +1112,7 @@ fn markdown_summary_with_limit(
link_names
.iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.new_text.as_str().into()))
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
};

let p = Parser::new_with_broken_link_callback(md, summary_opts(), Some(&mut replacer));
Expand Down Expand Up @@ -1187,7 +1193,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin
link_names
.iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.new_text.as_str().into()))
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
};

let p = Parser::new_with_broken_link_callback(md, summary_opts(), Some(&mut replacer));
Expand Down
17 changes: 17 additions & 0 deletions tests/rustdoc/intra-doc/basic.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,38 @@
// @has basic/index.html
// @has - '//a/@href' 'struct.ThisType.html'
// @has - '//a/@title' 'struct basic::ThisType'
// @has - '//a/@href' 'struct.ThisType.html#method.this_method'
// @has - '//a/@title' 'associated function basic::ThisType::this_method'
// @has - '//a/@href' 'enum.ThisEnum.html'
// @has - '//a/@title' 'enum basic::ThisEnum'
// @has - '//a/@href' 'enum.ThisEnum.html#variant.ThisVariant'
// @has - '//a/@title' 'variant basic::ThisEnum::ThisVariant'
// @has - '//a/@href' 'trait.ThisTrait.html'
// @has - '//a/@title' 'trait basic::ThisTrait'
// @has - '//a/@href' 'trait.ThisTrait.html#tymethod.this_associated_method'
// @has - '//a/@title' 'associated function basic::ThisTrait::this_associated_method'
// @has - '//a/@href' 'trait.ThisTrait.html#associatedtype.ThisAssociatedType'
// @has - '//a/@title' 'associated type basic::ThisTrait::ThisAssociatedType'
// @has - '//a/@href' 'trait.ThisTrait.html#associatedconstant.THIS_ASSOCIATED_CONST'
// @has - '//a/@title' 'associated constant basic::ThisTrait::THIS_ASSOCIATED_CONST'
// @has - '//a/@href' 'trait.ThisTrait.html'
// @has - '//a/@title' 'trait basic::ThisTrait'
// @has - '//a/@href' 'type.ThisAlias.html'
// @has - '//a/@title' 'type basic::ThisAlias'
// @has - '//a/@href' 'union.ThisUnion.html'
// @has - '//a/@title' 'union basic::ThisUnion'
// @has - '//a/@href' 'fn.this_function.html'
// @has - '//a/@title' 'fn basic::this_function'
// @has - '//a/@href' 'constant.THIS_CONST.html'
// @has - '//a/@title' 'constant basic::THIS_CONST'
// @has - '//a/@href' 'static.THIS_STATIC.html'
// @has - '//a/@title' 'static basic::THIS_STATIC'
// @has - '//a/@href' 'macro.this_macro.html'
// @has - '//a/@title' 'macro basic::this_macro'
// @has - '//a/@href' 'trait.SoAmbiguous.html'
// @has - '//a/@title' 'trait basic::SoAmbiguous'
// @has - '//a/@href' 'fn.SoAmbiguous.html'
// @has - '//a/@title' 'fn basic::SoAmbiguous'
//! In this crate we would like to link to:
//!
//! * [`ThisType`](ThisType)
Expand Down

0 comments on commit 8804e7f

Please sign in to comment.