Update deserialization logic + add principles. #147
Conversation
|
|
||
| // errors if numbers are too larger | ||
| deserialize_test_err::<Query<u8>>("a[1000]=2", "number too large to fit in target type"); | ||
| deserialize_test_err::<Query<u8>>("a[1000]=2", "invalid type: string \"1000\", expected u8"); |
There was a problem hiding this comment.
I slightly prefer this error
| deserialize_test_err::<Query>("vec[]=a&vec[]=2", "invalid digit found in string"); | ||
| deserialize_test_err::<Query>( | ||
| "vec[]=a&vec[]=2", | ||
| "invalid type: string \"a\", expected u32", |
There was a problem hiding this comment.
this is a much better error
| ParsedValue::String(s) => s, | ||
| ParsedValue::Sequence(mut seq) => get_last_string_value(&mut seq)?, | ||
| ParsedValue::Sequence(mut seq) => { | ||
| if let Some(v) = get_last_string_value(&mut seq) { |
There was a problem hiding this comment.
I am not sure about this approach, is it common to represent an string as an array?
I suppose this might be when you have the value specified multiple times but you are only asking for a singular value?
Is it desired to silently ignore extra values?
On another note, this function mutates the array, so calling it removes the last element regardless if it returns Some or None. So if we then forward the value to the visit_seq below it seems that we would be missing the last value.
There was a problem hiding this comment.
I suppose this might be when you have the value specified multiple times but you are only asking for a singular value?
Yup this is exactly is. This is to match the use case in #68 which is a fairly common web querystring use case. We could make this be configurable thogh.
On another note, this function mutates the array, so calling it removes the last element regardless if it returns Some or None. So if we then forward the value to the visit_seq below it seems that we would be missing the last value.
Good catch!
| //! | ||
| //! This intermediate structure is then deserialized into the target Rust types. | ||
| //! | ||
| //! ## Principles |
There was a problem hiding this comment.
Perhaps we should also link to #146
No description provided.