colin-kiegel / rust-derive-builder

derive builder implementation for rust structs
https://colin-kiegel.github.io/rust-derive-builder/
Apache License 2.0
1.32k stars 88 forks source link

Add an option to extend collection-like fields #196

Closed andy128k closed 3 years ago

andy128k commented 3 years ago

Addresses #114

andy128k commented 3 years ago

@TedDriggs I am not aware of such crates. I agree that name of setter <field>_extend_one is bumpy (it follows std). But guessing singular name of a setter from the plural name of a field and do it properly (English is hard :)) will take much more code than this feature deserves.

TedDriggs commented 3 years ago

Here my English vocabulary reaches a hardcover.

Your vocabulary is totally up to the task; the challenge here is that the underlying concept is more complex than a single word will accurately convey.

In the original issue, one proposal is that for some field example, calling builder.example(...) would push/extend/add an item, while builder.examples(...) would overwrite the whole collection. What if the declaration looked like this:

pub struct MyStruct {
    #[builder(setter(multiple(replace = "policies")))]
    policy: Vec<Policy>,
}

This would then generate

pub struct MyStructBuilder {
    policy: Option<Vec<Policy>>,
}

pub struct MyStructBuilder {
    policy: Option<Vec<Policy>>,
}

impl MyStructBuilder {
    pub fn policy(&mut self, value: Policy) -> &mut Self {
        self.policy
            .get_or_insert_with(|| Default::default())
            .extend(::std::iter::once(policy));
        self
    }

    pub fn policies(&mut self, value: Vec<Policy>) -> &mut Self {
        self.policy = Some(value);
        self
    }
}

The second setter is only emitted if replace = "..." is present in the setter's declaration. The main advantage of this approach is that the names stay concise, and each additional setter is opt-in:

Thoughts?

andy128k commented 3 years ago

@TedDriggs What if a field is named in plural form? In the case of #[builder(setter(multiple))] a setter will be fn policies(&mut self, value: Policy)? This looks strange.

What about #[builder(setter(multiple(one = "policy", all = "policies")))]? And if names are not given (#[builder(setter(multiple))] then fall back to <field_name> for all and <field_name>_add for one.

ShaneEverittM commented 3 years ago

In David Tolnay's proc-macro workshop, he uses the syntax: #[builder(each = "name")] above the field to be extended, which will generate a function called "name" that extends the collection. Would love it if this PR was merged.

andy128k commented 3 years ago

I like each = "name" and created an alternative PR #199

TedDriggs commented 3 years ago

Closing in favor of #199