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

Documentation no longer lists required features #2717

Closed
tim-weis opened this issue Nov 30, 2023 · 23 comments · Fixed by #2735, #2864 or #2865
Closed

Documentation no longer lists required features #2717

tim-weis opened this issue Nov 30, 2023 · 23 comments · Fixed by #2735, #2864 or #2865
Labels
enhancement New feature or request

Comments

@tim-weis
Copy link
Contributor

Summary

Starting with version 0.52.0, the documentation published here no longer lists all/some required features.

For example:

It looks like the requirements for the containing module have dropped off.

Crate manifest

No response

Crate code

No response

@tim-weis tim-weis added the bug Something isn't working label Nov 30, 2023
@ChrisDenton
Copy link
Collaborator

ChrisDenton commented Nov 30, 2023

Probably #2671. I think it no longer adds namespace features to the list of features.

EDIT: specifically

-            let mut tokens = format!(r#"`\"{}\"`"#, to_feature(self.namespace));
+            let mut tokens = String::new();

@kennykerr
Copy link
Collaborator

Yep the docs no longer explicitly include the feature for the enclosing module. This caused a huge amount of redundant doc comments and markup. A bit less intuitive for sure, but a tradeoff I think is worthwhile until I find a long-term replacement for rustdoc...

@kennykerr kennykerr added question Further information is requested and removed bug Something isn't working labels Nov 30, 2023
@junglie85
Copy link

OutputDebugStringW does not show up in search results to be able to find the module to use, it only shows the kernel32.dll.c definition. Maybe an edge case, but that makes it difficult to find which feature to enable.

@ChrisDenton
Copy link
Collaborator

From the docs, you just need to click on the parent module (Debug)

@junglie85
Copy link

That's fair - I tend to hunt around the GitHub repo rather than use the published docs.

@kennykerr
Copy link
Collaborator

Unfortunately, github's search capabilities are foiled by the size of the windows-rs repo.

@riverar
Copy link
Collaborator

riverar commented Nov 30, 2023

Think we should revert this change, this is markedly worse UX for no apparent benefit.

@tim-weis
Copy link
Contributor Author

tim-weis commented Dec 1, 2023

The only reason I care about the required features is to copy them into the features table of a Cargo.toml file. This used to work well when all features were listed.

After the change, things are far less convenient. It's not entirely unworkable as long as only the containing module's feature is required. When more features are required the documentation starts to feel very hostile (just go through the number of steps needed to get GetMessageW's features into a Cargo.toml).

Someone who knows their way around this crate may merely be inconvenienced. To anyone new, this may be reason enough to look into alternatives.

Reverting this change would be very much appreciated.

@ChrisDenton
Copy link
Collaborator

I wonder if it would help to remove the explicit need for things like Foundation and Win32_Foundation and instead make them automatically implied by using the relevant namespaces? They're almost certainly needed and I think they should not add significantly to code size in any case.

@kennykerr
Copy link
Collaborator

Yes, I'm looking for ways to reduce the feature complexity as simply reverting this change doesn't really solve the problem. Continuing to use rustdoc in this way is just not sustainable.

@tim-weis
Copy link
Contributor Author

tim-weis commented Dec 2, 2023

I'm looking for ways to reduce the feature complexity

I'm not objecting to that. I'm uncomfortable with having the documentation reflect a potentially simpler future world before it exists. Until we're there, the documentation should reflect the current state, with feature complexity and all.

@kennykerr
Copy link
Collaborator

cargo doc has a new doc_auto_cfg feature that finally seems to figure this out based on cfg attributes so we don't have to generate the redundant doc comments any longer. rust-lang/rust#43781

Unfortunately, formatting is not that great.

image

@kennykerr
Copy link
Collaborator

You also need to combine doc into all cfg attributes using #[cfg(any(doc, feature = "Win32"))] instead of just #[cfg(feature = "Win32")]. That's pretty lame.

@kennykerr
Copy link
Collaborator

I'm not objecting to that. I'm uncomfortable with having the documentation reflect a potentially simpler future world before it exists. Until we're there, the documentation should reflect the current state, with feature complexity and all.

Sure, but somebody has to support that "current state" until some "future world" where cargo doc and https://docs.rs/ supports the windows crate and I'm just not sure that should be on the windows crate. 😊

@ChrisDenton
Copy link
Collaborator

cargo doc has a new doc_auto_cfg feature that finally seems to figure this out based on cfg attributes so we don't have to generate the redundant doc comments any longer. rust-lang/rust#43781

Unfortunately, formatting is not that great.

image

cc @GuillaumeGomez Improving the formatting seems actionable?

@GuillaumeGomez
Copy link
Contributor

That does look very bad indeed. Can you open an issue on https://github.com/rust-lang/rust please?

@ChrisDenton
Copy link
Collaborator

ChrisDenton commented Dec 4, 2023

Done.

@GuillaumeGomez
Copy link
Contributor

To be more exact: rust-lang/rust#118615

@tim-weis
Copy link
Contributor Author

tim-weis commented Dec 4, 2023

Sure, but somebody has to support that "current state" [...] and I'm just not sure that should be on the windows crate. 😊

That's understood, and ideally, it shouldn't be. Realistically, though, the windows team is the only entity that can make up for the shortcomings at this time. No good deed goes unpunished, they say.

The output generated from the doc_auto_cfg feature is a regression compared against even the current state:

  • Features aren't enclosed in quotation marks, so we must manually add them.
  • Features aren't comma-separated, so more editing is required after copy-pasting them over.
  • The list of features is correct as far as the compiler is concerned, but it doesn't much help users: For OutputDebugStringW only "Win32_System_Diagnostics_Debug" needs to be enabled, with "parent features" getting enabled automatically.

Just to let you know, I'm not asking you to change things or take on responsibility for someone else's shortcomings. I'm just a user providing feedback on what works and what doesn't.

@kennykerr kennykerr added the enhancement New feature or request label Dec 29, 2023
@fgimian
Copy link

fgimian commented Jan 31, 2024

Honestly this is making it extremely hard to use the library at the moment. Is there a way the documentation which reflects the required features for the current version be made available somehow please?

e.g. https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/System/Registry/fn.RegEnumKeyW.html specifies that only Win32_Foundation is required when Win32_System_Registry is also required. I'm not sure where to find this information for the current v0.52.0 release.

@riverar
Copy link
Collaborator

riverar commented Feb 1, 2024

@fgimian Working on it, sorry for the delay. Here's a temporary tool you can use to unblock: https://riverar.github.io/4f76cc1e-88ae-41d3-906e-3591c67d2366/

We're working on standing this up next to the official docs shortly.

@fgimian
Copy link

fgimian commented Feb 1, 2024

@fgimian Working on it, sorry for the delay. Here's a temporary tool you can use to unblock: https://riverar.github.io/4f76cc1e-88ae-41d3-906e-3591c67d2366/

We're working on standing this up next to the official docs shortly.

Thank you so so much for this mate, really appreciate it!! 😄

@kennykerr
Copy link
Collaborator

I published the docs for windows 0.53.0 with the new feature search link:

https://microsoft.github.io/windows-docs-rs/doc/windows/

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
7 participants