Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code by GuillaumeGomez · Pull Request #141770 · rust-lang/rust

GuillaumeGomez · 2025-05-30T13:36:57Z

Follow-up of #141747.

Thanks @camelid for spotting it!

…common code * Fix invalid whitespace handling

GuillaumeGomez · 2025-05-30T13:38:41Z

src/librustdoc/clean/cfg.rs

+    fn render_long_inner(&self, format: Format) -> String {
        let on = if self.omit_preposition() {
-            ""
+            " "


I changed this a bit because the generated whitespace characters were invalid in render_long_plain with the previous code (two whitespace characters after Available for this case).

A little confused about why this had to be changed. It seems like every branch of this if-else has a leading space. But it doesn't matter.

GuillaumeGomez · 2025-05-30T13:39:49Z

tests/rustdoc/cfg-bool.rs

-// a cfg(true) will simply be ommited, as it is the same as no cfg.
-//@ !has 'foo/fn.bar.html' '//div[@class="stab portability"]' ''
+// a cfg(true) will simply be omited, as it is the same as no cfg.
+//@ count 'foo/fn.bar.html' '//*[@class="stab portability"]' 0


We should maybe restrain usage of !has as it's so easily outdated...

Probably would be a good idea to just mention it in the docs we have now, "consider using count <...> 0 instead of !has

It's not much better though. We need to find a way to prevent it from ever rotting.

tests/rustdoc/cfg-bool.rs

* Fix typo * Remove usage of `!has`

GuillaumeGomez · 2025-06-03T08:36:51Z

Fixed typo.

camelid · 2025-06-13T02:02:58Z

src/librustdoc/clean/cfg.rs

+    fn render_long_inner(&self, format: Format) -> String {
        let on = if self.omit_preposition() {
-            ""
+            " "


A little confused about why this had to be changed. It seems like every branch of this if-else has a leading space. But it doesn't matter.

camelid · 2025-06-13T02:04:53Z

src/librustdoc/clean/cfg.rs

+    /// Renders the configuration for long display, as a long HTML description.
+    pub(crate) fn render_long_html(&self) -> String {
+        let mut msg = self.render_long_inner(Format::LongHtml);
        msg.push('.');


Doesn't matter for this PR since it's preexisting, but any idea why we end with a . for the html but not for the plain?

camelid · 2025-06-13T02:05:30Z

@bors r+ rollup

bors · 2025-06-13T02:06:10Z

📌 Commit fca28ab has been approved by camelid

It is now in the queue for this repository.

Rollup of 10 pull requests Successful merges: - #134847 (Implement asymmetrical precedence for closures and jumps) - #141491 (Delegate `<CStr as Debug>` to `ByteStr`) - #141770 (Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code) - #142069 (Introduce `-Zmacro-stats`) - #142158 (Tracking the old name of renamed unstable library features) - #142221 ([AIX] strip underlying xcoff object) - #142340 (miri: we can use apfloat's mul_add now) - #142379 (Add bootstrap option to compile a tool with features) - #142410 (intrinsics: rename min_align_of to align_of) - #142413 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #141770 - GuillaumeGomez:cfg-false-mod-rendering, r=camelid Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code Follow-up of #141747. Thanks `@camelid` for spotting it! r? `@camelid`

Rollup of 10 pull requests Successful merges: - rust-lang/rust#134847 (Implement asymmetrical precedence for closures and jumps) - rust-lang/rust#141491 (Delegate `<CStr as Debug>` to `ByteStr`) - rust-lang/rust#141770 (Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code) - rust-lang/rust#142069 (Introduce `-Zmacro-stats`) - rust-lang/rust#142158 (Tracking the old name of renamed unstable library features) - rust-lang/rust#142221 ([AIX] strip underlying xcoff object) - rust-lang/rust#142340 (miri: we can use apfloat's mul_add now) - rust-lang/rust#142379 (Add bootstrap option to compile a tool with features) - rust-lang/rust#142410 (intrinsics: rename min_align_of to align_of) - rust-lang/rust#142413 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup

Kobzol · 2025-06-17T06:37:04Z

@rust-timer build c6e5e1c

(Trying for #142442)

rust-timer · 2025-06-17T10:43:32Z

Finished benchmarking commit (c6e5e1c): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.0%]	2
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-1.1%	[-1.1%, -1.0%]	2

Max RSS (memory usage)

Results (secondary 3.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.7%, -2.3%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 754.993s -> 690.705s (-8.52%)
Artifact size: 372.27 MiB -> 372.22 MiB (-0.01%)

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#134847 (Implement asymmetrical precedence for closures and jumps) - rust-lang#141491 (Delegate `<CStr as Debug>` to `ByteStr`) - rust-lang#141770 (Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code) - rust-lang#142069 (Introduce `-Zmacro-stats`) - rust-lang#142158 (Tracking the old name of renamed unstable library features) - rust-lang#142221 ([AIX] strip underlying xcoff object) - rust-lang#142340 (miri: we can use apfloat's mul_add now) - rust-lang#142379 (Add bootstrap option to compile a tool with features) - rust-lang#142410 (intrinsics: rename min_align_of to align_of) - rust-lang#142413 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 10 pull requests Successful merges: - rust-lang/rust#134847 (Implement asymmetrical precedence for closures and jumps) - rust-lang/rust#141491 (Delegate `<CStr as Debug>` to `ByteStr`) - rust-lang/rust#141770 (Merge `Cfg::render_long_html` and `Cfg::render_long_plain` methods common code) - rust-lang/rust#142069 (Introduce `-Zmacro-stats`) - rust-lang/rust#142158 (Tracking the old name of renamed unstable library features) - rust-lang/rust#142221 ([AIX] strip underlying xcoff object) - rust-lang/rust#142340 (miri: we can use apfloat's mul_add now) - rust-lang/rust#142379 (Add bootstrap option to compile a tool with features) - rust-lang/rust#142410 (intrinsics: rename min_align_of to align_of) - rust-lang/rust#142413 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup

* Merge Cfg::render_long_html and Cfg::render_long_plain methods …

ce0c17a

…common code * Fix invalid whitespace handling

rustbot assigned camelid May 30, 2025

GuillaumeGomez commented May 30, 2025

View reviewed changes

GuillaumeGomez force-pushed the cfg-false-mod-rendering branch from 4655783 to 358d5ea Compare May 30, 2025 13:39

GuillaumeGomez commented May 30, 2025

View reviewed changes

camelid reviewed Jun 3, 2025

View reviewed changes

tests/rustdoc/cfg-bool.rs Outdated Show resolved Hide resolved

* Add test case for cfg(false) on module level

fca28ab

* Fix typo * Remove usage of `!has`

GuillaumeGomez force-pushed the cfg-false-mod-rendering branch from 358d5ea to fca28ab Compare June 3, 2025 08:35

camelid approved these changes Jun 13, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2025

matthiaskrgr mentioned this pull request Jun 13, 2025

Rollup of 10 pull requests #142442

Merged

bors merged commit 06dc338 into rust-lang:master Jun 13, 2025
10 checks passed

rustbot added this to the 1.89.0 milestone Jun 13, 2025

GuillaumeGomez deleted the cfg-false-mod-rendering branch June 13, 2025 14:42

This comment has been minimized.

Sign in to view

Uh oh!

Conversation

GuillaumeGomez commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GuillaumeGomez commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camelid commented Jun 13, 2025

Uh oh!

bors commented Jun 13, 2025

Uh oh!

Uh oh!

Kobzol commented Jun 17, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 17, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants