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

Setter taking Option for non-optional fields #69

Open
dpc opened this issue Aug 5, 2022 · 2 comments
Open

Setter taking Option for non-optional fields #69

dpc opened this issue Aug 5, 2022 · 2 comments

Comments

@dpc
Copy link

dpc commented Aug 5, 2022

I have a a value coming from clap argument parsing that might or might not be set (so it's Option<u64>, and I want to use it (if set) to override a field that has a default in typed-builder for Params.

Ideally I'd like to:

let params = Parameters::builder()
  .db_path(opts.db_path.clone())
  .base_segment_file_size(opts.segment_size)
  .build();

but this doesn't work because opts.segment_size is Option<u64> and base_segment_file_size: u64.

Trying to achieve it with an if statement doesn't work either, because the type of each branch are different, and there seem to be no way to get the default value.

This would work:

    let params = if let Some(segment_size) = opts.base_segment_file_size {
        params.base_segment_file_size(segment_size)
    } else {
        params.base_segment_file_size(/* how to get the default? */)
    };

if there was a way to actually get the default defined in #[builder(default = 16 * 1024 * 1024)].

I eventually moved the default into separate constant and went with:

    let params = if let Some(segment_size) = opts.base_segment_file_size {
        params.base_segment_file_size(segment_size)
    } else {
        params.base_segment_file_size(Parameters::DEFAULT_BASE_SEGMENT_SIZE)
    };
@idanarye
Copy link
Owner

idanarye commented Aug 5, 2022

Maybe I can add a #[builder(setter(explicit_default))] option, that will make the field mandatory even if it has a default (I'll force it to have a default), but make the setter accept and Option and set it to the default if it receives a None?

As a side note - why are you using TypedBuilder to pass fields from clap a Parameters struct instead of making Parameters a #[derive(Parser)] and using #[clap(flatten)] to embed it in your main clap parser?

@dpc
Copy link
Author

dpc commented Aug 6, 2022

Maybe I can add a #[builder(setter(explicit_default))] option, that will make the field mandatory even if it has a default (I'll force it to have a default), but make the setter accept and Option and set it to the default if it receives a None?

I'm not entirely clear here on the details, but merging behavior (None argument just doesn't change the underlying value) is kind of preferable, IMO. In my use case this doesn't matter however.

As a side note - why are you using TypedBuilder to pass fields from clap a Parameters struct instead of making Parameters a #[derive(Parser)] and using #[clap(flatten)] to embed it in your main clap parser?

I am not sure what do you mean exactly, so it didn't even cross my mind. But that TypedBuilder is more generic and meant for library consumption, and the clap exposes only some of the values to override, and they could even eventually live in different crates.

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

No branches or pull requests

2 participants