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

Use doc_auto_cfg feature instead of ad-hoc #[doc] comments. #1965

Closed
wants to merge 1 commit into from

Conversation

glandium
Copy link
Contributor

docs.rs uses the nightly compiler, allowing to enable the doc_auto_cfg,
which automatically documents the features required for each documented
item.

Before:
$ du -sh crates/libs/{sys,windows}
32M crates/libs/sys
178M crates/libs/windows

After:
$ du -sh crates/libs/{sys,windows}
21M crates/libs/sys
164M crates/libs/windows

While here, add the missing all-features setting for docs.rs for the
windows crate.

@glandium
Copy link
Contributor Author

Waw, peak rustdoc memory usage for the windows crate is 11.5G... (and a very large runtime)

docs.rs uses the nightly compiler, allowing to enable the doc_auto_cfg,
which automatically documents the features required for each documented
item.

Before:
$ du -sh crates/libs/{sys,windows}
32M	crates/libs/sys
178M	crates/libs/windows

After:
$ du -sh crates/libs/{sys,windows}
21M	crates/libs/sys
164M	crates/libs/windows
@rylev
Copy link
Contributor

rylev commented Aug 18, 2022

If the final render seen on docs.rs is exactly equivalent before and after this change, then I think it's obvious to accept this change, but I doubt that's the case. Can you take a look at the output when running cargo doc and summarize the differences?

@glandium
Copy link
Contributor Author

Before:
Screenshot 2022-08-19 at 05-06-06 windows_sys Win32 - Rust
Screenshot 2022-08-19 at 05-06-28 windows_sys Win32 Foundation - Rust
Screenshot 2022-08-19 at 05-09-06 POINT in windows_sys Win32 Foundation - Rust
After:
Screenshot 2022-08-19 at 05-12-09 windows_sys Win32 - Rust
Screenshot 2022-08-19 at 05-12-34 windows_sys Win32 Foundation - Rust
Screenshot 2022-08-19 at 05-12-56 POINT in windows_sys Win32 Foundation - Rust

The main difference is the "Required features" not being shown on each item in the module, but that's because they are the same as the one listed for the module. In a module where extra features are required, they're shown:
Screenshot 2022-08-19 at 05-16-13 windows_sys Win32 Networking NetworkListManager - Rust

I'd argue the new output is better.

@riverar
Copy link
Collaborator

riverar commented Aug 18, 2022

This looks worse in every way possible. It's a distracting color, verbose due to conjunction (and) use, and can't be cut/pasted into a Cargo.toml. Can those issues be resolved? If not, I don't think they'll accept this change.

@glandium
Copy link
Contributor Author

It's also what's used in the rust ecosystem: https://docs.rs/tokio/1.20.1/tokio/macro.join.html

@glandium
Copy link
Contributor Author

Can those issues be resolved?

Issues can be filed against rustdoc.

@riverar
Copy link
Collaborator

riverar commented Aug 18, 2022

Will leave this open until Kenny returns but it's not likely to get merged in. Would recommend holding on any further commits.

@kennykerr
Copy link
Collaborator

Thanks for the contribution! Sorry I've been away for a few weeks. In future, please open an issue before creating a PR. We have discussed this before at length. The rustdoc feature support is very limited in many ways and lacks the support we need to provide high-quality feature documentation. While I would also prefer to use the built-in support, the rustdoc team hasn't made any progress on the various issues we encountered so I'm unlikely to revert the custom doc generation. I can try to dig up the old issues where this was discussed if you'd like to discuss this further.

@kennykerr kennykerr closed this Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants