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

Add an option to allow rustdoc to list modules by appearance #46787

Merged
merged 3 commits into from
Dec 20, 2017

Conversation

varkor
Copy link
Member

@varkor varkor commented Dec 17, 2017

The --sort-modules-by-appearance option will list modules in the
order that they appear in the source, rather than sorting them
alphabetically (as is the default). This resolves #8552.

The `--sort-modules-by-appearance` option will list modules in the
order that they appear in the source, rather than sorting them
alphabetically (as is the default). This resolves rust-lang#8552.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @QuietMisdreavus (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@QuietMisdreavus
Copy link
Member

Thanks for the PR! This looks pretty good, but there's a couple things i want to poke at:

First, the tidy check failed because the optflag line was too long. This is what failed the travis check:

[00:04:12] tidy error: /checkout/src/librustdoc/lib.rs:257: line longer than 100 chars
[00:04:13] some tidy checks failed

(If you want to test this yourself before uploading any new commits, you can run x.py test src/tools/tidy which doesn't take long to run. If you've been building rustdoc locally and adding --stage 1 to your builds, you can add that here as well.)

Next, i'd like to see a test for this. How well do you know XPath? I think the ordering could be possible to express in the XPath that the rustdoc tests use, and i know that adding the flag for rustdoc is possible. The tests for rustdoc output currently live in src/test/rustdoc. and are just single-file libraries you stick in there with special comments that are read by src/etc/htmldocck.py. There's a big comment at the front of htmldocck.py that explains the test directives available.

@varkor
Copy link
Member Author

varkor commented Dec 19, 2017

I couldn't quite work out how to use XPath effectively here, but at least for the current page generation, simply using a regex worked sufficiently. I've added the test — thanks, I should have considered adding one earlier!

@QuietMisdreavus
Copy link
Member

Nice! This looks great now. Thanks for making the flag an unstable one - that'll help if we need to mess with it later. If travis checks out, i'll go ahead and r+ it!

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 19, 2017

📌 Commit b3c6102 has been approved by QuietMisdreavus

@kennytm kennytm added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 19, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Dec 20, 2017
Add an option to allow rustdoc to list modules by appearance

The `--sort-modules-by-appearance` option will list modules in the
order that they appear in the source, rather than sorting them
alphabetically (as is the default). This resolves rust-lang#8552.
bors added a commit that referenced this pull request Dec 20, 2017
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 b3c6102 into rust-lang:master Dec 20, 2017
@varkor varkor deleted the contrib-6 branch January 9, 2019 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustdoc shouldn't list modules only alphabetically
5 participants