Skip to content

Merge Cfg::render_long_html and Cfg::render_long_plain methods common code #141770

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

Merged
merged 2 commits into from
Jun 13, 2025

Conversation

GuillaumeGomez
Copy link
Member

Follow-up of #141747.

Thanks @camelid for spotting it!

r? @camelid

…common code

* Fix invalid whitespace handling
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels May 30, 2025
let on = if self.omit_preposition() {
""
" "
Copy link
Member Author

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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 GuillaumeGomez force-pushed the cfg-false-mod-rendering branch from 4655783 to 358d5ea Compare May 30, 2025 13:39
// 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
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

* Fix typo
* Remove usage of `!has`
@GuillaumeGomez GuillaumeGomez force-pushed the cfg-false-mod-rendering branch from 358d5ea to fca28ab Compare June 3, 2025 08:35
@GuillaumeGomez
Copy link
Member Author

Fixed typo.

let on = if self.omit_preposition() {
""
" "
Copy link
Member

Choose a reason for hiding this comment

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

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.


/// 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('.');
Copy link
Member

Choose a reason for hiding this comment

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

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
Copy link
Member

camelid commented Jun 13, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

📌 Commit fca28ab has been approved by camelid

It is now in the queue for this repository.

@bors 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
bors added a commit that referenced this pull request Jun 13, 2025
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
@bors bors merged commit 06dc338 into rust-lang:master Jun 13, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 13, 2025
@GuillaumeGomez GuillaumeGomez deleted the cfg-false-mod-rendering branch June 13, 2025 14:42
rust-timer added a commit that referenced this pull request Jun 13, 2025
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`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 14, 2025
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
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jun 16, 2025
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
Copy link
Contributor

Kobzol commented Jun 17, 2025

@rust-timer build c6e5e1c

(Trying for #142442)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

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%)

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jun 18, 2025
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants