-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Introduce bool shorthand (#[builder(flag)]
)
#142
Comments
Maybe something like #[derive(Builder)]
struct Example {
#[builder(flag)]
color: bool,
} It would generate three methods:
I think a method that accepts a value is still useful in many cases to make conditional building painless. For example, if the value of color should be computed from an env var, the user will still be able to use In fact the Anyway this feature by itself can just provide 3 setters to simplify the syntax for the cases where the API is used with hardcoded values a lot. Also, this is how I imagine the syntax to override the names of each of the 3 setters: struct Example {
#[builder(
// the values specified here are the ones that will be used by default (#[builder(flag)])
flag(
true = color, // sets the bool to `true`
false = no_color, // sets the bool to `false`
set = set_color // accepts a `bool` argument
),
)]
color: bool,
} I'm not sure about For example, AWS SDK for Rust uses a similar convention of Also, what about #[derive(Builder)]
struct Example {
#[builder(flag)]
color: Option<bool>,
} There will be methods:
But.. This has to be supported because booleans are often annotated with I like the |
The naming sounds good to me 👍 Need to think about the potential confusion with options, that's an aspect I hadn't considered. |
Alternatively, the Also, I'm currently working on a wider feature that allows adding arbitrary setters and methods to the builder that will cover this use case, although it'll require you to write your own impl block with methods which is a bit more code, but will at least make it possible to create such API by yourself. After that is done we may have the |
Such a feature would be great, I'm currently having trouble with custom methods in my builders |
I've started work on the feature for custom method extensions of builder type and stable type state signature in #145. See the usage shown in the PR description there for an example of how you may implement this feature using this new upcoming flexible low-level API. I still need to add some syntax for hiding setters (overriding their visibility and renaming them) to make this use case possible, which I'll do as part of that PR. Unfortunatelly this update will require a 3.0 version bump of |
#[builder(flag)]
)
I'm revisiting the design for this yet again as part of the bigger I propose to make this syntax sugar a bit more limited, but probably more intuitive. // Generates two setters for booleans:
// - `my_lovely_flag() -> true`
// - `with_my_lovely_flag(bool)`
//
// It also automatically implies that the setters are optional to call.
// The default value is `false` automatically. The `#[builder(default)]`
// attribute is not allowed with the `flag` attribute.
#[builder(flag)]
my_lovely_flag: bool, I decided to change the by-value setter prefix to In the future there will be a member-level |
Hi there, thanks for your great work on this crate! I can relate that this I think we could recycle the logic applied to The pub struct MyType<T> {
my_option: Option<T>,
my_flag: bool,
}
#[bon]
impl<T> MyType<T> {
#[builder]
fn new(my_option: Option<T>, my_flag: #[builder(flag)] bool) -> Self {
Self { my_option, my_flag }
}
}
MyType::builder().maybe_my_option(Some(t)).build();
MyType::builder().maybe_my_option(None).build();
MyType::builder().my_option(t).build();
MyType::builder().maybe_my_flag(true).build();
MyType::builder().maybe_my_flag(false).build();
MyType::builder().my_flag().build(); Also, this Here's an example use case with redis's With the following AST: pub enum Condition {
// if not exists
Nx,
// if already exists
Xx,
}
pub struct Duration<P>(i64, PhantomData<P>);
pub struct UnixTimestamp<P>(Duration<P>);
pub struct Seconds;
pub struct Milliseconds;
pub enum Expiration {
Ex(Duration<Seconds>),
ExAt(UnixTimestamp<Seconds>),
Px(Duration<Milliseconds>),
PxAt(UnixTimestamp<Milliseconds>),
KeepTtl,
}
pub struct SetRequest<'a> {
key: &'a [u8],
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
get: bool
} Ideally, I'd like to be able to define the builder as such: use bon::bon;
#[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new<K, V>(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
#[builder(flag)] condition: Option<Condition>,
#[builder(flag)] expiration: Option<Expiration>,
#[builder(flag)] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get
}
}
} And it would provide the following usages: SetRequest::builder()
.key("hello")
.value("world")
.maybe_condition(Some(Condition::Nx))
.maybe_expiration(Some(Expiration::Ex(Duration::<Seconds>::try_from(42)?)))
.maybe_get(true)
.build()?;
SetRequest::builder()
.key("hello")
.value("world")
.condition(Condition::Nx)
.expiration(Expiration::Ex(Duration::<Seconds>::try_from(42)?))
.get()
.build()?;
SetRequest::builder()
.key("hello")
.value("world")
.nx()
.ex(Duration::<Seconds>::try_from(42)?)
.get()
.build()?; If you feel like this is interesting, it'd be happy to contribute :) PS: Sorry for the long post, I figured it would be easier to explain with actual code and real world example ^^" |
Hi, thank you for sharing your ideas!
I don't understand what the attribute
The The prefix Therefore, I think |
Wow, thanks for the fast and thorough reply. I didn't know that the I totally understand now why we should have a different prefix for My idea with the SetRequest::builder()
.key("hello")
.value("world")
.nx() // sugar for `.condition(Condition::Nx)` or `.maybe_condition(Some(Condition::Nx))`
.ex(42.try_into()?) // sugar for `.expiration(Expiration::Ex(42.try_into()?))` or `.maybe_expiration(Some(Expiration::Ex(42.try_into()?)))`
.get() // sugar for `get(true)` which will become `.with_get(true)` as far as I understand
.build()?; |
Aha, now I see the idea. There are several problems with this design, such that I think, it shouldn't be part of Limited lexical scopeThe fundamental limitation of macros (both declarative and procedural) is that they only see "the code" they were applied to. For example: #[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new<K, V>(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
#[builder(flag)] condition: Option<Condition>,
#[builder(flag)] expiration: Option<Expiration>,
#[builder(flag)] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get
}
}
} Here, the code generation is performed by the active Although, there are some hacks to provide more context about the Demand for this featureThis feature of generating a setter for every enum variant looks rather exotic to my taste. My gut feeling is that there isn't a considerable demand for this feature out there. So.. it doesn't really make sense to provide syntax sugar for this case. The attribute However, I did say the words "syntax sugar". The term "syntax sugar" means just a shorter notation for what's already possible to express. So it doesn't mean that this API isn't physically possible to express with AlternativeI'm currently working on a big PR stabilizing the builder's type state API (#145). It would allow you to add custom setters to the builder struct directly, by defining your own impl block for the builder. So this will be possible to achieve using the following lower-level code. It already compiles from my PR's branch (#145): use bon::bon;
use std::marker::PhantomData;
#[bon]
impl<'a> SetRequest<'a> {
#[builder]
fn new(
key: &'a (impl ?Sized + AsRef<[u8]>),
value: &'a (impl ?Sized + AsRef<[u8]>),
condition: Option<Condition>,
expiration: Option<Expiration>,
#[builder(setters(name = set_get))] get: bool,
) -> Self {
Self {
key: key.as_ref(),
value: value.as_ref(),
condition,
expiration,
get,
}
}
}
// Import type-state API tooling from the generated module
use set_request_builder::{IsUnset, SetCondition, SetExpiration, SetGet, State};
// Define a custom impl block for the builder with custom setters
impl<'a, S: State, K, V> SetRequestBuilder<'a, K, V, S>
where
K: ?Sized + AsRef<[u8]>,
V: ?Sized + AsRef<[u8]>,
{
// Custom setter for the enum
fn nx(self) -> SetRequestBuilder<'a, K, V, SetCondition<S>>
// Setters need to have a `where` clause requiring that they can only be called if the
// member wasn't set yet (i.e. the state for the member implements `IsUnset` trait)
where
S::Condition: IsUnset,
{
self.condition(Condition::Nx)
}
// Custom setter for the enum that accepts a value
fn ex(self, duration: Duration<Seconds>) -> SetRequestBuilder<'a, K, V, SetExpiration<S>>
where
S::Expiration: IsUnset,
{
self.expiration(Expiration::Ex(duration))
}
// Custom boolean `flag` setter. Note that `#[builder(flag)]` isn't implemented yet, so in the
// mean time it will be possible to just use this API to implement flag-like setters, but the
// `#[builder(flag)]` feature should be available in the `3.1` release shortly after `3.0` is released
fn get(self) -> SetRequestBuilder<'a, K, V, SetGet<S>>
where
S::Get: IsUnset,
{
self.set_get(true)
}
}
pub enum Condition {
// if not exists
Nx,
// if already exists
Xx,
}
pub struct Duration<P>(i64, PhantomData<P>);
impl<P> Duration<P> {
fn new(value: i64) -> Self {
Self(value, PhantomData)
}
}
pub struct UnixTimestamp<P>(Duration<P>);
pub struct Seconds;
pub struct Milliseconds;
pub enum Expiration {
Ex(Duration<Seconds>),
ExAt(UnixTimestamp<Seconds>),
Px(Duration<Milliseconds>),
PxAt(UnixTimestamp<Milliseconds>),
KeepTtl,
}
pub struct SetRequest<'a> {
key: &'a [u8],
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
get: bool,
}
fn main() {
SetRequest::builder()
.key("hello")
.value("world")
.maybe_condition(Some(Condition::Nx))
.maybe_expiration(Some(Expiration::Ex(Duration::new(42))))
.set_get(true)
.build();
SetRequest::builder()
.key("hello")
.value("world")
.condition(Condition::Nx)
.expiration(Expiration::Ex(Duration::new(42)))
.get()
.build();
SetRequest::builder()
.key("hello")
.value("world")
.nx()
.ex(Duration::new(42))
.get()
.build();
} |
Btw, I just noticed, that the impl block with the #[derive(bon::Builder)]
pub struct SetRequest<'a> {
#[builder(with = |value: &'a (impl ?Sized + AsRef<[u8]>)| value.as_ref())]
key: &'a [u8],
#[builder(with = |value: &'a (impl ?Sized + AsRef<[u8]>)| value.as_ref())]
value: &'a [u8],
condition: Option<Condition>,
expiration: Option<Expiration>,
// In `3.1` (after 3.0) this will just be `#[builder(flag)]`
#[builder(setters(name = set_get))]
get: bool,
} But you'll still need to have a custom impl block for the specialized enum setters |
I see you've given deep thoughts in the overall design already and it all makes sense. |
|
#[builder(flag)]
)#[builder(flag)]
)
Hey there,
Bon is great but it still feels weird to write
.property(true)
for booleans and I feel that an option to shorten this usage would be a nice addition.The syntax is just an example of how I would see it implemented :
And then use it without specifying
true
:For falsy values, one could think of adding the same method with a prefix (
no_
by default) :And then :
Feedback is welcome, it would greatly improve my builders!
A note for the community from the maintainers
Please vote on this issue by adding a 👍 reaction to help the maintainers with prioritizing it. You may add a comment describing your real use case related to this issue for us to better understand the problem domain.
The text was updated successfully, but these errors were encountered: