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

[rustdoc] Wrap code blocks in <code> tag #88093

Merged
merged 1 commit into from
Aug 19, 2021
Merged

[rustdoc] Wrap code blocks in <code> tag #88093

merged 1 commit into from
Aug 19, 2021

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Aug 16, 2021

This PR modifies Rustdoc output so that fenced code snippets, items and whole file source codes are wrapped in <pre><code> instead of just <pre>. This should improve the semantic meaning of the generated content.

I'm not sure what to do about render_attributes_in_pre and render_attributes_in_code. These functions were clearly expected to be used for things inside <pre> or <code>, and since I added <code> in this PR, some of them will be used in a different context than before. However, it seems to me that even before they were not consistent. For example, item_constant used render_attributes_in_code for its attributes, however there was no <code> used for constants before this PR...

Should I create some rustdoc-gui tests? For example to check that all <pre> tags have a <code> child?

Fixes: #88020

@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 Aug 16, 2021
@GuillaumeGomez GuillaumeGomez added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 18, 2021
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Aug 18, 2021 •
edited
Loading

I'm not sure what to do about render_attributes_in_pre and render_attributes_in_code. These functions were clearly expected to be used for things inside <pre> or <code>, and since I added <code> in this PR, some of them will be used in a different context than before. However, it seems to me that even before they were not consistent. For example, item_constant used render_attributes_in_code for its attributes, however there was no <code> used for constants before this PR...

Another shortcoming of rustdoc... Welcome to a huge years old codebase! :)

For consistency it should be applied the same on equivalent items but I think it's a bit out of scope here. For things other than codeblocks, just having a <code> (without the <pre> wrapping) is fine.

Should I create some rustdoc-gui tests? For example to check that all <pre> tags have a <code> child?

Well, not all <code> are supposed to be wrapped. For example, an inline code in documentation like this one isn't supposed to be wrapped into a <pre>.

But enforcing the existing is always a good idea and such a test would be very welcome. :)

@jyn514
Copy link
Member

jyn514 commented Aug 19, 2021

r? @GuillaumeGomez

@Kobzol Kobzol force-pushed the rustdoc-wrap-code-in-code-tag branch from 0c3f394 to 378d92c Compare August 19, 2021 07:48
@Kobzol
Copy link
Contributor Author

Kobzol commented Aug 19, 2021

I changed the newlines after <pre><code>, now no newline is generated. I also added a simple test that checks that a few item types and doc code blocks contain <pre><code>.

@Kobzol Kobzol force-pushed the rustdoc-wrap-code-in-code-tag branch from 378d92c to ccd550e Compare August 19, 2021 08:30
@GuillaumeGomez
Copy link
Member

I checked locally and it looks good to me.

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 19, 2021

📌 Commit ccd550e has been approved by GuillaumeGomez

@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 Aug 19, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 19, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2021
@bors
Auto merge of rust-lang#88165 - GuillaumeGomez:rollup-4o0v2ps, r=Guil…
6d64f7f 
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#86123 (Preserve more spans in internal `rustc_queries!` macro)
 - rust-lang#87874 (Add TcpStream type to TcpListener::incoming docs)
 - rust-lang#88034 (rustc_privacy: Replace `HirId`s and `DefId`s with `LocalDefId`s where possible)
 - rust-lang#88050 (Remove `HashStable` impls for `FileName` and `RealFileName`)
 - rust-lang#88093 ([rustdoc] Wrap code blocks in `<code>` tag)
 - rust-lang#88146 (Add tests for some `feature(const_evaluatable_checked)` incr comp issues)
 - rust-lang#88153 (Update .mailmap)
 - rust-lang#88159 (Use a trait instead of the now disallowed missing trait there)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 10165f8 into rust-lang:master Aug 19, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 19, 2021
@Kobzol Kobzol deleted the rustdoc-wrap-code-in-code-tag branch August 20, 2021 06:11
@camelid camelid mentioned this pull request Aug 28, 2021
Closed
@gemmaro gemmaro mentioned this pull request Nov 14, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) 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.56.0
Development

Successfully merging this pull request may close these issues.

rustdoc, when I use google translate, that translate the doc as well as the code.
7 participants
@Kobzol @rust-highfive @GuillaumeGomez @jyn514 @bors @ollie27 @rustbot