-
-
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
The filler expression for a field to be able to use other field. #56
Comments
Hi! Thank you for creating the issue. It's definitely possible to update For example, the generated code can define local variables for every member, and may look like this: fn build(self) -> TestStruct {
// This is the value of `y` set by the setters
let y = self.__private_impl.y.0;
// Now the `skip_expression` has access to `y`. It's wrapped in an
// immediately-invoked closure to prevent the `skip_expression` from
// using the `return` keyword and returning from `build` instead.
let x = (|| skip_expression)();
TestStruct { y, x }
} However, I think the order of struct fields will have to matter then. For example, if the skip expression for What's available in the meantimeIt's possible to generate a builder that has this behaviour using a bit more verbose syntax today using the stable version of use bon::bon;
#[derive(Debug)]
struct TestStruct {
x: i32,
y: i32,
}
#[bon]
impl TestStruct {
#[builder]
fn new(y: i32) -> Self {
Self { x: y + 23, y }
}
}
// Works as expected
assert_eq!(25, TestStruct::builder().y(2).build().x); Did you consider using this approach? Or maybe you are already using this approach in the meantime? |
Thanks a lot @Veetaha for considering this feature.
The order of fields only matters if there is a transitive dependency. If ->->->->->->->->->->->->->->->->->->->->->->->->->->->- What's available in the meantime
I am assuming that |
I you are okay with me sending a PR, I can do that. You can just guide me through. But I understand if you are not accepting PRs at this point. |
Yeah, makes sense. I was just thinking to keep things simple, but we can make it more flexible. We can have such a rule that all variables without
Yeah, that's true. I see why you'd like this feature then.
I'm generally open to any PRs. However, at this specific time, I'm preparing a breaking change around disabling the automatic into conversions (as described in #15 (comment)) and a new 2.0 release. It's close to being done. Most of the docs were updated, but some more pages are left (#54), I think it'll be in master by the end of the week. That PR changes the code for Some more thoughts out-loud on this feature:
Another thought.. Since I'm already doing a 2.0 I can just make another breaking change of prohibiting the usage of |
Thanks a lot for helping out with this feature request.
That 's totally understandable. I will wait for you to merge this feature. |
I agree with ignoring this. I think of re-naming a field to be a feature of a serialization library like serde or serde-dynamo where how one deals with the variables in code is different from how they are transferred or persisted. It is mainly because one cares about readability when writing code but cares about byte-size efficiency at other times. We should also care about this macro playing nice with other widely used macros like
Makes sense to me.
When writing a builder, most of the action happens in the
Although we should try to minimize breaking changes, it should be okay to include some necessary ones as part of major version upgrade. We can just document it in the release notes of the version and provide a few examples of how to resolve the errors if one does run into them. |
I implemented (#58) a simpler version of this feature where every member is initialized in the order of declaration, and This makes the API simpler and easier to understand. It will also make it easier for people to reason about the order of initialization, especially if something in their code depends on this order, like if the I also, think it's not a big deal for people to reorder their members to achieve the desired order of initialization. I'd rather wait for a compelling use case where reordering of members is unacceptable. If someone creates an issue about it, we'll discuss it later. Anyway, it's possible to extend the behavior implemented in #58 without breaking changes if needed. I'll release it as part of the 2.0 release. |
Let me give an example and I will describe the ask as an inline comment.
Then call it as following:
The text was updated successfully, but these errors were encountered: