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: word wrap CamelCase in the item list table and sidebar #126247

Merged
merged 7 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}} {# #}
notriddle marked this conversation as resolved.
Show resolved Hide resolved
</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']",
Copy link
Member

Choose a reason for hiding this comment

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

In a few cases like this one, are there <wbr> tags? Seems surprising, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used a regex to automatically replace all text() with normalize-space(). This case definitely doesn't have any <wbr> in it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. Please revert these cases then. :)

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
Loading