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

codegen/doc: Omit "Feature: vXXX" text in favour of doc_cfg #1086

Merged
merged 1 commit into from
Apr 26, 2021

Conversation

MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Mar 28, 2021

Nowadays we have the doc_cfg feature rendering clean, consistent and obvious markers for feature requirements through doc(cfg()) attributes in the code (perhaps automatically derived from cfg() attributes in the not so distant future). With this it is not necessary to repeat the requirement in the documentation text once again, which wasn't really obvious nor good-looking to begin with.

Besides, this "Feature:" text is currently erroneously generated on the last member of an enum, instead of on the enum itself (parts omitted for brevity):

#[cfg(any(feature = "v1_14_1", feature = "dox"))]
#[cfg_attr(feature = "dox", doc(cfg(feature = "v1_14_1")))]
pub enum WebRTCFECType {
    /// none
    None,
    /// ulpfec + red
    ///
    /// Feature: `v1_14_1`
    ///
    UlpRed,
}

Note that other documentation comments on the enum are correctly generated on the enum itself.

Nowadays we have the `doc_cfg` feature rendering clean, consistent and
obvious markers for feature requirements through `doc(cfg())` attributes
in the code (perhaps automatically derived from `cfg()` attributes in
the not so distant future). With this it is not necessary to repeat the
requirement in the documentation text once again, which wasn't really
obvious nor good-looking to begin with.

Besides, this "Feature:" text is currently erroneously generated on the
last member of an enum, instead of on the enum itself (parts omitted for
brevity):

    #[cfg(any(feature = "v1_14_1", feature = "dox"))]
    #[cfg_attr(feature = "dox", doc(cfg(feature = "v1_14_1")))]
    pub enum WebRTCFECType {
        /// none
        None,
        /// ulpfec + red
        ///
        /// Feature: `v1_14_1`
        ///
        UlpRed,
    }
@GuillaumeGomez
Copy link
Member

The problem is that doc_cfg is unstable and that it'll be removed in a not so far future. I really don't think this is a good idea here...

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Mar 30, 2021

@GuillaumeGomez You mean this was just some nightly experiment that will disappear entirely? Will it be superseded by automatically detecting the feature from #[cfg()] directly or have no immediate replacement at all until much later?

@GuillaumeGomez
Copy link
Member

The goal is to make rustdoc aware of the cfg attributes directly so there is no need for the doc(cfg). A PR is already in progress. The end goal being to completely remove it.

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Mar 30, 2021

The goal is to make rustdoc aware of the cfg attributes directly so there is no need for the doc(cfg). A PR is already in progress. The end goal being to completely remove it.

@GuillaumeGomez That's really good news, but that still means the vx_xx features are rendered in the HTML right? That is still a valid reason to remove this now-superfluous comment from the documentation, now?

@GuillaumeGomez
Copy link
Member

I'm not sure of what you mean. My point is that I'm worried that using an unstable feature might prevent users to generate doc locally or even break our crates' code. The update on rustdoc side will add it automatically depending on the cfg.

@sdroege
Copy link
Member

sdroege commented Mar 31, 2021

My point is that I'm worried that using an unstable feature

That's already the case with the current docs. They only build with nightly.

@MarijnS95
Copy link
Contributor Author

I'm not sure of what you mean. My point is that I'm worried that using an unstable feature might prevent users to generate doc locally or even break our crates' code. The update on rustdoc side will add it automatically depending on the cfg.

The unstable feature is already fully in use and docs (if dox is enabled) already build with that. This PR does nothing to change or prolong that requirement. It only removes some redundant (again, if building with doc_cfg) text from the documentation body.

In other words: if you want the textual Feature: vXXX message to stay until doc_cfg is stabilized and everyone can build (implicitly) with the dox feature without nightly, this PR should be stashed for a bit.

@sdroege sdroege merged commit deec94b into gtk-rs:master Apr 26, 2021
@MarijnS95 MarijnS95 deleted the doc-features-use-doc_cfg branch April 26, 2021 08:40
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.

4 participants