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.

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

Rustdoc: ignore deref-inherited static methods #24657

Merged
merged 1 commit into from
May 26, 2015
Merged

Rustdoc: ignore deref-inherited static methods #24657

merged 1 commit into from
May 26, 2015

Conversation

aochagavia
Copy link
Contributor

Fixes #24575
Fixes #25673

r? @alexcrichton

@aochagavia
Copy link
Contributor Author

I am not sure on how to test this. At this moment I only know that it compiles...

@alexcrichton
Copy link
Member

Could you modify the existing tests for #19190 in conjunction with the @count directive to ensure that none of these show up in the various cases tested by #19190? You should be able to just modify the contents of the src/test/rustdoc folder.

Also, could you upload a copy of the docs so they could be manually verified as well?

@aochagavia
Copy link
Contributor Author

It seems that my changes aren't entirely correct. Hiding static methods works if the target type is a primitive (in this case i32), but doesn't if it is user-defined. I have checked the logic of the code (mainly the following functions: build_deref_target_impls, build_deref_impls, build_impls, build_impls_helper, build_impl) and I am unable to find the problem. Are there any other functions which could interact with this feature?

btw, I used @!has instead of @count to check that the static methods aren't shown. Is that ok?

@alexcrichton
Copy link
Member

I think you may want to modify the rendering function instead of the inlining functions as local Deref target impl blocks are not inlined (they're already in the AST) so they're not pruned already.

Also yeah using @!has is fine, I didn't even know it existed!

@aochagavia
Copy link
Contributor Author

Thanks for your feedback! I have updated the PR and uploaded the documentation to http://aochagavia.github.io/rust/index.html

@alexcrichton
Copy link
Member

@bors: r+ 2c79fb5

Thanks!

@alexcrichton
Copy link
Member

@bors: r-

er actually, could you squash the commits and a more descriptive commit message about this fix? Unfortunately Fix #XXXXX isn't too descriptive :(

@bors
Copy link
Contributor

bors commented Apr 27, 2015

☔ The latest upstream changes (presumably #23606) made this pull request unmergeable. Please resolve the merge conflicts.

@sfackler
Copy link
Member

Ping?

@aochagavia
Copy link
Contributor Author

Sorry, I won't have time to resolve the merge conflicts and test the patch again in the following weeks. Feel free to pick it up, though.

@bluss
Copy link
Member

bluss commented May 23, 2015

There's another tricky case, all types that deref to slice also show the method fn into_vec(self: Box<[T]>) -> Vec<T>. Is it possible to filter out that as well?

@aochagavia
Copy link
Contributor Author

Finally I found time to rebase this. I think it would be better to create an issue for the case @bluss pointed out, since I will not be able to tackle it anytime soon.

@alexcrichton r+?

@bluss
Copy link
Member

bluss commented May 25, 2015

that's fine, I'm happy with every improvement to rustdoc, so thanks!

@bluss
Copy link
Member

bluss commented May 25, 2015

Also fixes dup #25673

@alexcrichton
Copy link
Member

@bors: r+ 8703883

Thanks!

@bors
Copy link
Contributor

bors commented May 26, 2015

⌛ Testing commit 8703883 with merge b7408c1...

@bors
Copy link
Contributor

bors commented May 26, 2015

💔 Test failed - auto-linux-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Tue, May 26, 2015 at 6:58 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-64-opt
http://buildbot.rust-lang.org/builders/auto-linux-64-opt/builds/5088


Reply to this email directly or view it on GitHub
#24657 (comment).

@bors
Copy link
Contributor

bors commented May 26, 2015

⌛ Testing commit 8703883 with merge 1742a01...

bors added a commit that referenced this pull request May 26, 2015
@bors bors merged commit 8703883 into rust-lang:master May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants