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

Do not display hidden types, fixes issue 23912 #46359

Merged
merged 1 commit into from
Dec 20, 2017
Merged

Do not display hidden types, fixes issue 23912 #46359

merged 1 commit into from
Dec 20, 2017

Conversation

GuillaumeGomez
Copy link
Member

Fixes #23912.

r? @QuietMisdreavus

(It's the one I was talking about a few days ago, just close it if it's useless.)

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2017
@QuietMisdreavus
Copy link
Member

It looks like would just hide the [src] link? The rest of the impl will still be visible if the item had no visibility set. And the impl will not even get there in the first place if the for_ type is in a #[doc(masked)] crate anyway. The issue was actually fixed by #44026, and i'll close it out. It seems the issue i actually referenced/closed in that PR was a duplicate of that one. >_>

On the other hand, the test you have here is useful, but i think it would be better if the trait that was #[doc(hidden)] were public as well, since it would be removed from docs anyway for being a private item.

ollie27
ollie27 reviewed Nov 29, 2017
View reviewed changes
Copy link
Member

@ollie27 ollie27 left a comment

Choose a reason for hiding this comment

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

You have to be very careful when writing @!has tests because typos mean the test passes without testing anything 😉.

src/test/rustdoc/hidden-trait-struct-impls.rs Outdated
// @has foo/struct.Bar.html '//*[@id="impl-Bam"]' 'impl Bam for Bar'
// @has foo/trait.Bam.html '//*[@id="implementors-list"]' 'impl Bam for Bar'
impl Bam for Bar {}
// @!has foo/struct.Bar.html '//*[@id="iimplementors-list"]' 'impl Bam for Hidden'
Copy link
Member

Choose a reason for hiding this comment

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

struct.Bar.html -> trait.Bam.html

iimplementors-list -> implementors-list

src/test/rustdoc/hidden-trait-struct-impls.rs Outdated

struct Hidden;

// @!has foo/struct.Bar.html '//*[@id="impl-Bar"]' 'impl Foo for Bar'
Copy link
Member

Choose a reason for hiding this comment

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

impl-Bar -> impl-Foo

@GuillaumeGomez
Copy link
Member Author

@ollie27: Arf, you're right! Thanks!

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2017
@kennytm
Copy link
Member

kennytm commented Dec 6, 2017

@GuillaumeGomez @QuietMisdreavus Is this PR still needed? #23912 has been closed by #44026.

@QuietMisdreavus
Copy link
Member

@kennytm I want it for the test test it adds. If @GuillaumeGomez can take out the change in render.rs and address the review comments on the test, i'll add it.

@carols10cents
Copy link
Member

Friendly ping to keep this on your radar @GuillaumeGomez!

@GuillaumeGomez
Copy link
Member Author

Going back on it this week-end.

@GuillaumeGomez
Copy link
Member Author

Updated.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 17, 2017
QuietMisdreavus
QuietMisdreavus reviewed Dec 18, 2017
View reviewed changes
src/test/rustdoc/hidden-trait-struct-impls.rs Outdated
#![crate_name = "foo"]

#[doc(hidden)]
trait Foo {}
Copy link
Member

Choose a reason for hiding this comment

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

Tagging a private item with #[doc(hidden)] has no effect when you're not changing which passes are run. Can you make this pub trait Foo instead?

@GuillaumeGomez
Copy link
Member Author

Updated.

@QuietMisdreavus
Copy link
Member

r=me pending travis

This is... kinda unrelated to #23912 now, but as i said, the test is still useful.

@QuietMisdreavus
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 19, 2017

📌 Commit 03530ee has been approved by QuietMisdreavus

@kennytm kennytm 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 Dec 19, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Dec 20, 2017
@kennytm
Rollup merge of rust-lang#46359 - GuillaumeGomez:remove-dead-linkage,…
8de8196 
… r=QuietMisdreavus

Do not display hidden types, fixes issue 23912

Fixes rust-lang#23912.

r? @QuietMisdreavus

(It's the one I was talking about a few days ago, just close it if it's useless.)
@kennytm kennytm mentioned this pull request Dec 20, 2017
Merged
bors added a commit that referenced this pull request Dec 20, 2017
@bors
Auto merge of #46874 - kennytm:rollup, r=kennytm
81622c6 
Rollup of 14 pull requests

- Successful merges: #46359, #46517, #46671, #46751, #46760, #46787, #46794, #46828, #46831, #46835, #46851, #46852, #46856, #46870
- Failed merges:
@bors bors merged commit 03530ee into rust-lang:master Dec 20, 2017
@GuillaumeGomez GuillaumeGomez deleted the remove-dead-linkage branch December 21, 2017 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ollie27 ollie27 ollie27 left review comments

@QuietMisdreavus QuietMisdreavus QuietMisdreavus left review comments

Assignees

@QuietMisdreavus QuietMisdreavus

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc links to private structs, causing 404s
6 participants
@GuillaumeGomez @QuietMisdreavus @kennytm @carols10cents @bors @ollie27