Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Associated consts sidebar #89815

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Associated consts sidebar #89815

merged 3 commits into from
Oct 14, 2021

Conversation

GuillaumeGomez
Copy link
Member

Fixes #89354.

A screenshot with f32:

Screenshot from 2021-10-12 15-07-57

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Oct 12, 2021
@rust-highfive
Copy link
Collaborator

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2021
@GuillaumeGomez
Copy link
Member Author

r? @notriddle

@GuillaumeGomez
Copy link
Member Author

Updated! Thanks again for the great suggestion @notriddle!

Since I suspect it'll impact performance (positively), let's prevent it from being into a rollup:

@bors: rollup=iffy

notriddle
notriddle reviewed Oct 13, 2021
View reviewed changes
src/librustdoc/html/render/mod.rs Outdated
@@ -1928,7 +1960,7 @@ fn sidebar_assoc_items(cx: &Context<'_>, out: &mut Buffer, it: &clean::Item) {
<div class=\"sidebar-links\">",
);
for line in methods {
out.push_str(&line);
out.push_str(&line.to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
out.push_str(&line.to_string());
write!(&mut out, line);

https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/format.rs#L94

@GuillaumeGomez GuillaumeGomez force-pushed the associated-consts-sidebar branch from a40447c to 6dc0b06 Compare October 13, 2021 15:39
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the associated-consts-sidebar branch from 6dc0b06 to 44a185e Compare October 13, 2021 15:46
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@GuillaumeGomez GuillaumeGomez force-pushed the associated-consts-sidebar branch from 44a185e to 641eeab Compare October 13, 2021 15:59
notriddle
notriddle approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@notriddle notriddle left a comment

Choose a reason for hiding this comment

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

Fix that one last nit, and r=me.

@GuillaumeGomez GuillaumeGomez force-pushed the associated-consts-sidebar branch from 641eeab to 38f6c07 Compare October 13, 2021 19:59
@GuillaumeGomez
Copy link
Member Author

Thanks a lot for your suggestions @notriddle !

@bors: r=notriddle

@bors
Copy link
Contributor

bors commented Oct 13, 2021

📌 Commit 38f6c07 has been approved by notriddle

@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 Oct 13, 2021
@bors
Copy link
Contributor

bors commented Oct 14, 2021

⌛ Testing commit 38f6c07 with merge 7807a69...

@bors
Copy link
Contributor

bors commented Oct 14, 2021

☀️ Test successful - checks-actions
Approved by: notriddle
Pushing 7807a69 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 14, 2021
@bors bors merged commit 7807a69 into rust-lang:master Oct 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 14, 2021
@GuillaumeGomez GuillaumeGomez deleted the associated-consts-sidebar branch October 14, 2021 08:07
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7807a69): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@GuillaumeGomez
Copy link
Member Author

Seems like performance are a little bit worse (+0.03%).

@fmease fmease mentioned this pull request Mar 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notriddle notriddle notriddle approved these changes

Assignees
No one assigned
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) merged-by-bors This PR was explicitly merged by bors. 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.
Projects
None yet
Milestone
1.57.0
Development

Successfully merging this pull request may close these issues.

Rustdoc does not list associated constants for types in the sidebar
8 participants
@GuillaumeGomez @rust-highfive @rust-log-analyzer @bors @rust-timer @notriddle @ollie27 @rustbot