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

feat: add strip_option(fallback = field_opt) #150

Merged
merged 4 commits into from
Aug 22, 2024

Conversation

ifiokjr
Copy link
Contributor

@ifiokjr ifiokjr commented Aug 21, 2024

Description

Fixes #117 using the API described in #117 (comment).

#[derive(TypedBuilder)]
struct SomeStruct {
     #[builder(default, setter(strip_option)]
     inner: Option<u32>,
     #[builder(setter(strip_option(fallback="outer_opt"))]
     outer: Option<u32>,

}

/// `strip_option` retains its current behaviour avoiding a breaking change.
/// `strip_option(fallback ="outer_opt")` strips the `Option`, and creates a fallback that still takes the option. 
/// This makes it fully opt-in.
fn new_api(value: Option<u32>) -> SomeStruct {
    match value {
        None => SomeStruct::builder().outer_opt(None).build(),
        Some(v) => SomeStruct::builder().inner(v).outer(v).build(),
    }
}

/// `strip_option(fallback ="outer_opt") makes the original `outer` builder method take the value, and creates an `outer_opt` which still takes an `Option`.
fn new_way(value: Option<u32>) -> SomeStruct {
    SomeStruct::builder().outer_opt(value).build()
}

@ifiokjr
Copy link
Contributor Author

ifiokjr commented Aug 22, 2024

@idanarye maybe I can extend the functionality here to also cover strip_bool.

Copy link
Owner

@idanarye idanarye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some remarks on this. Do you want to fix them? If not, I don't mind doing them myself after merging.

typed-builder-macro/src/field_info.rs Outdated Show resolved Hide resolved
typed-builder-macro/src/field_info.rs Outdated Show resolved Hide resolved
typed-builder-macro/src/field_info.rs Outdated Show resolved Hide resolved
@ifiokjr
Copy link
Contributor Author

ifiokjr commented Aug 22, 2024

@idanarye should I create a separate PR for strip_bool?

@ifiokjr ifiokjr changed the title feat: add strip_option(fallback = "field_opt") feat: add strip_option(fallback = field_opt) Aug 22, 2024
@idanarye
Copy link
Owner

Yes, but let's merge this one first. Much of it would probably be the same so lets iron it out once instead of working on the same issues twice.

@ifiokjr
Copy link
Contributor Author

ifiokjr commented Aug 22, 2024

Yes, but let's merge this one first. Much of it would probably be the same so lets iron it out once instead of working on the same issues twice.

I'll create a follow up PR once this is merged.

@idanarye idanarye merged commit 60ae4b6 into idanarye:master Aug 22, 2024
9 checks passed
@ifiokjr ifiokjr deleted the strip_option branch August 22, 2024 14:58
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.

feature: add an opt version of the builders when strip_option is enabled.
2 participants