Allows prevention of alloc dependency through feature and disabling of ValidationError(String) variant.
#302
Conversation
derive_builder_core/src/builder.rs
Outdated
| @@ -231,15 +237,35 @@ impl<'a> ToTokens for Builder<'a> { | |||
| let builder_error_ident = format_ident!("{}Error", builder_ident); | |||
| let builder_error_doc = format!("Error type for {}", builder_ident); | |||
|
|
|||
| let validation_error = self | |||
| .generate_validation_error | |||
| .then_some(quote!( | |||
There was a problem hiding this comment.
This is pretty cool, and I hadn't seen it before. However, if the discussion in #303 does lead to this being on track for a merge, we need to avoid using a method that isn't available to 1.56.0; can you change each use of it to an if block?
There was a problem hiding this comment.
Updated the code then built and ran cargo test using 1.56.0 x86_64.
|
Apologies; I didn’t get a chance to review this today, so it’ll probably be about a week before I next get the chance to. I do intend to review it, and think we are on track towards merging this |
| @@ -95,6 +95,25 @@ pub struct BuildFn { | |||
| /// * If `validate` is specified, then this type must provide a conversion from the specified | |||
| /// function's error type. | |||
| error: Option<Path>, | |||
There was a problem hiding this comment.
I'm wondering if we should make this into a type that can either be a key-value pair with a path, or a list that accepts options about the generated error:
error = "path"
error(validation = false)
The advantage would be that it's then impossible to incorrectly disable the validation variant when using a custom error.
There was a problem hiding this comment.
I managed to implement it so it supports the two forms from your comment.
|
Thank you for the review. No worry about time, I'm in not in a hurry for this feature. I'm looking into the workflow breakage. This PR has somewhat of a breaking change since |
|
My most recent commit addresses the GH workflows. It adds I also added a compile error when |
|
I just pushed a simplified version of the |
I think it's because this PR is on my fork while your code is a branch here. I pulled the commit and just pushed it. Thanks for simplifying the macro code! I had a feeling that there was a nicer way to do it but this was my first time using darling_opt :) |
|
That's totally fine! I'm the author of |
|
Please rebase this; I think that will fix the CI issue. Once we have a clean bill of health from CI, we are ready to merge. |
Fixes code that does not compile for 1.56.0
Avoid custom parsing of meta items.
red1bluelost
force-pushed
the
no-alloc
branch
from
1f6b554
to
8ed6123
Compare
|
Rebased it 👍 |
Currently, using
derive_builderinno_stdstill forces the user to make aglobal_allocatorsince the crate links againstalloc. This PR tries to avoid that in two aways.allocthat controls whether thealloccrate is used. This allowsno_stduser to opt into using allocation if they want it.build_fnfield,validation_error: boolthat controls whether the generated error includesValidationError(String). This prevents usage of anyalloccode but also limits the user from using thevalidatefield.build_fnerrorto avoidalloccode from being used.This is to fix issue: #303.