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 readme for librustdoc #48283

Merged
merged 2 commits into from
Mar 3, 2018
Merged

Conversation

QuietMisdreavus
Copy link
Member

In the same vein as the other compiler-library readmes, here's one for rustdoc! It's mainly a "how does rustdoc even" blog-post-style writeup, but i wanted to have something in-repo so people could get a sense of what bits did what.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2018
@QuietMisdreavus
Copy link
Member Author

@GuillaumeGomez @steveklabnik Did i miss anything glaringly obvious here? This file kinda sat half-written on my server for a few months, so at some point i just wanted to get something written up so we could have it online.

@mark-i-m
Copy link
Member

@QuietMisdreavus Could you add/adapt the same for rustc-guide?

Also, I'm guessing this is for current rustdoc, not https://github.com/steveklabnik/rustdoc?

@QuietMisdreavus
Copy link
Member Author

@mark-i-m Yeah, i can add this when it gets approved up here. Don't want to go through the same review nits more than once, haha.

Also yes, since the readme is sitting in src/librustdoc, it's describing that code. I wanted it to be like the README.md in src/librustc, where it can be seen alongside the code when browsing on GitHub.

@GuillaumeGomez
Copy link
Member

Sounds good to me but I'd prefer an english native speaker to give a look before r+.

Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. I left a few comments about minor nits...

CSS/JS and landing page are.
* Most of the HTML printing code is in `html/format.rs` and `html/render.rs`. It's in a bunch of
`fmt::Display` implementations and supplementary functions.
* The types that operates on are defined in `clean/mod.rs`, right next to the custom `Clean` trait
Copy link
Member

Choose a reason for hiding this comment

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

"The types that operates on"

Copy link
Member Author

Choose a reason for hiding this comment

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

How about:

The types that got Display impls above are defined in...

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Yeah, that's a lot better.

line in the module page.

Back in `clean/mod.rs`, the other major thing that happens here is the special collection of doc
comments (and basic `#[doc=""]` attributes) into a separate field in the `Attributes` struct from
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is a bit hard to follow.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about:

The other major thing that happens in clean/mod.rs is the collection of doc comments and #[doc=""] attributes into a separate field of the Attributes struct, present on anything that gets hand-written documentation. This makes it easier to collect this documentation later in the process.

The primary output of this process is a clean::Crate with a tree of Items which describe the publicly-documentable items in the target crate.

These statements weren't that related anyway, so breaking that last one out into its own paragraph makes that explicit.

whitespace to make the document easier on the markdown parser, or drop items that are not public or
deliberately hidden with `#[doc(hidden)]`. These are all implemented in the `passes/` directory, one
file per pass. By default, all of these passes are run on a crate, but the ones regarding dropping
private/hidden items can be bypassed by passing `--document-private-items` to rustdoc.
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to note where the passes are called from so that adding a new pass is easier...

Copy link
Member Author

Choose a reason for hiding this comment

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

I avoided mentioning the passes explicitly, since we kinda wanted to abandon the concept, but i can leave a note. It's awkward and wrapped up in the plugin interface that we've explicitly deprecated.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see... In that case, maybe it would be better to not document that so it is harder to add another pass.

Copy link
Member Author

Choose a reason for hiding this comment

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

In other words, take out this section entirely? The passes do some fairly important things, so i would be remiss to leave it out. The entry to the passes isn't necessarily difficult, just awkward. Technically each "pass" is a plugin, loaded up at the same time as any dynamically-loaded plugin that could potentially have been given.

rust/src/librustdoc/lib.rs

Lines 583 to 612 in 27a046e

if default_passes {
for name in passes::DEFAULT_PASSES.iter().rev() {
passes.insert(0, name.to_string());
}
}
// Load all plugins/passes into a PluginManager
let path = plugin_path.unwrap_or("/tmp/rustdoc/plugins".to_string());
let mut pm = plugins::PluginManager::new(PathBuf::from(path));
for pass in &passes {
let plugin = match passes::PASSES.iter()
.position(|&(p, ..)| {
p == *pass
}) {
Some(i) => passes::PASSES[i].1,
None => {
error!("unknown pass {}, skipping", *pass);
continue
},
};
pm.add_plugin(plugin);
}
info!("loading plugins...");
for pname in plugins {
pm.load_plugin(pname);
}
// Run everything!
info!("Executing passes/plugins");
let krate = pm.run_plugins(krate);

Copy link
Member

Choose a reason for hiding this comment

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

No, I think we can just leave it as is. Those who need to deal with it can ask for further clarification when they cross that bridge.

@mark-i-m
Copy link
Member

@GuillaumeGomez r=english-speaker=me

@shepmaster
Copy link
Member

Everyone likes progress, yeah?

@bors r=@GuillaumeGomez

@bors
Copy link
Contributor

bors commented Mar 2, 2018

📌 Commit 8d893c1 has been approved by @GuillaumeGomez

@bors bors 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 Mar 2, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 3, 2018
…GuillaumeGomez

add readme for librustdoc

In the same vein as the other compiler-library readmes, here's one for rustdoc! It's mainly a "how does rustdoc even" blog-post-style writeup, but i wanted to have something in-repo so people could get a sense of what bits did what.
kennytm added a commit to kennytm/rust that referenced this pull request Mar 3, 2018
…GuillaumeGomez

add readme for librustdoc

In the same vein as the other compiler-library readmes, here's one for rustdoc! It's mainly a "how does rustdoc even" blog-post-style writeup, but i wanted to have something in-repo so people could get a sense of what bits did what.
bors added a commit that referenced this pull request Mar 3, 2018
Rollup of 8 pull requests

- Successful merges: #48283, #48466, #48569, #48629, #48637, #48680, #48513, #48664
- Failed merges:
@bors bors merged commit 8d893c1 into rust-lang:master Mar 3, 2018
@mark-i-m
Copy link
Member

mark-i-m commented Mar 4, 2018

🎉 perhaps it's time to add it to the rustc guide?

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.

7 participants