-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Merge Cfg::render_long_html
and Cfg::render_long_plain
methods common code
#141770
Conversation
…common code * Fix invalid whitespace handling
let on = if self.omit_preposition() { | ||
"" | ||
" " |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
4655783
to
358d5ea
Compare
// 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 |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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`
358d5ea
to
fca28ab
Compare
Fixed typo. |
let on = if self.omit_preposition() { | ||
"" | ||
" " |
There was a problem hiding this comment.
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('.'); |
There was a problem hiding this comment.
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?
@bors r+ rollup |
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 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
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
@rust-timer build c6e5e1c (Trying for #142442) |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (c6e5e1c): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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 Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
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.
CyclesResults (secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 754.993s -> 690.705s (-8.52%) |
…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
Follow-up of #141747.
Thanks @camelid for spotting it!
r? @camelid