From a8de713e26cb0f8e8ae4a7ecb0bf8a413b539926 Mon Sep 17 00:00:00 2001 From: Wim Looman Date: Sun, 9 Aug 2020 13:25:18 +0200 Subject: [PATCH 1/3] Improve rendering of crate features via doc(cfg) --- src/librustdoc/clean/cfg.rs | 7 +++++++ src/test/rustdoc/duplicate-cfg.rs | 10 +++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/cfg.rs b/src/librustdoc/clean/cfg.rs index 53979d27052d3..20da28d1c5b98 100644 --- a/src/librustdoc/clean/cfg.rs +++ b/src/librustdoc/clean/cfg.rs @@ -413,6 +413,13 @@ impl<'a> fmt::Display for Html<'a> { return write!(fmt, "target feature {}", feat); } } + (sym::feature, Some(feat)) => { + if self.1 { + return write!(fmt, "{}", feat); + } else { + return write!(fmt, "crate feature {}", feat); + } + } _ => "", }; if !human_readable.is_empty() { diff --git a/src/test/rustdoc/duplicate-cfg.rs b/src/test/rustdoc/duplicate-cfg.rs index 9ccc5d7882eb8..f6d4e9968a301 100644 --- a/src/test/rustdoc/duplicate-cfg.rs +++ b/src/test/rustdoc/duplicate-cfg.rs @@ -4,13 +4,13 @@ #![feature(doc_cfg)] // @has 'foo/struct.Foo.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync only.' #[doc(cfg(feature = "sync"))] #[doc(cfg(feature = "sync"))] pub struct Foo; // @has 'foo/bar/struct.Bar.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync only.' #[doc(cfg(feature = "sync"))] pub mod bar { #[doc(cfg(feature = "sync"))] @@ -18,7 +18,7 @@ pub mod bar { } // @has 'foo/baz/struct.Baz.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send only.' #[doc(cfg(all(feature = "sync", feature = "send")))] pub mod baz { #[doc(cfg(feature = "sync"))] @@ -26,7 +26,7 @@ pub mod baz { } // @has 'foo/qux/struct.Qux.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send only.' #[doc(cfg(feature = "sync"))] pub mod qux { #[doc(cfg(all(feature = "sync", feature = "send")))] @@ -34,7 +34,7 @@ pub mod qux { } // @has 'foo/quux/struct.Quux.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on feature="sync" and feature="send" and foo and bar only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send and foo and bar only.' #[doc(cfg(all(feature = "sync", feature = "send", foo)))] pub mod quux { #[doc(cfg(all(feature = "send", feature = "sync", bar)))] From 234ec956ab91d4aef51b63f25b78d176aa364a60 Mon Sep 17 00:00:00 2001 From: Wim Looman Date: Sun, 9 Aug 2020 14:01:19 +0200 Subject: [PATCH 2/3] Render longhand multiple crate/target features nicer --- src/librustdoc/clean/cfg.rs | 51 +++++++++++++++++++++++++++++-- src/test/rustdoc/duplicate-cfg.rs | 4 +-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/cfg.rs b/src/librustdoc/clean/cfg.rs index 20da28d1c5b98..3741d517f6afd 100644 --- a/src/librustdoc/clean/cfg.rs +++ b/src/librustdoc/clean/cfg.rs @@ -324,21 +324,68 @@ impl<'a> fmt::Display for Html<'a> { Cfg::Any(ref sub_cfgs) => { let separator = if sub_cfgs.iter().all(Cfg::is_simple) { " or " } else { ", or " }; + + let short_longhand = !self.1 && { + let all_crate_features = sub_cfgs + .iter() + .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_)))); + let all_target_features = sub_cfgs + .iter() + .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::target_feature, Some(_)))); + + if all_crate_features { + fmt.write_str("crate features ")?; + true + } else if all_target_features { + fmt.write_str("target features ")?; + true + } else { + false + } + }; + for (i, sub_cfg) in sub_cfgs.iter().enumerate() { if i != 0 { fmt.write_str(separator)?; } - write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?; + if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) { + write!(fmt, "{}", feat)?; + } else { + write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?; + } } Ok(()) } Cfg::All(ref sub_cfgs) => { + let short_longhand = !self.1 && { + let all_crate_features = sub_cfgs + .iter() + .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_)))); + let all_target_features = sub_cfgs + .iter() + .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::target_feature, Some(_)))); + + if all_crate_features { + fmt.write_str("crate features ")?; + true + } else if all_target_features { + fmt.write_str("target features ")?; + true + } else { + false + } + }; + for (i, sub_cfg) in sub_cfgs.iter().enumerate() { if i != 0 { fmt.write_str(" and ")?; } - write_with_opt_paren(fmt, !sub_cfg.is_simple(), Html(sub_cfg, self.1))?; + if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) { + write!(fmt, "{}", feat)?; + } else { + write_with_opt_paren(fmt, !sub_cfg.is_simple(), Html(sub_cfg, self.1))?; + } } Ok(()) } diff --git a/src/test/rustdoc/duplicate-cfg.rs b/src/test/rustdoc/duplicate-cfg.rs index f6d4e9968a301..99d02bc288c4a 100644 --- a/src/test/rustdoc/duplicate-cfg.rs +++ b/src/test/rustdoc/duplicate-cfg.rs @@ -18,7 +18,7 @@ pub mod bar { } // @has 'foo/baz/struct.Baz.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate features sync and send only.' #[doc(cfg(all(feature = "sync", feature = "send")))] pub mod baz { #[doc(cfg(feature = "sync"))] @@ -26,7 +26,7 @@ pub mod baz { } // @has 'foo/qux/struct.Qux.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send only.' +// @has '-' '//*[@class="stab portability"]' 'This is supported on crate features sync and send only.' #[doc(cfg(feature = "sync"))] pub mod qux { #[doc(cfg(all(feature = "sync", feature = "send")))] From 3328bd9a0f19b4c7bb8932b262583b8edf885338 Mon Sep 17 00:00:00 2001 From: Wim Looman Date: Tue, 18 Aug 2020 22:19:02 +0200 Subject: [PATCH 3/3] Add long cfg description to tooltip on short description --- src/librustdoc/clean/cfg.rs | 115 +++++++++++++++++++++--------- src/librustdoc/html/render/mod.rs | 10 +-- src/test/rustdoc/duplicate-cfg.rs | 18 ++++- 3 files changed, 105 insertions(+), 38 deletions(-) diff --git a/src/librustdoc/clean/cfg.rs b/src/librustdoc/clean/cfg.rs index 3741d517f6afd..1839683b61004 100644 --- a/src/librustdoc/clean/cfg.rs +++ b/src/librustdoc/clean/cfg.rs @@ -135,7 +135,7 @@ impl Cfg { /// Renders the configuration for human display, as a short HTML description. pub(crate) fn render_short_html(&self) -> String { - let mut msg = Html(self, true).to_string(); + let mut msg = Display(self, Format::ShortHtml).to_string(); if self.should_capitalize_first_letter() { if let Some(i) = msg.find(|c: char| c.is_ascii_alphanumeric()) { msg[i..i + 1].make_ascii_uppercase(); @@ -148,7 +148,11 @@ impl Cfg { pub(crate) fn render_long_html(&self) -> String { let on = if self.should_use_with_in_description() { "with" } else { "on" }; - let mut msg = format!("This is supported {} {}", on, Html(self, false)); + let mut msg = format!( + "This is supported {} {}", + on, + Display(self, Format::LongHtml) + ); if self.should_append_only_to_description() { msg.push_str(" only"); } @@ -156,6 +160,17 @@ impl Cfg { msg } + /// Renders the configuration for long display, as a long plain text description. + pub(crate) fn render_long_plain(&self) -> String { + let on = if self.should_use_with_in_description() { "with" } else { "on" }; + + let mut msg = format!("This is supported {} {}", on, Display(self, Format::LongPlain)); + if self.should_append_only_to_description() { + msg.push_str(" only"); + } + msg + } + fn should_capitalize_first_letter(&self) -> bool { match *self { Cfg::False | Cfg::True | Cfg::Not(..) => true, @@ -286,9 +301,31 @@ impl ops::BitOr for Cfg { } } -/// Pretty-print wrapper for a `Cfg`. Also indicates whether the "short-form" rendering should be -/// used. -struct Html<'a>(&'a Cfg, bool); +#[derive(Clone, Copy)] +enum Format { + LongHtml, + LongPlain, + ShortHtml, +} + +impl Format { + fn is_long(self) -> bool { + match self { + Format::LongHtml | Format::LongPlain => true, + Format::ShortHtml => false, + } + } + + fn is_html(self) -> bool { + match self { + Format::LongHtml | Format::ShortHtml => true, + Format::LongPlain => false, + } + } +} + +/// Pretty-print wrapper for a `Cfg`. Also indicates what form of rendering should be used. +struct Display<'a>(&'a Cfg, Format); fn write_with_opt_paren( fmt: &mut fmt::Formatter<'_>, @@ -305,7 +342,7 @@ fn write_with_opt_paren( Ok(()) } -impl<'a> fmt::Display for Html<'a> { +impl<'a> fmt::Display for Display<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match *self.0 { Cfg::Not(ref child) => match **child { @@ -314,18 +351,18 @@ impl<'a> fmt::Display for Html<'a> { if sub_cfgs.iter().all(Cfg::is_simple) { " nor " } else { ", nor " }; for (i, sub_cfg) in sub_cfgs.iter().enumerate() { fmt.write_str(if i == 0 { "neither " } else { separator })?; - write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?; + write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?; } Ok(()) } - ref simple @ Cfg::Cfg(..) => write!(fmt, "non-{}", Html(simple, self.1)), - ref c => write!(fmt, "not ({})", Html(c, self.1)), + ref simple @ Cfg::Cfg(..) => write!(fmt, "non-{}", Display(simple, self.1)), + ref c => write!(fmt, "not ({})", Display(c, self.1)), }, Cfg::Any(ref sub_cfgs) => { let separator = if sub_cfgs.iter().all(Cfg::is_simple) { " or " } else { ", or " }; - let short_longhand = !self.1 && { + let short_longhand = self.1.is_long() && { let all_crate_features = sub_cfgs .iter() .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_)))); @@ -349,16 +386,20 @@ impl<'a> fmt::Display for Html<'a> { fmt.write_str(separator)?; } if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) { - write!(fmt, "{}", feat)?; + if self.1.is_html() { + write!(fmt, "{}", feat)?; + } else { + write!(fmt, "`{}`", feat)?; + } } else { - write_with_opt_paren(fmt, !sub_cfg.is_all(), Html(sub_cfg, self.1))?; + write_with_opt_paren(fmt, !sub_cfg.is_all(), Display(sub_cfg, self.1))?; } } Ok(()) } Cfg::All(ref sub_cfgs) => { - let short_longhand = !self.1 && { + let short_longhand = self.1.is_long() && { let all_crate_features = sub_cfgs .iter() .all(|sub_cfg| matches!(sub_cfg, Cfg::Cfg(sym::feature, Some(_)))); @@ -382,9 +423,13 @@ impl<'a> fmt::Display for Html<'a> { fmt.write_str(" and ")?; } if let (true, Cfg::Cfg(_, Some(feat))) = (short_longhand, sub_cfg) { - write!(fmt, "{}", feat)?; + if self.1.is_html() { + write!(fmt, "{}", feat)?; + } else { + write!(fmt, "`{}`", feat)?; + } } else { - write_with_opt_paren(fmt, !sub_cfg.is_simple(), Html(sub_cfg, self.1))?; + write_with_opt_paren(fmt, !sub_cfg.is_simple(), Display(sub_cfg, self.1))?; } } Ok(()) @@ -453,33 +498,39 @@ impl<'a> fmt::Display for Html<'a> { }, (sym::target_endian, Some(endian)) => return write!(fmt, "{}-endian", endian), (sym::target_pointer_width, Some(bits)) => return write!(fmt, "{}-bit", bits), - (sym::target_feature, Some(feat)) => { - if self.1 { - return write!(fmt, "{}", feat); - } else { + (sym::target_feature, Some(feat)) => match self.1 { + Format::LongHtml => { return write!(fmt, "target feature {}", feat); } - } - (sym::feature, Some(feat)) => { - if self.1 { - return write!(fmt, "{}", feat); - } else { + Format::LongPlain => return write!(fmt, "target feature `{}`", feat), + Format::ShortHtml => return write!(fmt, "{}", feat), + }, + (sym::feature, Some(feat)) => match self.1 { + Format::LongHtml => { return write!(fmt, "crate feature {}", feat); } - } + Format::LongPlain => return write!(fmt, "crate feature `{}`", feat), + Format::ShortHtml => return write!(fmt, "{}", feat), + }, _ => "", }; if !human_readable.is_empty() { fmt.write_str(human_readable) } else if let Some(v) = value { - write!( - fmt, - "{}=\"{}\"", - Escape(&name.as_str()), - Escape(&v.as_str()) - ) - } else { + if self.1.is_html() { + write!( + fmt, + r#"{}="{}""#, + Escape(&name.as_str()), + Escape(&v.as_str()) + ) + } else { + write!(fmt, r#"`{}="{}"`"#, name, v) + } + } else if self.1.is_html() { write!(fmt, "{}", Escape(&name.as_str())) + } else { + write!(fmt, "`{}`", name) } } } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 5fb2d9f6f917c..de0964f530200 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2108,8 +2108,8 @@ fn item_module(w: &mut Buffer, cx: &Context, item: &clean::Item, items: &[clean: fn stability_tags(item: &clean::Item) -> String { let mut tags = String::new(); - fn tag_html(class: &str, contents: &str) -> String { - format!(r#"{}"#, class, contents) + fn tag_html(class: &str, title: &str, contents: &str) -> String { + format!(r#"{}"#, class, Escape(title), contents) } // The trailing space after each tag is to space it properly against the rest of the docs. @@ -2118,7 +2118,7 @@ fn stability_tags(item: &clean::Item) -> String { if !stability::deprecation_in_effect(depr.is_since_rustc_version, depr.since.as_deref()) { message = "Deprecation planned"; } - tags += &tag_html("deprecated", message); + tags += &tag_html("deprecated", "", message); } // The "rustc_private" crates are permanently unstable so it makes no sense @@ -2129,11 +2129,11 @@ fn stability_tags(item: &clean::Item) -> String { .map(|s| s.level == stability::Unstable && s.feature.as_deref() != Some("rustc_private")) == Some(true) { - tags += &tag_html("unstable", "Experimental"); + tags += &tag_html("unstable", "", "Experimental"); } if let Some(ref cfg) = item.attrs.cfg { - tags += &tag_html("portability", &cfg.render_short_html()); + tags += &tag_html("portability", &cfg.render_long_plain(), &cfg.render_short_html()); } tags diff --git a/src/test/rustdoc/duplicate-cfg.rs b/src/test/rustdoc/duplicate-cfg.rs index 99d02bc288c4a..47ba362c97789 100644 --- a/src/test/rustdoc/duplicate-cfg.rs +++ b/src/test/rustdoc/duplicate-cfg.rs @@ -3,12 +3,20 @@ #![crate_name = "foo"] #![feature(doc_cfg)] +// @has 'foo/index.html' +// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync$' +// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` only' + // @has 'foo/struct.Foo.html' -// @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync only.' +// @has '-' '//*[@class="stab portability"]' 'sync' #[doc(cfg(feature = "sync"))] #[doc(cfg(feature = "sync"))] pub struct Foo; +// @has 'foo/bar/index.html' +// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync$' +// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` only' + // @has 'foo/bar/struct.Bar.html' // @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync only.' #[doc(cfg(feature = "sync"))] @@ -17,6 +25,10 @@ pub mod bar { pub struct Bar; } +// @has 'foo/baz/index.html' +// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync and send$' +// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate features `sync` and `send` only' + // @has 'foo/baz/struct.Baz.html' // @has '-' '//*[@class="stab portability"]' 'This is supported on crate features sync and send only.' #[doc(cfg(all(feature = "sync", feature = "send")))] @@ -33,6 +45,10 @@ pub mod qux { pub struct Qux; } +// @has 'foo/quux/index.html' +// @matches '-' '//*[@class="module-item"]//*[@class="stab portability"]' '^sync and send and foo and bar$' +// @has '-' '//*[@class="module-item"]//*[@class="stab portability"]/@title' 'This is supported on crate feature `sync` and crate feature `send` and `foo` and `bar` only' + // @has 'foo/quux/struct.Quux.html' // @has '-' '//*[@class="stab portability"]' 'This is supported on crate feature sync and crate feature send and foo and bar only.' #[doc(cfg(all(feature = "sync", feature = "send", foo)))]