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

Fine-grained selection of where to add type or field attributes using filters #549

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marjakm
Copy link

@marjakm marjakm commented Oct 19, 2021

This does not break the external API, it only adds 2 new functions

pub fn type_attribute_with_filter<P, A, F>(
    &mut self,
    path: P,
    attribute: A,
    filter: F,
) -> &mut Self;
where
    P: AsRef<str>,
    A: AsRef<str>,
    F: Into<TypeFilter>
    
pub fn field_attribute_with_filter<P, A, TF, LF, FF>(
    &mut self,
    path: P,
    attribute: A,
    type_filter: TF,
    label_filter: LF,
    field_filter: FF,
) -> &mut Self;
where
    P: AsRef<str>,
    A: AsRef<str>,
    TF: Into<TypeFilter>,
    LF: Into<LabelFilter>,
    FF: Into<FieldFilter>

and 6 types - 3 pairs of Filter and Selector:

TypeFilter, TypeSelector,
LabelFilter, LabelSelector, 
FieldFilter, FieldSelector, 

Filters are bitsets (newtypes around u32) and Selectors give meaning to values in that bitset (c-like enums with selected discriminator values). Attribute maps contain these filters and attributes are only applied if all filters match.

    type_attributes: PathMap<(String, TypeFilter)>,
    field_attributes: PathMap<(String, TypeFilter, LabelFilter, FieldFilter)>,

This allows very specific selection of where to add attributes, example:

        .type_attribute(".", "#[derive(serde::Serialize, serde::Deserialize)]")
        // enums with data should use serde adjacently tagged format
        .type_attribute_with_filter(
            ".",
            "#[serde(tag = \"kind\", content = \"data\")]",
            TypeSelector::RustEnumWithData
        )
        // All c-like enums should have clap::ArgEnum impl
        .type_attribute_with_filter(
            ".",
            "#[derive(clap::ArgEnum)]",
            TypeSelector::RustEnumCLike
        )
        // and structs a clap::Parser impl
        .type_attribute_with_filter(
            ".",
            "#[derive(clap::Parser)]",
            TypeSelector::RustStruct
        )
        // use a trait to find parsers for non-scalar types
        .field_attribute_with_filter(
            ".",
            "#[clap(parse(try_from_str=crate::CustomClapParse::parse))]",
            TypeSelector::RustStruct,
            LabelSelector::Everything,
            !FieldSelector::ProtobufScalar | FieldSelector::Bytes,
        )
        // repeated String also needs the custom parser
        .field_attribute_with_filter(
            ".",
            "#[clap(parse(try_from_str=crate::CustomClapParse::parse))]",
            TypeSelector::RustStruct,
            LabelSelector::Repeated,
            FieldSelector::String
        )

This probably needs some tests, but that would be quite pointless if you don't like this feature - if this seems reasonable, I can add a bunch of tests.

Previous pull request had a similar idea, but implemented it differently:
#494

May solve cases like these:
#371
#332

@anelson
Copy link

anelson commented Nov 4, 2021

@marjakm this is a great improvement, we've been suffering the lack of this fine-grained functionality for a long time!

@danburkert this would nicely solve our problem in #371 .

cc @Veetaha

@marjakm
Copy link
Author

marjakm commented Nov 4, 2021

I don't quite understand why the github actions failed, doesn't seem like related to anything I changed:

 error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/half-1.8.1/Cargo.toml`
Error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/half-1.8.1/Cargo.toml`
Caused by:
  feature `resolver` is required

  this Cargo does not support nightly features, but if you
  switch to nightly channel you can add
  `cargo-features = ["resolver"]` to enable this feature
Error: The process '/usr/share/rust/.cargo/bin/cargo' failed with exit code 101

@marjakm
Copy link
Author

marjakm commented Dec 9, 2021

@LucioFranco any comments?

@sunqi1993
Copy link

any progress?

@LucioFranco
Copy link
Member

@marjakm you will need to rebase against master to get CI working again

@marjakm
Copy link
Author

marjakm commented Jan 21, 2022

I'll be on vacation for a week starting tomorrow, I'll do it afterwards (just to let you know, I'm still interested in getting this merged).

@XL-Lewis
Copy link

This feature would be really handy for adding custom derivations to particular types i.e. strum's variety of derive functions for enums.

Can I enquire if there's any progress?

@LucioFranco
Copy link
Member

I think something like this could eventually be merged but I am not sure atm if this is the right approach.

@marjakm
Copy link
Author

marjakm commented Sep 6, 2022

Squashed commits and rebased onto master

@sunny-g
Copy link

sunny-g commented Dec 20, 2022

any updates on this? running into many of the same issues, re: specificity with type/field attributes applied to nested defs

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.

6 participants