allow any string-like value #177
Conversation
|
It's not really going to cover your usecase if you have an Option for example. Maybe we just look if |
cortopy
force-pushed
the
less-strict-string-validation
branch
from
c8e44c2
to
e8253db
Compare
|
@Keats you were totally right. As I carried on adding validation I stumbled upon more cases. I've added changes for While doing this I realised that the main issue is that prost supports I hope this PR helps with widening compatibility |
| @@ -113,7 +113,7 @@ pub fn assert_has_len(field_name: String, type_name: &str, field_type: &syn::Typ | |||
| return; | |||
| } | |||
|
|
|||
| if type_name != "String" | |||
| if !type_name.ends_with("String") | |||
| && !type_name.starts_with("Vec<") | |||
| && !type_name.starts_with("Option<Vec<") | |||
| && !type_name.starts_with("Option<Option<Vec<") | |||
There was a problem hiding this comment.
should we just search for String and str rather than this ever-expanding list of possibilities? Worst case, it will be a compile time error if they tried to use it on a type it can't be used but that can be documented in the README.
There was a problem hiding this comment.
it does sound like a sensible approach to me. The validation error feels like a very nice message for easier development, but if it's getting too much on the way then relaxing it would help too.
I've made the change to simplify this as much as possible. There are still Option<Option< checks for other types but they don't affect the case of strings
There was a problem hiding this comment.
We should keep the same logic for the length validator below though
There was a problem hiding this comment.
(apologies for the delayed response. I was caught with a few things and this slipped through! )
Thanks for that. I just pushed the recommended changes
cortopy
force-pushed
the
less-strict-string-validation
branch
from
e8253db
to
5d13af7
Compare
cortopy
force-pushed
the
less-strict-string-validation
branch
from
5d13af7
to
0380ae7
Compare
As per #176, I've tried to relax the validation a bit. It does work for prost Strings. Would this be acceptable?