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

Enum / Flags: generate user defined `derive` #611

Merged
merged 1 commit into from Jun 20, 2018

Conversation

Projects
None yet
2 participants
@fengalin
Contributor

fengalin commented Jun 18, 2018

Allow using API mode TOML config such as:

[[object]]
name = "Gst.BufferFlags"
status = "generate"
    [[object.derive]]
    name = "Serialize, Deserialize"
    cfg_condition = "feature = \"ser_de\""

[[object]]
name = "Gst.Format"
status = "generate"
    [[object.derive]]
    name = "Serialize, Deserialize"
    cfg_condition = "feature = \"ser_de\""

to generate:

 bitflags! {
     #[cfg_attr(feature = "ser_de", derive(Serialize, Deserialize))]
     pub struct BufferFlags: u32 {
         const LIVE = 16;
         [...]
     }
 }

and

 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 #[cfg_attr(feature = "ser_de", derive(Serialize, Deserialize))]
 pub enum Format {
     Undefined,
     [...]
 }
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2018

Member

@fengalin Code looks good, but problem that in normal struct cfg applied to it, not to next derive.
You sure that it works with bitflags?

Member

EPashkin commented Jun 18, 2018

@fengalin Code looks good, but problem that in normal struct cfg applied to it, not to next derive.
You sure that it works with bitflags?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 18, 2018

Contributor

What do you mean? It works for me, even with the example you provided. This is the output I get:

NONE
Ok("{\"bf\":{\"bits\":0}}")

What's wrong with that?

Contributor

fengalin commented Jun 18, 2018

What do you mean? It works for me, even with the example you provided. This is the output I get:

NONE
Ok("{\"bf\":{\"bits\":0}}")

What's wrong with that?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2018

Member

try remove not

Member

EPashkin commented Jun 18, 2018

try remove not

@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 18, 2018

Contributor

Yes, you're right! Let me take a look at this...

Contributor

fengalin commented Jun 18, 2018

Yes, you're right! Let me take a look at this...

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 18, 2018

Contributor

It seems possible

Thanks. Implementing it now.

Contributor

fengalin commented Jun 18, 2018

It seems possible

Thanks. Implementing it now.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
Member

EPashkin commented Jun 18, 2018

@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 18, 2018

Contributor

Thanks for the review @EPashkin! It should be better now.

Contributor

fengalin commented Jun 18, 2018

Thanks for the review @EPashkin! It should be better now.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2018

Member

Also better check doc generation, maybe feature="dox" will cause error without "ser_de" in real project

Member

EPashkin commented Jun 18, 2018

Also better check doc generation, maybe feature="dox" will cause error without "ser_de" in real project

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 18, 2018

Member

.. and IMHO `[[object.derive]]name = "Serialize, Deserialize" will works too, so readme can be updated to reflect this

Member

EPashkin commented Jun 18, 2018

.. and IMHO `[[object.derive]]name = "Serialize, Deserialize" will works too, so readme can be updated to reflect this

@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 19, 2018

Contributor

Also better check doc generation, maybe feature="dox" will cause error without "ser_de" in real project

I couldn't find a way to detect doc generation without feature="dox" and for some reasons, doc is never generated for Serialize and Deserialize when used as #[derive()], even without a condition. So I decided to keep only the condition and remove feature="dox". I hope this is acceptable.

Contributor

fengalin commented Jun 19, 2018

Also better check doc generation, maybe feature="dox" will cause error without "ser_de" in real project

I couldn't find a way to detect doc generation without feature="dox" and for some reasons, doc is never generated for Serialize and Deserialize when used as #[derive()], even without a condition. So I decided to keep only the condition and remove feature="dox". I hope this is acceptable.

@@ -353,6 +354,25 @@ pub fn cfg_condition_string(
}
}
pub fn derives(

This comment has been minimized.

@EPashkin

EPashkin Jun 19, 2018

Member

Please add #[cfg_attr(feature = "cargo-clippy", allow(ptr_arg))]

@EPashkin

EPashkin Jun 19, 2018

Member

Please add #[cfg_attr(feature = "cargo-clippy", allow(ptr_arg))]

This comment has been minimized.

@fengalin

fengalin Jun 20, 2018

Contributor

I fixed the clippy warning.

@fengalin

fengalin Jun 20, 2018

Contributor

I fixed the clippy warning.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 19, 2018

Member

@fengalin Thanks, only small nit remain.
Yes, without 'dox' is acceptable, as we can write it in cfg_condition if needed.

Just in case: your old real code compiles with dox but without ser-de ?
I worried that it just don't compiles as serde* crates not included but #[derive(Serialise)] is.

Member

EPashkin commented Jun 19, 2018

@fengalin Thanks, only small nit remain.
Yes, without 'dox' is acceptable, as we can write it in cfg_condition if needed.

Just in case: your old real code compiles with dox but without ser-de ?
I worried that it just don't compiles as serde* crates not included but #[derive(Serialise)] is.

Enum / Flags: generate user defined `derive`
Allow using API mode TOML config such as:

``` toml
[[object]]
name = "Gst.BufferFlags"
status = "generate"
    [[object.derive]]
    name = "Serialize, Deserialize"
    cfg_condition = "feature = \"ser_de\""

[[object]]
name = "Gst.Format"
status = "generate"
    [[object.derive]]
    name = "Serialize, Deserialize"
    cfg_condition = "feature = \"ser_de\""
```

to generate:

``` rust
 bitflags! {
     #[cfg_attr(feature = "ser_de", derive(Serialize, Deserialize))]
     pub struct BufferFlags: u32 {
         const LIVE = 16;
         [...]
     }
 }
```

and

``` rust
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 #[cfg_attr(feature = "ser_de", derive(Serialize, Deserialize))]
 pub enum Format {
     Undefined,
     [...]
 }
```
@fengalin

This comment has been minimized.

Show comment
Hide comment
@fengalin

fengalin Jun 20, 2018

Contributor

your old real code compiles with dox but without ser-de ?

Sorry, my comment was not clear. The code compiles and behaves as expected. It's just the documentation for the Serialize and Deserialize Trait implementations for enums and structs that use #[derive(Serialize, Deserialize)] that is not generated.

Contributor

fengalin commented Jun 20, 2018

your old real code compiles with dox but without ser-de ?

Sorry, my comment was not clear. The code compiles and behaves as expected. It's just the documentation for the Serialize and Deserialize Trait implementations for enums and structs that use #[derive(Serialize, Deserialize)] that is not generated.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jun 20, 2018

Member

Strange.
Thank again, PR is great.

Member

EPashkin commented Jun 20, 2018

Strange.
Thank again, PR is great.

@EPashkin EPashkin merged commit 39654c7 into gtk-rs:master Jun 20, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@fengalin fengalin deleted the fengalin:enum_flags_user_derives branch Jun 20, 2018

vhdirk pushed a commit to vhdirk/gir that referenced this pull request Jul 6, 2018

Merge pull request #611 from fengalin/enum_flags_user_derives
Enum / Flags: generate user defined `derive`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment