Skip to content

Commit aa61575

Browse files
Rollup merge of #88173 - camelid:refactor-markdown-length-limit, r=GuillaumeGomez
Refactor Markdown length-limited summary implementation This PR is a new approach to #79749. This PR refactors the implementation of `markdown_summary_with_limit()`, separating the logic of determining when the limit has been reached from the actual rendering process. The main advantage of the new approach is that it guarantees that all HTML tags are closed, whereas the previous implementation could generate tags that were never closed. It also ensures that no empty tags are generated (e.g., `<em></em>`). The new implementation consists of a general-purpose struct `HtmlWithLimit` that manages the length-limiting logic and a function `markdown_summary_with_limit()` that renders Markdown to HTML using the struct. r? `@GuillaumeGomez`
2 parents 63cfbf5 + 4478ecc commit aa61575

File tree

6 files changed

+271
-43
lines changed

6 files changed

+271
-43
lines changed

src/librustdoc/html/length_limit.rs

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
//! See [`HtmlWithLimit`].
2+
3+
use std::fmt::Write;
4+
use std::ops::ControlFlow;
5+
6+
use crate::html::escape::Escape;
7+
8+
/// A buffer that allows generating HTML with a length limit.
9+
///
10+
/// This buffer ensures that:
11+
///
12+
/// * all tags are closed,
13+
/// * tags are closed in the reverse order of when they were opened (i.e., the correct HTML order),
14+
/// * no tags are left empty (e.g., `<em></em>`) due to the length limit being reached,
15+
/// * all text is escaped.
16+
#[derive(Debug)]
17+
pub(super) struct HtmlWithLimit {
18+
buf: String,
19+
len: usize,
20+
limit: usize,
21+
/// A list of tags that have been requested to be opened via [`Self::open_tag()`]
22+
/// but have not actually been pushed to `buf` yet. This ensures that tags are not
23+
/// left empty (e.g., `<em></em>`) due to the length limit being reached.
24+
queued_tags: Vec<&'static str>,
25+
/// A list of all tags that have been opened but not yet closed.
26+
unclosed_tags: Vec<&'static str>,
27+
}
28+
29+
impl HtmlWithLimit {
30+
/// Create a new buffer, with a limit of `length_limit`.
31+
pub(super) fn new(length_limit: usize) -> Self {
32+
let buf = if length_limit > 1000 {
33+
// If the length limit is really large, don't preallocate tons of memory.
34+
String::new()
35+
} else {
36+
// The length limit is actually a good heuristic for initial allocation size.
37+
// Measurements showed that using it as the initial capacity ended up using less memory
38+
// than `String::new`.
39+
// See https://github.com/rust-lang/rust/pull/88173#discussion_r692531631 for more.
40+
String::with_capacity(length_limit)
41+
};
42+
Self {
43+
buf,
44+
len: 0,
45+
limit: length_limit,
46+
unclosed_tags: Vec::new(),
47+
queued_tags: Vec::new(),
48+
}
49+
}
50+
51+
/// Finish using the buffer and get the written output.
52+
/// This function will close all unclosed tags for you.
53+
pub(super) fn finish(mut self) -> String {
54+
self.close_all_tags();
55+
self.buf
56+
}
57+
58+
/// Write some plain text to the buffer, escaping as needed.
59+
///
60+
/// This function skips writing the text if the length limit was reached
61+
/// and returns [`ControlFlow::Break`].
62+
pub(super) fn push(&mut self, text: &str) -> ControlFlow<(), ()> {
63+
if self.len + text.len() > self.limit {
64+
return ControlFlow::BREAK;
65+
}
66+
67+
self.flush_queue();
68+
write!(self.buf, "{}", Escape(text)).unwrap();
69+
self.len += text.len();
70+
71+
ControlFlow::CONTINUE
72+
}
73+
74+
/// Open an HTML tag.
75+
///
76+
/// **Note:** HTML attributes have not yet been implemented.
77+
/// This function will panic if called with a non-alphabetic `tag_name`.
78+
pub(super) fn open_tag(&mut self, tag_name: &'static str) {
79+
assert!(
80+
tag_name.chars().all(|c| ('a'..='z').contains(&c)),
81+
"tag_name contained non-alphabetic chars: {:?}",
82+
tag_name
83+
);
84+
self.queued_tags.push(tag_name);
85+
}
86+
87+
/// Close the most recently opened HTML tag.
88+
pub(super) fn close_tag(&mut self) {
89+
match self.unclosed_tags.pop() {
90+
// Close the most recently opened tag.
91+
Some(tag_name) => write!(self.buf, "</{}>", tag_name).unwrap(),
92+
// There are valid cases where `close_tag()` is called without
93+
// there being any tags to close. For example, this occurs when
94+
// a tag is opened after the length limit is exceeded;
95+
// `flush_queue()` will never be called, and thus, the tag will
96+
// not end up being added to `unclosed_tags`.
97+
None => {}
98+
}
99+
}
100+
101+
/// Write all queued tags and add them to the `unclosed_tags` list.
102+
fn flush_queue(&mut self) {
103+
for tag_name in self.queued_tags.drain(..) {
104+
write!(self.buf, "<{}>", tag_name).unwrap();
105+
106+
self.unclosed_tags.push(tag_name);
107+
}
108+
}
109+
110+
/// Close all unclosed tags.
111+
fn close_all_tags(&mut self) {
112+
while !self.unclosed_tags.is_empty() {
113+
self.close_tag();
114+
}
115+
}
116+
}
117+
118+
#[cfg(test)]
119+
mod tests;
+120
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
use super::*;
2+
3+
#[test]
4+
fn empty() {
5+
assert_eq!(HtmlWithLimit::new(0).finish(), "");
6+
assert_eq!(HtmlWithLimit::new(60).finish(), "");
7+
}
8+
9+
#[test]
10+
fn basic() {
11+
let mut buf = HtmlWithLimit::new(60);
12+
buf.push("Hello ");
13+
buf.open_tag("em");
14+
buf.push("world");
15+
buf.close_tag();
16+
buf.push("!");
17+
assert_eq!(buf.finish(), "Hello <em>world</em>!");
18+
}
19+
20+
#[test]
21+
fn no_tags() {
22+
let mut buf = HtmlWithLimit::new(60);
23+
buf.push("Hello");
24+
buf.push(" world!");
25+
assert_eq!(buf.finish(), "Hello world!");
26+
}
27+
28+
#[test]
29+
fn limit_0() {
30+
let mut buf = HtmlWithLimit::new(0);
31+
buf.push("Hello ");
32+
buf.open_tag("em");
33+
buf.push("world");
34+
buf.close_tag();
35+
buf.push("!");
36+
assert_eq!(buf.finish(), "");
37+
}
38+
39+
#[test]
40+
fn exactly_limit() {
41+
let mut buf = HtmlWithLimit::new(12);
42+
buf.push("Hello ");
43+
buf.open_tag("em");
44+
buf.push("world");
45+
buf.close_tag();
46+
buf.push("!");
47+
assert_eq!(buf.finish(), "Hello <em>world</em>!");
48+
}
49+
50+
#[test]
51+
fn multiple_nested_tags() {
52+
let mut buf = HtmlWithLimit::new(60);
53+
buf.open_tag("p");
54+
buf.push("This is a ");
55+
buf.open_tag("em");
56+
buf.push("paragraph");
57+
buf.open_tag("strong");
58+
buf.push("!");
59+
buf.close_tag();
60+
buf.close_tag();
61+
buf.close_tag();
62+
assert_eq!(buf.finish(), "<p>This is a <em>paragraph<strong>!</strong></em></p>");
63+
}
64+
65+
#[test]
66+
fn forgot_to_close_tags() {
67+
let mut buf = HtmlWithLimit::new(60);
68+
buf.open_tag("p");
69+
buf.push("This is a ");
70+
buf.open_tag("em");
71+
buf.push("paragraph");
72+
buf.open_tag("strong");
73+
buf.push("!");
74+
assert_eq!(buf.finish(), "<p>This is a <em>paragraph<strong>!</strong></em></p>");
75+
}
76+
77+
#[test]
78+
fn past_the_limit() {
79+
let mut buf = HtmlWithLimit::new(20);
80+
buf.open_tag("p");
81+
(0..10).try_for_each(|n| {
82+
buf.open_tag("strong");
83+
buf.push("word#")?;
84+
buf.push(&n.to_string())?;
85+
buf.close_tag();
86+
ControlFlow::CONTINUE
87+
});
88+
buf.close_tag();
89+
assert_eq!(
90+
buf.finish(),
91+
"<p>\
92+
<strong>word#0</strong>\
93+
<strong>word#1</strong>\
94+
<strong>word#2</strong>\
95+
</p>"
96+
);
97+
}
98+
99+
#[test]
100+
fn quickly_past_the_limit() {
101+
let mut buf = HtmlWithLimit::new(6);
102+
buf.open_tag("p");
103+
buf.push("Hello");
104+
buf.push(" World");
105+
// intentionally not closing <p> before finishing
106+
assert_eq!(buf.finish(), "<p>Hello</p>");
107+
}
108+
109+
#[test]
110+
fn close_too_many() {
111+
let mut buf = HtmlWithLimit::new(60);
112+
buf.open_tag("p");
113+
buf.push("Hello");
114+
buf.close_tag();
115+
// This call does not panic because there are valid cases
116+
// where `close_tag()` is called with no tags left to close.
117+
// So `close_tag()` does nothing in this case.
118+
buf.close_tag();
119+
assert_eq!(buf.finish(), "<p>Hello</p>");
120+
}

src/librustdoc/html/markdown.rs

+28-43
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,21 @@ use rustc_hir::HirId;
2323
use rustc_middle::ty::TyCtxt;
2424
use rustc_span::edition::Edition;
2525
use rustc_span::Span;
26+
2627
use std::borrow::Cow;
2728
use std::cell::RefCell;
2829
use std::collections::VecDeque;
2930
use std::default::Default;
3031
use std::fmt::Write;
31-
use std::ops::Range;
32+
use std::ops::{ControlFlow, Range};
3233
use std::str;
3334

3435
use crate::clean::RenderedLink;
3536
use crate::doctest;
3637
use crate::html::escape::Escape;
3738
use crate::html::format::Buffer;
3839
use crate::html::highlight;
40+
use crate::html::length_limit::HtmlWithLimit;
3941
use crate::html::toc::TocBuilder;
4042

4143
use pulldown_cmark::{
@@ -1081,15 +1083,6 @@ fn markdown_summary_with_limit(
10811083
return (String::new(), false);
10821084
}
10831085

1084-
let mut s = String::with_capacity(md.len() * 3 / 2);
1085-
let mut text_length = 0;
1086-
let mut stopped_early = false;
1087-
1088-
fn push(s: &mut String, text_length: &mut usize, text: &str) {
1089-
write!(s, "{}", Escape(text)).unwrap();
1090-
*text_length += text.len();
1091-
}
1092-
10931086
let mut replacer = |broken_link: BrokenLink<'_>| {
10941087
if let Some(link) =
10951088
link_names.iter().find(|link| &*link.original_text == broken_link.reference)
@@ -1101,56 +1094,48 @@ fn markdown_summary_with_limit(
11011094
};
11021095

11031096
let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut replacer));
1104-
let p = LinkReplacer::new(p, link_names);
1097+
let mut p = LinkReplacer::new(p, link_names);
11051098

1106-
'outer: for event in p {
1099+
let mut buf = HtmlWithLimit::new(length_limit);
1100+
let mut stopped_early = false;
1101+
p.try_for_each(|event| {
11071102
match &event {
11081103
Event::Text(text) => {
1109-
for word in text.split_inclusive(char::is_whitespace) {
1110-
if text_length + word.len() >= length_limit {
1111-
stopped_early = true;
1112-
break 'outer;
1113-
}
1114-
1115-
push(&mut s, &mut text_length, word);
1104+
let r =
1105+
text.split_inclusive(char::is_whitespace).try_for_each(|word| buf.push(word));
1106+
if r.is_break() {
1107+
stopped_early = true;
11161108
}
1109+
return r;
11171110
}
11181111
Event::Code(code) => {
1119-
if text_length + code.len() >= length_limit {
1112+
buf.open_tag("code");
1113+
let r = buf.push(code);
1114+
if r.is_break() {
11201115
stopped_early = true;
1121-
break;
1116+
} else {
1117+
buf.close_tag();
11221118
}
1123-
1124-
s.push_str("<code>");
1125-
push(&mut s, &mut text_length, code);
1126-
s.push_str("</code>");
1119+
return r;
11271120
}
11281121
Event::Start(tag) => match tag {
1129-
Tag::Emphasis => s.push_str("<em>"),
1130-
Tag::Strong => s.push_str("<strong>"),
1131-
Tag::CodeBlock(..) => break,
1122+
Tag::Emphasis => buf.open_tag("em"),
1123+
Tag::Strong => buf.open_tag("strong"),
1124+
Tag::CodeBlock(..) => return ControlFlow::BREAK,
11321125
_ => {}
11331126
},
11341127
Event::End(tag) => match tag {
1135-
Tag::Emphasis => s.push_str("</em>"),
1136-
Tag::Strong => s.push_str("</strong>"),
1137-
Tag::Paragraph => break,
1138-
Tag::Heading(..) => break,
1128+
Tag::Emphasis | Tag::Strong => buf.close_tag(),
1129+
Tag::Paragraph | Tag::Heading(..) => return ControlFlow::BREAK,
11391130
_ => {}
11401131
},
1141-
Event::HardBreak | Event::SoftBreak => {
1142-
if text_length + 1 >= length_limit {
1143-
stopped_early = true;
1144-
break;
1145-
}
1146-
1147-
push(&mut s, &mut text_length, " ");
1148-
}
1132+
Event::HardBreak | Event::SoftBreak => buf.push(" ")?,
11491133
_ => {}
1150-
}
1151-
}
1134+
};
1135+
ControlFlow::CONTINUE
1136+
});
11521137

1153-
(s, stopped_early)
1138+
(buf.finish(), stopped_early)
11541139
}
11551140

11561141
/// Renders a shortened first paragraph of the given Markdown as a subset of Markdown,

src/librustdoc/html/markdown/tests.rs

+2
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ fn test_short_markdown_summary() {
225225
assert_eq!(output, expect, "original: {}", input);
226226
}
227227

228+
t("", "");
228229
t("hello [Rust](https://www.rust-lang.org) :)", "hello Rust :)");
229230
t("*italic*", "<em>italic</em>");
230231
t("**bold**", "<strong>bold</strong>");
@@ -264,6 +265,7 @@ fn test_plain_text_summary() {
264265
assert_eq!(output, expect, "original: {}", input);
265266
}
266267

268+
t("", "");
267269
t("hello [Rust](https://www.rust-lang.org) :)", "hello Rust :)");
268270
t("**bold**", "bold");
269271
t("Multi-line\nsummary", "Multi-line summary");

src/librustdoc/html/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ crate mod escape;
22
crate mod format;
33
crate mod highlight;
44
crate mod layout;
5+
mod length_limit;
56
// used by the error-index generator, so it needs to be public
67
pub mod markdown;
78
crate mod render;

src/librustdoc/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#![feature(rustc_private)]
66
#![feature(array_methods)]
77
#![feature(box_patterns)]
8+
#![feature(control_flow_enum)]
89
#![feature(in_band_lifetimes)]
910
#![feature(nll)]
1011
#![feature(test)]

0 commit comments

Comments
 (0)