-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Remove duplicates in rustdoc #43966
Remove duplicates in rustdoc #43966
Conversation
This really needs to be done when inlining instead. We need to not inline things from globs if they are shadowed.
You can use Lines 99 to 100 in 80be2f8
|
src/librustdoc/html/render.rs
Outdated
Some(full_path(cx, &items[*i]).clone()) | ||
} else { | ||
items[*i].name.clone() | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this else branch will return anything but None? Also was there any major reason that is_some
needed the as_ref
there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid a panic mainly. But I could directly return None
, indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does something like items.get(*i).map(|n| full_path(cx, &items[*i]))
work here?
nevermind, this is not equivalent
@ollie27: Nice, thanks! |
9c2bf4f
to
e770c09
Compare
(the rest of the travis run is stuck on mac builders) |
e770c09
to
5d71280
Compare
Fixed the tidy issue (deprecating TODO is strange). |
I'm willing to land this as-is. If we want to change how this deduplication happens later, we only need to take out one statement to revert this change. (Plus, we have a test for this now. @bors r+ rollup |
📌 Commit 5d71280 has been approved by |
…isdreavus Remove duplicates in rustdoc Fixes rust-lang#43934. Two things however: 1. I'm not happy with the current check. It seems completely overkill and unsatisfying. 2. I have no idea how to test if there is only one element and not two. r? @rust-lang/docs
Fixes #43934.
Two things however:
r? @rust-lang/docs