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

Hide windows-sys's documentation behind a feature flag? #2665

Closed
LikeLakers2 opened this issue Sep 21, 2023 · 15 comments · Fixed by #2676
Closed

Hide windows-sys's documentation behind a feature flag? #2665

LikeLakers2 opened this issue Sep 21, 2023 · 15 comments · Fixed by #2676
Labels
enhancement New feature or request

Comments

@LikeLakers2
Copy link

LikeLakers2 commented Sep 21, 2023

Suggestion

Hi! I've been building a program that uses the bevy game engine crate, and in the midst of this, I've found it useful to render documentation for everything via cargo doc.

However, I noticed that the windows-sys crate generates quite a lot of documentation. Unfortunately, cargo doc has no way to exclude certain dependencies without excluding all dependencies (the --exclude flag only seems to work on packages that are within the current workspace).

I don't believe this is an issue with the windows-sys crate, per se - it's doing its job just fine. However, unless a user is using windows-sys directly, they may not need to see windows-sys's documentation anyways - meaning windows-sys's documentation is simply wasting disk space, and worse, making cargo clean take a lot longer due to the mass amount of files generated.

Still, some users may need windows-sys's docs, so hiding them behind a blanket #[doc(hidden)] may not be a good idea. Perhaps windows-sys could attach #[doc(hidden)] to most stuff, unless a certain feature flag (let's say windows-sys/render_docs) is specified?

I just think it may be useful to avoid generating so much documentation unless a user needs it.

@LikeLakers2 LikeLakers2 added the enhancement New feature or request label Sep 21, 2023
@LikeLakers2 LikeLakers2 changed the title windows-sys generates a large amount of documentation that many folks won't need windows-sys generates a lot of documentation, with no way to hide it via cargo doc Sep 21, 2023
@LikeLakers2 LikeLakers2 changed the title windows-sys generates a lot of documentation, with no way to hide it via cargo doc Hide windows-sys's documentation behind a feature flag? Sep 21, 2023
@LikeLakers2 LikeLakers2 changed the title Hide windows-sys's documentation behind a feature flag? Hide windows's and windows-sys's documentation behind a feature flag? Sep 21, 2023
@LikeLakers2 LikeLakers2 changed the title Hide windows's and windows-sys's documentation behind a feature flag? Hide windows-sys's documentation behind a feature flag? Sep 21, 2023
@riverar
Copy link
Collaborator

riverar commented Sep 23, 2023

So you'd like to generate docs for your crate and all dependencies except windows-sys, right? (Just making sure I understand, thanks!)

@LikeLakers2
Copy link
Author

LikeLakers2 commented Sep 23, 2023

Yes. windows-sys generates a lot of documentation, and because I'm using the bevy game engine crate, it abstracts away much of the stuff needed to make a program - so I have no need to look at the windows-sys documentation ever. So I'd like a way to avoid generating windows-sys documentation.

There is rust-lang/cargo#4049, however that was opened in 2017 with no plans to add it yet. Thus, as a stopgap, I'm hoping windows-sys can provide a method to skip generating so much documentation.

@kennykerr
Copy link
Collaborator

Hey, sorry for the delay. Been off the grid camping. 🏕️

Just catching up here but this is what I do. Is that sufficient?

cargo doc -p package --all-features --no-deps

@kennykerr kennykerr added question Further information is requested and removed enhancement New feature or request labels Sep 27, 2023
@LikeLakers2
Copy link
Author

LikeLakers2 commented Sep 27, 2023

@kennykerr While it could work, there's some problems I'd have with using that approach.

When I'm building apps with bevy, there's often a large number of crates named bevy_* that I'm interested in. Combine this with other dependencies that I've added to the app, and now I have a rather large number of -p parameters to specify just to exclude windows-sys from being documented.

Additionally, I use cargo-watch in order to automatically regenerate documentation as I write my app. Currently, I have cargo watch --exec doc as a VSCode task that I (manually) run whenever I open a project - and then documentation generation is taken care of for me for as long as VSCode is open. However, having to specify all the packages I'd want would result in me needing to write one cargo-watch command per project, not to mention having to keep it up-to-date when dependencies change.

So suffice to say, I'm not exactly keen on that approach.

@kennykerr
Copy link
Collaborator

Fair enough. I've been meaning to reduce the overhead of doc comments for other reasons, so I'll keep this in mind for that effort.

@kennykerr kennykerr added enhancement New feature or request and removed question Further information is requested labels Sep 28, 2023
@kennykerr
Copy link
Collaborator

Mainly just for me: if we add #![doc(hidden)] to the crate's lib.rs file then all docs are effectively removed, rather than having to add the attribute to each and every item individually.

@LikeLakers2
Copy link
Author

LikeLakers2 commented Sep 28, 2023

@kennykerr If it isn't too much effort, you may still want to leave a note (whether as a cargo warning when executing cargo doc, or in the crate root) about why there's no documentation, so as to avoid confusion. For example, "Documentation is disabled. To view documentation, pass --features "windows-sys/render_docs" to cargo." or similar. That way, folks aren't confused when the documentation suddenly isn't visible.

@riverar
Copy link
Collaborator

riverar commented Sep 28, 2023

Not a fan of tying docs to a feature. It's not part of the standard cargo doc experience and folks will overlook it causing confusion. We should really try to lean on rust-lang/cargo#4049 more.

@LikeLakers2
Copy link
Author

LikeLakers2 commented Sep 28, 2023

@riverar I agree, and I would use rust-lang/cargo#4049 if it were already added - but given that it's been six years since that issue was opened, and there doesn't appear to be many comments, I hesitate to think that it'd be added soon.

So for the interim (or in lieu of that arg, if we assume it'll never be added), I would hope to have some way to exclude windows-sys - and a feature flag would be a good way to do it.

@kennykerr
Copy link
Collaborator

Sure, I wouldn't exclude docs automatically. If provided, that would be something that you would have to opt in to.

@kennykerr
Copy link
Collaborator

There are also cfg options.

https://github.com/microsoft/windows-rs/blob/master/.cargo/config.toml

@kennykerr
Copy link
Collaborator

Features are meant to be additive so that doesn't seem like a natural fit. I experimented with a cfg option but rustdoc seems to fail to handle cfg_attr correctly.

// lib.rs
#![cfg_attr(windows_doc_hidden, doc(hidden))]

This would seem like the simplest and most natural way to skip doc generation for the crate, if it worked.

@LikeLakers2
Copy link
Author

LikeLakers2 commented Oct 3, 2023

@kennykerr I'm confused by that response.

How I understand such a feature flag is that documentation would be hidden by default - and specifying the feature unhides it. Thus, the feature is additive in the sense that it adds documentation.

I'm also confused by your proposed alternative - it seems to remove documentation if a flag is specified, which is most definitely not additive.

So could you perhaps elaborate on how you see the issue? Perhaps explain why you feel the feature flag would be non-additive?

@kennykerr
Copy link
Collaborator

I was assuming the docs feature would need to be enabled by default, but I don't suppose that is necessary.

@LikeLakers2
Copy link
Author

LikeLakers2 commented Oct 3, 2023

Ah, okay that makes sense.

Yeah, my thought is that the vast majority of the time, windows-sys is being abstracted away. For example, tempfile uses windows-sys, but the user never needs to know of windows-sys's existance when using tempfile - because tempfile abstracts that away.

Thus, the vast majority of Rust crates will have little need for the windows-sys docs during development. This means that documentation could be disabled by default - only being enabled when someone actually needs the documentation, using a feature flag passed to cargo doc or cargo-watch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants