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

Switch to doc cfg instead of feature dox #1009

Closed
GuillaumeGomez opened this issue Nov 27, 2020 · 12 comments
Closed

Switch to doc cfg instead of feature dox #1009

GuillaumeGomez opened this issue Nov 27, 2020 · 12 comments

Comments

@GuillaumeGomez
Copy link
Member

It was first implemented in #1005 but proved impossible to work when the PR on gtk-rs was opened.

This is currently not possible because of two things (but only one of them fixed would allow us to make it work):

@MarijnS95
Copy link
Contributor

@GuillaumeGomez Is it time to flip this back on? I read somewhere that #[cfg] is now picked up without requiring #[doc(cfg())] too? Is the second issue still a problem in practice?

@GuillaumeGomez
Copy link
Member Author

It's nightly only for now and requires to enable the feature.

@MarijnS95
Copy link
Contributor

@GuillaumeGomez Sure, the "advantage" of the current solution is that you can still build (incomplete) docs on stable as long as dox is not set. docs.rs already builds with nightly though.

@bilelmoussaoui
Copy link
Member

Note that our current docs require nightly as well

@GuillaumeGomez
Copy link
Member Author

gtk-rs documentation generated on docs.rs is mostly useless unfortunately. Also, we'd need to use cfg_hide to not display too many cfgs (because we might have some only needed for docs in any case). So not a straightforward conversion.

Note that our current docs require nightly as well

True, but relying on a very recent nightly feature which is not complete yet (I need to send some follow-ups) is not the best idea here I think.

@MarijnS95
Copy link
Contributor

MarijnS95 commented Oct 30, 2021

In GStreamer we already have a (by accident, in the GL windowing modules I added probably right during this back-and-forth transition) #![cfg_attr(all(not(doctest), doc), feature(doc_cfg))] that doesn't compile on stable rustdoc anyway.

@GuillaumeGomez Yes, we'd have to do the transition atomically. If we just start relying on not needing doc(cfg()) anymore the feature="dox" will show up until we can replace that with doc as cfg option.

EDIT: Ah, apparently #[cfg(any(doc, feature = "vx_y"))] makes doc show up next to the version requirement, too :(

@GuillaumeGomez
Copy link
Member Author

There is cfg_hide for that. :3

@MarijnS95
Copy link
Contributor

@GuillaumeGomez Sure, but I don't like that :|

@GuillaumeGomez
Copy link
Member Author

On one hand, we can remove all doc(cfg), but on the other we have to also use cfg_hide globally to avoid having some cfgs showing up. For me it seems like a big win. Just that I'd prefer to wait a bit before doing that.

@MarijnS95
Copy link
Contributor

MarijnS95 commented Oct 30, 2021

@GuillaumeGomez Yes, using cfg_hide to just statically hide doc is a lot more convenient than having to emit a dynamic doc(cfg()) that duplicates whatever was written in #[cfg()] (minus the things we don't want to show up), but I'd just have liked if doc didn't show up at all.

Surely that may confuse something as --cfg doc does enable that codepath, so it makes sense to show up without cfg_hide.

@GuillaumeGomez
Copy link
Member Author

We had issues only relying on cfg(doc), don't remember which but it was quite painful.

@pentamassiv
Copy link
Contributor

I think this can be closed now that #[cfg(docsrs)] is used and the dox feature is gone

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

No branches or pull requests

4 participants