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

Remove duplicates in rustdoc #43966

Merged
merged 2 commits into from
Aug 26, 2017
Merged

Remove duplicates in rustdoc #43966

merged 2 commits into from
Aug 26, 2017

Conversation

GuillaumeGomez
Copy link
Member

Fixes #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

@ollie27
Copy link
Member

ollie27 commented Aug 21, 2017

This really needs to be done when inlining instead. We need to not inline things from globs if they are shadowed.

I have no idea how to test if there is only one element and not two.

You can use @count in rustdoc tests.

rust/src/etc/htmldocck.py

Lines 99 to 100 in 80be2f8

* `@count PATH XPATH COUNT' checks for the occurrence of given XPath
in the given file. The number of occurrences must match the given count.

Some(full_path(cx, &items[*i]).clone())
} else {
items[*i].name.clone()
},
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

@frewsxcv frewsxcv Aug 23, 2017

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

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 22, 2017
@GuillaumeGomez
Copy link
Member Author

@ollie27: Nice, thanks!

@QuietMisdreavus
Copy link
Member

[00:03:07] tidy error: /checkout/src/librustdoc/html/render.rs:1782: TODO is deprecated; use FIXME
[00:03:08] some tidy checks failed

(the rest of the travis run is stuck on mac builders)

@GuillaumeGomez
Copy link
Member Author

Fixed the tidy issue (deprecating TODO is strange).

@QuietMisdreavus
Copy link
Member

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. :3)

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 24, 2017

📌 Commit 5d71280 has been approved by QuietMisdreavus

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Aug 26, 2017
…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
bors added a commit that referenced this pull request Aug 26, 2017
Rollup of 7 pull requests

- Successful merges: #43776, #43966, #43979, #44072, #44086, #44090, #44091
- Failed merges:
@bors bors merged commit 5d71280 into rust-lang:master Aug 26, 2017
@GuillaumeGomez GuillaumeGomez deleted the remove-dup branch August 26, 2017 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants