colin-kiegel / rust-derive-builder

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

Consuming build but by mut setters? #321

Open VorpalBlade opened 5 months ago

VorpalBlade commented 5 months ago

It seems #[builder(pattern = "owned")] changes the behaviour of both the build function as well as the setters. I would like to decouple that:

The pattern looks something like this in pseudo-rust:

let mut results = vec![];
let mut builder = None
let mut buffer = String::new();

while input.read_line(&mut buffer)? > 0 {
    let line = buffer.trim();
    // Handle new section (which also has the name of the section in it)
    if let Some(stripped) = line.strip_prefix("EntryThatIndicatesNewSection: ") {
        if let Some(inner) = builder {
            results.push(inner.build());
        }
        builder = Some(Builder::new());
        builder.as_mut().unwrap().name(stripped)
    } else if let Some(stripped) = line.strip_prefix("OtherEntry: ") {
        // Handle other fields, some are mandatory, some are optional (have defaults on builder)
        builder.as_mut().unwrap().other_entry(stripped);
    } else if let Some(stripped) = line.strip_prefix("AnotherEntry: ") {
        // ...
    }
    buffer.clear();
}

// Handle final entry
if let Some(inner) = builder {
    results.push(inner.build());
}

This is leaving out proper error handling etc for clarity, but it is there in the real thing.

VorpalBlade commented 5 months ago

By the way, I checked with profiling and in my case the clone calls for those Vecs do not get optimised away unfortunately.

TedDriggs commented 4 months ago

This should be straightforward to implement.

API

We'd allow setting the pattern in #[builder(build_fn(pattern = "..."))]. if present, this would override the one set at the #[builder(pattern = "...")] level; if absent, the builder-level pattern will be used.

Implementation

Options::as_build_method would be updated to use the new build_fn override when initializing BuildMethod's pattern field.

TedDriggs commented 4 months ago

Looking at this again, I'm now second-guessing my original thought on the API. It feels like it might be better for the top-level pattern field to be the one that controls the signature of the build function, while allowing the override of the setters in the #[builder(setter(pattern = "..."))] meta item at the struct level.

TedDriggs commented 4 months ago

Okay, so I think the reason this is proving more challenging than expected is that the current behavior of the pattern field is weird.

Currently, that field can be set on the struct or on a field. The strange part is how setting it on a field interacts with the build method. I don't understand the rationale behind the current design, so I'm hesitant to change it too much.