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

Add tests and improve rendering of cfgs on traits #78678

Merged
merged 1 commit into from
Nov 17, 2020
Merged

Add tests and improve rendering of cfgs on traits #78678

merged 1 commit into from
Nov 17, 2020

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Nov 2, 2020

Shows the additional features required to get the trait implementation, suppressing any already shown on the current page. One interesting effect from this is if you have a cfg-ed type, implementing a cfg-ed trait (so the implementation depends on both cfgs), you will get the inverted pair of cfgs shown on each page:

image

image

The hidden items on the trait implementation also now get the correct cfgs displayed on them.

Tests are blocked on #78673.

fixes #68100
cc #43781

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(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 Nov 2, 2020
@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 2, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020 

tidy error: /checkout/src/test/rustdoc/doc-cfg-traits.rs:77: trailing whitespace
tidy error: /checkout/src/test/rustdoc/doc-cfg-traits.rs:81: trailing whitespace
tidy error: /checkout/src/test/rustdoc/doc-cfg-traits.rs:102: trailing whitespace
tidy error: /checkout/src/test/rustdoc/doc-cfg-traits.rs:106: trailing whitespace

@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

r=me with CI passing

@Nemo157
Copy link
Member Author

Nemo157 commented Nov 2, 2020

This is passing tests locally on top of #78683, I'll rebase once that's merged to get the green check.

@GuillaumeGomez
Copy link
Member

I'm not a big fan of the rendering of the feature name. It looks super weird to have a <code> after bold text.

@Nemo157
Copy link
Member Author

Nemo157 commented Nov 13, 2020

That's how features have always rendered, as <code> embedded inside the <b> of the overall cfg description, #75330 changed the wording which made it more obvious, but this PR doesn't touch it at all.

@GuillaumeGomez
Copy link
Member

Well, the PR is about improvement the rendering, so why not doing it at the same time? ;)

@Nemo157
Copy link
Member Author

Nemo157 commented Nov 13, 2020

Ah, different sort of rendering, this is about which cfg get rendered with traits, not about the actual style of how they get rendered.

@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2020
@Nemo157
Copy link
Member Author

Nemo157 commented Nov 14, 2020

Rebased.

I do have one idea about a way to make the style nicer, I'll try and open a separate PR for it soon.

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Nov 14, 2020
@GuillaumeGomez
Copy link
Member

Thanks! Please open an issue about the "rendering improvement part".

@bors: r=jyn514,GuillaumeGomez

@bors
Copy link
Contributor

bors commented Nov 15, 2020

📌 Commit c34350a has been approved by jyn514,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 Nov 15, 2020
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 15, 2020
@GuillaumeGomez
Rollup merge of rust-lang#78678 - Nemo157:doc-cfg-w-traits, r=jyn514,…
f0965d5 
…GuillaumeGomez

Add tests and improve rendering of cfgs on traits

Shows the additional features required to get the trait implementation, suppressing any already shown on the current page. One interesting effect from this is if you have a cfg-ed type, implementing a cfg-ed trait (so the implementation depends on both cfgs), you will get the inverted pair of cfgs shown on each page:

![image](https://user-images.githubusercontent.com/81079/97904671-207bdc00-1d41-11eb-8144-707e8017d2b6.png)

![image](https://user-images.githubusercontent.com/81079/97904700-27a2ea00-1d41-11eb-8b9f-e925ba339044.png)

The hidden items on the trait implementation also now get the correct cfgs displayed on them.

Tests are blocked on rust-lang#78673.

fixes rust-lang#68100
cc rust-lang#43781
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 15, 2020
Closed
@m-ou-se m-ou-se mentioned this pull request Nov 16, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2020
@bors
Auto merge of rust-lang#79104 - m-ou-se:rollup-v74492y, r=m-ou-se
c6a6105 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#74989 (Implement `Index` and `IndexMut` for arrays)
 - rust-lang#76339 (Test structural matching for all range types)
 - rust-lang#77691 (Rename/Deprecate LayoutErr in favor of LayoutError)
 - rust-lang#78364 (Update RELEASES.md for 1.48.0)
 - rust-lang#78678 (Add tests and improve rendering of cfgs on traits)
 - rust-lang#78714 (Simplify output capturing)
 - rust-lang#78769 (Remove unneeded lifetimes in array/mod.rs)
 - rust-lang#78903 (BTreeMap: test chaotic ordering & other bits & bobs)
 - rust-lang#79032 (improve type const mismatch errors)
 - rust-lang#79061 (Make all rustdoc functions and structs crate-private)
 - rust-lang#79087 (Update E0744 about control flow in `const` contexts to accurately describe when the error is triggered and why)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7a1bd80 into rust-lang:master Nov 17, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 17, 2020
@Nemo157 Nemo157 deleted the doc-cfg-w-traits branch November 17, 2020 08:39
@Nemo157 Nemo157 mentioned this pull request Nov 21, 2020
Closed
@jyn514 jyn514 added the F-doc_cfg `#![feature(doc_cfg)]` label Nov 21, 2020
@GuillaumeGomez GuillaumeGomez mentioned this pull request Oct 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
F-doc_cfg `#![feature(doc_cfg)]` 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.50.0
Development

Successfully merging this pull request may close these issues.

rustdoc: doc_cfg on trait impls needs more visibility
6 participants
@Nemo157 @rust-highfive @jyn514 @GuillaumeGomez @bors @rustbot