Skip to content

Commit 8730c2b

Browse files
authored
Rollup merge of #75330 - Nemo157:improve-doc-cfg-features, r=GuillaumeGomez
Improve rendering of crate features via doc(cfg) The current rendering of crate features with `doc(cfg(feature = ".."))` is verbose and unwieldy for users, `doc(cfg(target_feature = ".."))` is special-cased to make it render nicely, and a similar rendering can be applied to `doc(cfg(feature))` to make it easier for users to read. I also added special casing of `all`/`any` cfgs consisting of just `feature`/`target-feature` to remove the repetitive "target/crate feature" prefix. The downside of this current rendering is that there is no distinction between `feature` and `target_feature` in the shorthand display. IMO this is ok, or if anything `target_feature` should have a more verbose shorthand, because `doc(cfg(feature = ".."))` usage is going to vastly outstrip `doc(cfg(target_feature = ".."))` usage in non-stdlib crates when it eventually stabilizes (or even before that given the number of crates using `cfg_attr(docsrs)` like constructs). ## Previously <img width="259" alt="Screenshot 2020-08-09 at 13 32 42" src="https://user-images.githubusercontent.com/81079/89731110-d090c000-da44-11ea-96fa-56adc6339123.png"> <img width="438" alt="image" src="https://user-images.githubusercontent.com/81079/89731116-d7b7ce00-da44-11ea-87c6-022d192d6eca.png"> <img width="765" alt="image" src="https://user-images.githubusercontent.com/81079/89731152-24030e00-da45-11ea-9552-1c270bff2729.png"> <img width="671" alt="image" src="https://user-images.githubusercontent.com/81079/89731158-28c7c200-da45-11ea-8acb-97d8a4ce00eb.png"> ## Now <img width="216" alt="image" src="https://user-images.githubusercontent.com/81079/89731123-e1d9cc80-da44-11ea-82a8-5900bd9448a5.png"> <img width="433" alt="image" src="https://user-images.githubusercontent.com/81079/89731127-e8684400-da44-11ea-9d18-572fd810f19f.png"> <img width="606" alt="image" src="https://user-images.githubusercontent.com/81079/89731162-2feed000-da45-11ea-98d2-8a88c364d903.png"> <img width="669" alt="image" src="https://user-images.githubusercontent.com/81079/89731991-ccb46c00-da4b-11ea-9416-cd20a3193826.png"> cc #43781
2 parents 41aaa90 + 3328bd9 commit 8730c2b

File tree

3 files changed

+154
-33
lines changed

3 files changed

+154
-33
lines changed

src/librustdoc/clean/cfg.rs

+128-23
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl Cfg {
135135

136136
/// Renders the configuration for human display, as a short HTML description.
137137
pub(crate) fn render_short_html(&self) -> String {
138-
let mut msg = Html(self, true).to_string();
138+
let mut msg = Display(self, Format::ShortHtml).to_string();
139139
if self.should_capitalize_first_letter() {
140140
if let Some(i) = msg.find(|c: char| c.is_ascii_alphanumeric()) {
141141
msg[i..i + 1].make_ascii_uppercase();
@@ -148,14 +148,29 @@ impl Cfg {
148148
pub(crate) fn render_long_html(&self) -> String {
149149
let on = if self.should_use_with_in_description() { "with" } else { "on" };
150150

151-
let mut msg = format!("This is supported {} <strong>{}</strong>", on, Html(self, false));
151+
let mut msg = format!(
152+
"This is supported {} <strong>{}</strong>",
153+
on,
154+
Display(self, Format::LongHtml)
155+
);
152156
if self.should_append_only_to_description() {
153157
msg.push_str(" only");
154158
}
155159
msg.push('.');
156160
msg
157161
}
158162

163+
/// Renders the configuration for long display, as a long plain text description.
164+
pub(crate) fn render_long_plain(&self) -> String {
165+
let on = if self.should_use_with_in_description() { "with" } else { "on" };
166+
167+
let mut msg = format!("This is supported {} {}", on, Display(self, Format::LongPlain));
168+
if self.should_append_only_to_description() {
169+
msg.push_str(" only");
170+
}
171+
msg
172+
}
173+
159174
fn should_capitalize_first_letter(&self) -> bool {
160175
match *self {
161176
Cfg::False | Cfg::True | Cfg::Not(..) => true,
@@ -286,9 +301,31 @@ impl ops::BitOr for Cfg {
286301
}
287302
}
288303

289-
/// Pretty-print wrapper for a `Cfg`. Also indicates whether the "short-form" rendering should be
290-
/// used.
291-
struct Html<'a>(&'a Cfg, bool);
304+
#[derive(Clone, Copy)]
305+
enum Format {
306+
LongHtml,
307+
LongPlain,
308+
ShortHtml,
309+
}
310+
311+
impl Format {
312+
fn is_long(self) -> bool {
313+
match self {
314+
Format::LongHtml | Format::LongPlain => true,
315+
Format::ShortHtml => false,
316+
}
317+
}
318+
319+
fn is_html(self) -> bool {
320+
match self {
321+
Format::LongHtml | Format::ShortHtml => true,
322+
Format::LongPlain => false,
323+
}
324+
}
325+
}
326+
327+
/// Pretty-print wrapper for a `Cfg`. Also indicates what form of rendering should be used.
328+
struct Display<'a>(&'a Cfg, Format);
292329

293330
fn write_with_opt_paren<T: fmt::Display>(
294331
fmt: &mut fmt::Formatter<'_>,
@@ -305,7 +342,7 @@ fn write_with_opt_paren<T: fmt::Display>(
305342
Ok(())
306343
}
307344

308-
impl<'a> fmt::Display for Html<'a> {
345+
impl<'a> fmt::Display for Display<'a> {
309346
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
310347
match *self.0 {
311348
Cfg::Not(ref child) => match **child {
@@ -314,31 +351,86 @@ impl<'a> fmt::Display for Html<'a> {
314351
if sub_cfgs.iter().all(Cfg::is_simple) { " nor " } else { ", nor " };
315352
for (i, sub_cfg) in sub_cfgs.iter().enumerate() {
316353
fmt.write_str(if i == 0 { "neither " } else { separator })?;
317-
write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?;
354+
write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?;
318355
}
319356
Ok(())
320357
}
321-
ref simple @ Cfg::Cfg(..) => write!(fmt, "non-{}", Html(simple, self.1)),
322-
ref c => write!(fmt, "not ({})", Html(c, self.1)),
358+
ref simple @ Cfg::Cfg(..) => write!(fmt, "non-{}", Display(simple, self.1)),
359+
ref c => write!(fmt, "not ({})", Display(c, self.1)),
323360
},
324361

325362
Cfg::Any(ref sub_cfgs) => {
326363
let separator = if sub_cfgs.iter().all(Cfg::is_simple) { " or " } else { ", or " };
364+
365+
let short_longhand = self.1.is_long() && {
366+
let all_crate_features = sub_cfgs
367+
.iter()
368+
.all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_))));
369+
let all_target_features = sub_cfgs
370+
.iter()
371+
.all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::target_feature, Some(_))));
372+
373+
if all_crate_features {
374+
fmt.write_str("crate features ")?;
375+
true
376+
} else if all_target_features {
377+
fmt.write_str("target features ")?;
378+
true
379+
} else {
380+
false
381+
}
382+
};
383+
327384
for (i, sub_cfg) in sub_cfgs.iter().enumerate() {
328385
if i != 0 {
329386
fmt.write_str(separator)?;
330387
}
331-
write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?;
388+
if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) {
389+
if self.1.is_html() {
390+
write!(fmt, "<code>{}</code>", feat)?;
391+
} else {
392+
write!(fmt, "`{}`", feat)?;
393+
}
394+
} else {
395+
write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?;
396+
}
332397
}
333398
Ok(())
334399
}
335400

336401
Cfg::All(ref sub_cfgs) => {
402+
let short_longhand = self.1.is_long() && {
403+
let all_crate_features = sub_cfgs
404+
.iter()
405+
.all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_))));
406+
let all_target_features = sub_cfgs
407+
.iter()
408+
.all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::target_feature, Some(_))));
409+
410+
if all_crate_features {
411+
fmt.write_str("crate features ")?;
412+
true
413+
} else if all_target_features {
414+
fmt.write_str("target features ")?;
415+
true
416+
} else {
417+
false
418+
}
419+
};
420+
337421
for (i, sub_cfg) in sub_cfgs.iter().enumerate() {
338422
if i != 0 {
339423
fmt.write_str(" and ")?;
340424
}
341-
write_with_opt_paren(fmt, !sub_cfg.is_simple(), Html(sub_cfg, self.1))?;
425+
if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) {
426+
if self.1.is_html() {
427+
write!(fmt, "<code>{}</code>", feat)?;
428+
} else {
429+
write!(fmt, "`{}`", feat)?;
430+
}
431+
} else {
432+
write_with_opt_paren(fmt, !sub_cfg.is_simple(), Display(sub_cfg, self.1))?;
433+
}
342434
}
343435
Ok(())
344436
}
@@ -406,26 +498,39 @@ impl<'a> fmt::Display for Html<'a> {
406498
},
407499
(sym::target_endian, Some(endian)) => return write!(fmt, "{}-endian", endian),
408500
(sym::target_pointer_width, Some(bits)) => return write!(fmt, "{}-bit", bits),
409-
(sym::target_feature, Some(feat)) => {
410-
if self.1 {
411-
return write!(fmt, "<code>{}</code>", feat);
412-
} else {
501+
(sym::target_feature, Some(feat)) => match self.1 {
502+
Format::LongHtml => {
413503
return write!(fmt, "target feature <code>{}</code>", feat);
414504
}
415-
}
505+
Format::LongPlain => return write!(fmt, "target feature `{}`", feat),
506+
Format::ShortHtml => return write!(fmt, "<code>{}</code>", feat),
507+
},
508+
(sym::feature, Some(feat)) => match self.1 {
509+
Format::LongHtml => {
510+
return write!(fmt, "crate feature <code>{}</code>", feat);
511+
}
512+
Format::LongPlain => return write!(fmt, "crate feature `{}`", feat),
513+
Format::ShortHtml => return write!(fmt, "<code>{}</code>", feat),
514+
},
416515
_ => "",
417516
};
418517
if !human_readable.is_empty() {
419518
fmt.write_str(human_readable)
420519
} else if let Some(v) = value {
421-
write!(
422-
fmt,
423-
"<code>{}=\"{}\"</code>",
424-
Escape(&name.as_str()),
425-
Escape(&v.as_str())
426-
)
427-
} else {
520+
if self.1.is_html() {
521+
write!(
522+
fmt,
523+
r#"<code>{}="{}"</code>"#,
524+
Escape(&name.as_str()),
525+
Escape(&v.as_str())
526+
)
527+
} else {
528+
write!(fmt, r#"`{}="{}"`"#, name, v)
529+
}
530+
} else if self.1.is_html() {
428531
write!(fmt, "<code>{}</code>", Escape(&name.as_str()))
532+
} else {
533+
write!(fmt, "`{}`", name)
429534
}
430535
}
431536
}

src/librustdoc/html/render/mod.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -2130,8 +2130,8 @@ fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean:
21302130
fn stability_tags(item: &clean::Item) -> String {
21312131
let mut tags = String::new();
21322132

2133-
fn tag_html(class: &str, contents: &str) -> String {
2134-
format!(r#"<span class="stab {}">{}</span>"#, class, contents)
2133+
fn tag_html(class: &str, title: &str, contents: &str) -> String {
2134+
format!(r#"<span class="stab {}" title="{}">{}</span>"#, class, Escape(title), contents)
21352135
}
21362136

21372137
// The trailing space after each tag is to space it properly against the rest of the docs.
@@ -2140,7 +2140,7 @@ fn stability_tags(item: &clean::Item) -> String {
21402140
if !stability::deprecation_in_effect(depr.is_since_rustc_version, depr.since.as_deref()) {
21412141
message = "Deprecation planned";
21422142
}
2143-
tags += &tag_html("deprecated", message);
2143+
tags += &tag_html("deprecated", "", message);
21442144
}
21452145

21462146
// The "rustc_private" crates are permanently unstable so it makes no sense
@@ -2151,11 +2151,11 @@ fn stability_tags(item: &clean::Item) -> String {
21512151
.map(|s| s.level == stability::Unstable && s.feature != "rustc_private")
21522152
== Some(true)
21532153
{
2154-
tags += &tag_html("unstable", "Experimental");
2154+
tags += &tag_html("unstable", "", "Experimental");
21552155
}
21562156

21572157
if let Some(ref cfg) = item.attrs.cfg {
2158-
tags += &tag_html("portability", &cfg.render_short_html());
2158+
tags += &tag_html("portability", &cfg.render_long_plain(), &cfg.render_short_html());
21592159
}
21602160

21612161
tags

src/test/rustdoc/duplicate-cfg.rs

+21-5
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,54 @@
33
#![crate_name = "foo"]
44
#![feature(doc_cfg)]
55

6+
// @has 'foo/index.html'
7+
// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync$'
8+
// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` only'
9+
610
// @has 'foo/struct.Foo.html'
7-
// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" only.'
11+
// @has '-' '//*[@class="stab portability"]' 'sync'
812
#[doc(cfg(feature = "sync"))]
913
#[doc(cfg(feature = "sync"))]
1014
pub struct Foo;
1115

16+
// @has 'foo/bar/index.html'
17+
// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync$'
18+
// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` only'
19+
1220
// @has 'foo/bar/struct.Bar.html'
13-
// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" only.'
21+
// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync only.'
1422
#[doc(cfg(feature = "sync"))]
1523
pub mod bar {
1624
#[doc(cfg(feature = "sync"))]
1725
pub struct Bar;
1826
}
1927

28+
// @has 'foo/baz/index.html'
29+
// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync and send$'
30+
// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate features `sync` and `send` only'
31+
2032
// @has 'foo/baz/struct.Baz.html'
21-
// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" only.'
33+
// @has '-' '//*[@class="stab portability"]' 'This is supported on crate features sync and send only.'
2234
#[doc(cfg(all(feature = "sync", feature = "send")))]
2335
pub mod baz {
2436
#[doc(cfg(feature = "sync"))]
2537
pub struct Baz;
2638
}
2739

2840
// @has 'foo/qux/struct.Qux.html'
29-
// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" only.'
41+
// @has '-' '//*[@class="stab portability"]' 'This is supported on crate features sync and send only.'
3042
#[doc(cfg(feature = "sync"))]
3143
pub mod qux {
3244
#[doc(cfg(all(feature = "sync", feature = "send")))]
3345
pub struct Qux;
3446
}
3547

48+
// @has 'foo/quux/index.html'
49+
// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync and send and foo and bar$'
50+
// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` and crate feature `send` and `foo` and `bar` only'
51+
3652
// @has 'foo/quux/struct.Quux.html'
37-
// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" and foo and bar only.'
53+
// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send and foo and bar only.'
3854
#[doc(cfg(all(feature = "sync", feature = "send", foo)))]
3955
pub mod quux {
4056
#[doc(cfg(all(feature = "send", feature = "sync", bar)))]

0 commit comments

Comments
 (0)