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

Document feature requirements #1264

Closed
wants to merge 1 commit into from
Closed

Document feature requirements #1264

wants to merge 1 commit into from

Conversation

kennykerr
Copy link
Collaborator

Fixes #1246

@jyn514 It would be great if it could detect when a feature enables other features and only show the smallest set of features. For example, here Win32_Storage_CloudFilters enables Win32_Storage which enables Win32 so it would ideally elide those for brevity and only list the features that must be enabled to actually enable the function. From the Windows crate's Cargo.toml:

Win32_Storage = ["Win32"]
Win32_Storage_CloudFilters = ["Win32_Storage"]

This is because it is modeling a module hierarchy.

image

@kennykerr
Copy link
Collaborator Author

Ah, this is not yet stable so I may opt for a different approach for now.

@kennykerr kennykerr closed this Oct 30, 2021
@jyn514
Copy link

jyn514 commented Oct 30, 2021

would ideally elide those for brevity and only list the features that must be enabled to actually enable the function

This would need major design changes, rustdoc doesn't have knowledge of cargo and doesn't know which features enable other features.

@kennykerr
Copy link
Collaborator Author

Fair enough, then I may just generate a simple list with #[doc = "..."]

@MarijnS95
Copy link
Contributor

MarijnS95 commented Nov 1, 2021

Ah, this is not yet stable so I may opt for a different approach for now.

Fwiw other projects introduce something like a dox feature or docsrs cfg that is enabled when building for docs.rs (1 / 2), which uses nightly (and you can enable it manually locally and on the CI if buildtesting the docs with nightly).

Secondly you can override the features shown here with #[doc(cfg(features = "Win32_Storage_CloudFilters", features ...))] but that requires some extra codegen on your end.
(You will again have to use some form of #[cfg_attr(docsrs, doc(cfg(...)))] to add this attribute on nightly only)

@kennykerr
Copy link
Collaborator Author

Thanks, I ended up just generating it myself: #1268

@MarijnS95
Copy link
Contributor

@kennykerr Cool - just keep doc_cfg in mind for when it stabilizes, as it's quite a bit nicer than having in-line documentation text (ie. nice, visible bubbles that also show in module-level documentation).

@kennykerr
Copy link
Collaborator Author

I'm unlikely to use it, even if it were stable, as the output is not appealing.

image

vs

image

I like how it "pops", but the wordiness and the redundant features makes it less practical.

Obviously, if it improves over time I can reconsider.

@MarijnS95
Copy link
Contributor

@kennykerr I meant using #[doc(cfg())] (the override) to specify exactly what features are shown.

@MarijnS95
Copy link
Contributor

@kennykerr So with that trivial set of changes, here's what the rendered docs look like:

image

And the functions:

image

Note that these bubbles don't repeat what is shown at the top of the page for the entire CloudFilters module. However, you may have already noticed that Win32 made its return, and that also means it unfortunately remains shown on individual types and functions:

image

I think this is rustdoc recursively adding all the features it needed to get to this module in the first place, and that the #[doc(cfg())] override is only able to change the innermost config requirements set by #[cfg()]. The code generated for this functions is as follows:

#[cfg(all(feature = "Win32_Foundation", feature = "Win32_Storage_FileSystem", feature = "Win32_System_SystemServices"))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "Win32_Storage_CloudFilters", feature = "Win32_Foundation", feature = "Win32_Storage_FileSystem", feature = "Win32_System_SystemServices"))))]

@jyn514
Copy link

jyn514 commented Nov 1, 2021

I think this is rustdoc recursively adding all the features it needed to get to this module in the first place, and that the #[doc(cfg())] override is only able to change the innermost config requirements set by #[cfg()]. T

FWIW, I would be ok with changing the behavior of rustdoc here, I think it makes sense not to propagate the cfgs if you have a custom doc(cfg). cc @Nemo157

@MarijnS95
Copy link
Contributor

MarijnS95 commented Nov 1, 2021

@jyn514 I think either is fine - I may have even hinted to @GuillaumeGomez to want that at some point while working on gtk/gstreamer/gir docs in the past - but also retrospectively not showing feature requirements on items when they are already specified on the surrounding type/impl/mod (but then we're getting in rust-lang/rust#83428 territory).

It's probably more interesting to implement @kennykerr's suggestion above: don't show features that are already required for other (explicitly required/overridden) feature(s). I wasn't expecting rustdoc to not know anything about the cargo environment yet, I guess all crate dependencies coming from it are passed on the cmdline?

@riverar
Copy link
Collaborator

riverar commented Nov 1, 2021

@MarijnS95 The and conjunctions in the feature callout prevents devs from easily copy/pasting into Cargo.toml (and is hard to read). Is that customizable too? Just throwing that out there.

@jyn514
Copy link

jyn514 commented Nov 1, 2021

The and conjunctions in the feature callout prevents devs from easily copy/pasting into Cargo.toml (and is hard to read).

It's not customizable (feel free to open an issue for that - rust-lang/rust#43781 (comment)), but I think it would be reasonable for rustdoc to make it a comma-separated list instead.

@GuillaumeGomez
Copy link
Contributor

This issue too. We are talking about how to render the cfg information. If you have ideas, please share them. :)

@MarijnS95
Copy link
Contributor

MarijnS95 commented Nov 1, 2021

Just wondering, does the and change to or when any() instead of all() is used? [EDIT: Yes, I can see or being used in the "long cfg expression" issue linked just above.] And are these parenthesized to disambiguate when nested combinations of ie. any(feature = x, all(feature = y, feature = z)) are specified?

@kennykerr kennykerr deleted the doc_cfg branch November 16, 2021 22:09
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.

Some functions do not enable with their corresponding feature
5 participants