Skip to content

Commit

Permalink
Rollup merge of rust-lang#126247 - notriddle:notriddle/word-wrap-item…
Browse files Browse the repository at this point in the history
…-table, r=GuillaumeGomez

rustdoc: word wrap CamelCase in the item list table and sidebar

This is an alternative to rust-lang#126209. That is, it fixes the issue that affects the very long type names in https://docs.rs/async-stripe/0.31.0/stripe/index.html#structs.

This is, necessarily, a pile of nasty heuristics. We need to balance a few issues:

- Sometimes, there's no real word break. For example, `BTreeMap` should be `BTree<wbr>Map`, not `B<wbr>Tree<wbr>Map`.

- Sometimes, there's a legit word break, but the name is tiny and the HTML overhead isn't worth it. For example, if we're typesetting `TyCtx`, writing `Ty<wbr>Ctx` would have an HTML overhead of 50%. Line breaking inside it makes no sense.

# Screenshots

| Before | After |
| ------ | ----- |
| ![image](https://github.com/rust-lang/rust/assets/1593513/d51201fd-46c0-4f48-aee6-a477eadba288) | ![image](https://github.com/rust-lang/rust/assets/1593513/d8e77582-adcf-4966-bbfd-19dfdad7336a)
  • Loading branch information
matthiaskrgr authored Jul 29, 2024
2 parents 66e5852 + ac303df commit 9aedec9
Show file tree
Hide file tree
Showing 26 changed files with 208 additions and 46 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4826,6 +4826,7 @@ dependencies = [
"tracing",
"tracing-subscriber",
"tracing-tree",
"unicode-segmentation",
]

[[package]]
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ tempfile = "3"
tracing = "0.1"
tracing-tree = "0.3.0"
threadpool = "1.8.1"
unicode-segmentation = "1.9"

[dependencies.tracing-subscriber]
version = "0.3.3"
Expand Down
55 changes: 55 additions & 0 deletions src/librustdoc/html/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use std::fmt;

use unicode_segmentation::UnicodeSegmentation;

/// Wrapper struct which will emit the HTML-escaped version of the contained
/// string when passed to a format string.
pub(crate) struct Escape<'a>(pub &'a str);
Expand Down Expand Up @@ -74,3 +76,56 @@ impl<'a> fmt::Display for EscapeBodyText<'a> {
Ok(())
}
}

/// Wrapper struct which will emit the HTML-escaped version of the contained
/// string when passed to a format string. This function also word-breaks
/// CamelCase and snake_case word names.
///
/// This is only safe to use for text nodes. If you need your output to be
/// safely contained in an attribute, use [`Escape`]. If you don't know the
/// difference, use [`Escape`].
pub(crate) struct EscapeBodyTextWithWbr<'a>(pub &'a str);

impl<'a> fmt::Display for EscapeBodyTextWithWbr<'a> {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
let EscapeBodyTextWithWbr(text) = *self;
if text.len() < 8 {
return EscapeBodyText(text).fmt(fmt);
}
let mut last = 0;
let mut it = text.grapheme_indices(true).peekable();
let _ = it.next(); // don't insert wbr before first char
while let Some((i, s)) = it.next() {
let pk = it.peek();
if s.chars().all(|c| c.is_whitespace()) {
// don't need "First <wbr>Second"; the space is enough
EscapeBodyText(&text[last..i]).fmt(fmt)?;
last = i;
continue;
}
let is_uppercase = || s.chars().any(|c| c.is_uppercase());
let next_is_uppercase =
|| pk.map_or(true, |(_, t)| t.chars().any(|c| c.is_uppercase()));
let next_is_underscore = || pk.map_or(true, |(_, t)| t.contains('_'));
let next_is_colon = || pk.map_or(true, |(_, t)| t.contains(':'));
if i - last > 3 && is_uppercase() && !next_is_uppercase() {
EscapeBodyText(&text[last..i]).fmt(fmt)?;
fmt.write_str("<wbr>")?;
last = i;
} else if (s.contains(':') && !next_is_colon())
|| (s.contains('_') && !next_is_underscore())
{
EscapeBodyText(&text[last..i + 1]).fmt(fmt)?;
fmt.write_str("<wbr>")?;
last = i + 1;
}
}
if last < text.len() {
EscapeBodyText(&text[last..]).fmt(fmt)?;
}
Ok(())
}
}

#[cfg(test)]
mod tests;
68 changes: 68 additions & 0 deletions src/librustdoc/html/escape/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// basic examples
#[test]
fn escape_body_text_with_wbr() {
use super::EscapeBodyTextWithWbr as E;
// extreme corner cases
assert_eq!(&E("").to_string(), "");
assert_eq!(&E("a").to_string(), "a");
assert_eq!(&E("A").to_string(), "A");
assert_eq!(&E("_").to_string(), "_");
assert_eq!(&E(":").to_string(), ":");
assert_eq!(&E(" ").to_string(), " ");
assert_eq!(&E("___________").to_string(), "___________");
assert_eq!(&E(":::::::::::").to_string(), ":::::::::::");
assert_eq!(&E(" ").to_string(), " ");
// real(istic) examples
assert_eq!(&E("FirstSecond").to_string(), "First<wbr>Second");
assert_eq!(&E("First_Second").to_string(), "First_<wbr>Second");
assert_eq!(&E("First Second").to_string(), "First Second");
assert_eq!(&E("First HSecond").to_string(), "First HSecond");
assert_eq!(&E("First HTTPSecond").to_string(), "First HTTP<wbr>Second");
assert_eq!(&E("First SecondThird").to_string(), "First Second<wbr>Third");
assert_eq!(&E("First<T>_Second").to_string(), "First&lt;<wbr>T&gt;_<wbr>Second");
assert_eq!(&E("first_second").to_string(), "first_<wbr>second");
assert_eq!(&E("first:second").to_string(), "first:<wbr>second");
assert_eq!(&E("first::second").to_string(), "first::<wbr>second");
assert_eq!(&E("MY_CONSTANT").to_string(), "MY_<wbr>CONSTANT");
// a string won't get wrapped if it's less than 8 bytes
assert_eq!(&E("HashSet").to_string(), "HashSet");
// an individual word won't get wrapped if it's less than 4 bytes
assert_eq!(&E("VecDequeue").to_string(), "VecDequeue");
assert_eq!(&E("VecDequeueSet").to_string(), "VecDequeue<wbr>Set");
// how to handle acronyms
assert_eq!(&E("BTreeMap").to_string(), "BTree<wbr>Map");
assert_eq!(&E("HTTPSProxy").to_string(), "HTTPS<wbr>Proxy");
// more corners
assert_eq!(&E("ṼẽçÑñéå").to_string(), "Ṽẽç<wbr>Ññéå");
assert_eq!(&E("V\u{0300}e\u{0300}c\u{0300}D\u{0300}e\u{0300}q\u{0300}u\u{0300}e\u{0300}u\u{0300}e\u{0300}").to_string(), "V\u{0300}e\u{0300}c\u{0300}<wbr>D\u{0300}e\u{0300}q\u{0300}u\u{0300}e\u{0300}u\u{0300}e\u{0300}");
assert_eq!(&E("LPFNACCESSIBLEOBJECTFROMWINDOW").to_string(), "LPFNACCESSIBLEOBJECTFROMWINDOW");
}
// property test
#[test]
fn escape_body_text_with_wbr_makes_sense() {
use itertools::Itertools as _;

use super::EscapeBodyTextWithWbr as E;
const C: [u8; 3] = [b'a', b'A', b'_'];
for chars in [
C.into_iter(),
C.into_iter(),
C.into_iter(),
C.into_iter(),
C.into_iter(),
C.into_iter(),
C.into_iter(),
C.into_iter(),
]
.into_iter()
.multi_cartesian_product()
{
let s = String::from_utf8(chars).unwrap();
assert_eq!(s.len(), 8);
let esc = E(&s).to_string();
assert!(!esc.contains("<wbr><wbr>"));
assert!(!esc.ends_with("<wbr>"));
assert!(!esc.starts_with("<wbr>"));
assert_eq!(&esc.replace("<wbr>", ""), &s);
}
}
3 changes: 2 additions & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::clean::utils::find_nearest_parent_module;
use crate::clean::{self, ExternalCrate, PrimitiveType};
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::escape::Escape;
use crate::html::escape::{Escape, EscapeBodyText};
use crate::html::render::Context;
use crate::passes::collect_intra_doc_links::UrlFragment;

Expand Down Expand Up @@ -988,6 +988,7 @@ pub(crate) fn anchor<'a, 'cx: 'a>(
f,
r#"<a class="{short_ty}" href="{url}" title="{short_ty} {path}">{text}</a>"#,
path = join_with_double_colon(&fqp),
text = EscapeBodyText(text.as_str()),
)
} else {
f.write_str(text.as_str())
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/html/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ struct PageLayout<'a> {
display_krate_version_extra: &'a str,
}

pub(crate) use crate::html::render::sidebar::filters;

pub(crate) fn render<T: Print, S: Print>(
layout: &Layout,
page: &Page<'_>,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod tests;

mod context;
mod print_item;
mod sidebar;
pub(crate) mod sidebar;
mod span_map;
mod type_layout;
mod write_shared;
Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::clean;
use crate::config::ModuleSorting;
use crate::formats::item_type::ItemType;
use crate::formats::Impl;
use crate::html::escape::Escape;
use crate::html::escape::{Escape, EscapeBodyTextWithWbr};
use crate::html::format::{
display_fn, join_with_double_colon, print_abi_with_space, print_constness_with_space,
print_where_clause, visibility_print_with_space, Buffer, Ending, PrintWithSpace,
Expand Down Expand Up @@ -423,7 +423,7 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
"<div class=\"item-name\"><code>{}extern crate {} as {};",
visibility_print_with_space(myitem, cx),
anchor(myitem.item_id.expect_def_id(), src, cx),
myitem.name.unwrap(),
EscapeBodyTextWithWbr(myitem.name.unwrap().as_str()),
),
None => write!(
w,
Expand Down Expand Up @@ -520,7 +520,7 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
{stab_tags}\
</div>\
{docs_before}{docs}{docs_after}",
name = myitem.name.unwrap(),
name = EscapeBodyTextWithWbr(myitem.name.unwrap().as_str()),
visibility_and_hidden = visibility_and_hidden,
stab_tags = extra_info_tags(myitem, item, tcx),
class = myitem.type_(),
Expand Down Expand Up @@ -558,7 +558,7 @@ fn extra_info_tags<'a, 'tcx: 'a>(
display_fn(move |f| {
write!(
f,
r#"<span class="stab {class}" title="{title}">{contents}</span>"#,
r#"<wbr><span class="stab {class}" title="{title}">{contents}</span>"#,
title = Escape(title),
)
})
Expand Down
16 changes: 16 additions & 0 deletions src/librustdoc/html/render/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,22 @@ impl<'a> Link<'a> {
}
}

pub(crate) mod filters {
use std::fmt::Display;

use rinja::filters::Safe;

use crate::html::escape::EscapeBodyTextWithWbr;
use crate::html::render::display_fn;
pub(crate) fn wrapped<T>(v: T) -> rinja::Result<Safe<impl Display>>
where
T: Display,
{
let string = v.to_string();
Ok(Safe(display_fn(move |f| EscapeBodyTextWithWbr(&string).fmt(f))))
}
}

pub(super) fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buffer) {
let blocks: Vec<LinkBlock<'_>> = match *it.kind {
clean::StructItem(ref s) => sidebar_struct(cx, it, s),
Expand Down
5 changes: 4 additions & 1 deletion src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,15 @@ ul.block, .block li {
}

.sidebar h2 {
text-wrap: balance;
overflow-wrap: anywhere;
padding: 0;
margin: 0.7rem 0;
}

.sidebar h3 {
text-wrap: balance;
overflow-wrap: anywhere;
font-size: 1.125rem; /* 18px */
padding: 0;
margin: 0;
Expand Down Expand Up @@ -2222,7 +2225,7 @@ in src-script.js and main.js
width: 33%;
}
.item-table > li > div {
word-break: break-all;
overflow-wrap: anywhere;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/templates/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
</a> {# #}
{% endif %}
<h2> {# #}
<a href="{{page.root_path|safe}}{{display_krate_with_trailing_slash|safe}}index.html">{{display_krate}}</a> {# #}
<a href="{{page.root_path|safe}}{{display_krate_with_trailing_slash|safe}}index.html">{{display_krate|wrapped|safe}}</a> {# #}
{% if !display_krate_version_number.is_empty() %}
<span class="version">{{+ display_krate_version_number}}</span>
{% endif %}
Expand Down
8 changes: 5 additions & 3 deletions src/librustdoc/html/templates/sidebar.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% if !title.is_empty() %}
<h2 class="location"> {# #}
<a href="#">{{title_prefix}}{{title}}</a> {# #}
<a href="#">{{title_prefix}}{{title|wrapped|safe}}</a> {# #}
</h2>
{% endif %}
<div class="sidebar-elems">
Expand All @@ -15,7 +15,9 @@ <h2 class="location"> {# #}
{% for block in blocks %}
{% if block.should_render() %}
{% if !block.heading.name.is_empty() %}
<h3><a href="#{{block.heading.href|safe}}">{{block.heading.name}}</a></h3>
<h3><a href="#{{block.heading.href|safe}}"> {# #}
{{block.heading.name|wrapped|safe}} {# #}
</a></h3> {# #}
{% endif %}
{% if !block.links.is_empty() %}
<ul class="block{% if !block.class.is_empty() +%} {{+block.class}}{% endif %}">
Expand All @@ -29,6 +31,6 @@ <h3><a href="#{{block.heading.href|safe}}">{{block.heading.name}}</a></h3>
</section>
{% endif %}
{% if !path.is_empty() %}
<h2><a href="{% if is_mod %}../{% endif %}index.html">In {{+ path}}</a></h2>
<h2><a href="{% if is_mod %}../{% endif %}index.html">In {{+ path|wrapped|safe}}</a></h2>
{% endif %}
</div>
1 change: 1 addition & 0 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2731,6 +2731,7 @@ impl<'test> TestCx<'test> {

#[rustfmt::skip]
let tidy_args = [
"--new-blocklevel-tags", "rustdoc-search",
"--indent", "yes",
"--indent-spaces", "2",
"--wrap", "0",
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc-gui/duplicate-macro-reexport.goml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ go-to: "file://" + |DOC_PATH| + "/test_docs/macro.a.html"
wait-for: ".sidebar-elems .macro"
// Check there is only one macro named "a" listed in the sidebar.
assert-count: (
"//*[@class='sidebar-elems']//*[@class='block macro']//li/a[text()='a']",
"//*[@class='sidebar-elems']//*[@class='block macro']//li/a[normalize-space()='a']",
1,
)
// Check there is only one macro named "b" listed in the sidebar.
assert-count: (
"//*[@class='sidebar-elems']//*[@class='block macro']//li/a[text()='b']",
"//*[@class='sidebar-elems']//*[@class='block macro']//li/a[normalize-space()='b']",
1,
)
4 changes: 2 additions & 2 deletions tests/rustdoc-gui/font-weight.goml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// This test checks that the font weight is correctly applied.
go-to: "file://" + |DOC_PATH| + "/lib2/struct.Foo.html"
assert-css: ("//*[@class='rust item-decl']//a[text()='Alias']", {"font-weight": "400"})
assert-css: ("//*[@class='rust item-decl']//a[normalize-space()='Alias']", {"font-weight": "400"})
assert-css: (
"//*[@class='structfield section-header']//a[text()='Alias']",
"//*[@class='structfield section-header']//a[normalize-space()='Alias']",
{"font-weight": "400"},
)
assert-css: ("#method\.a_method > .code-header", {"font-weight": "600"})
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc-gui/item-info.goml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ assert-position: (".item-info .stab", {"x": 245})
// test for <https://github.com/rust-lang/rust/issues/118615>.
set-window-size: (850, 800)
store-position: (
"//*[@class='stab portability']//code[text()='Win32_System']",
"//*[@class='stab portability']//code[normalize-space()='Win32_System']",
{"x": first_line_x, "y": first_line_y},
)
store-position: (
"//*[@class='stab portability']//code[text()='Win32_System_Diagnostics']",
"//*[@class='stab portability']//code[normalize-space()='Win32_System_Diagnostics']",
{"x": second_line_x, "y": second_line_y},
)
assert: |first_line_x| != |second_line_x| && |first_line_x| == 516 && |second_line_x| == 272
Expand Down
Loading

0 comments on commit 9aedec9

Please sign in to comment.