eww
eww copied to clipboard
Support empty string for safe access operator Currently only an empty json string, i.e. a string in a string was supported: `'""'`.
Please follow this template, if applicable.
Description
Fixes ?.
for empty strings
Usage
{""?.test == "null"}
Additional Notes
TBD: should a json string i.e. '""'
be considered null
?
Checklist
Please make sure you can check all the boxes that apply to this PR.
- [ ] I added my changes to CHANGELOG.md, if appropriate.
- [x] I used
cargo fmt
to automatically format all code before committing
What was the error you were seeing when indexing an empty string?
What was the error you were seeing when indexing an empty string?
@oldwomanjosiah
When I add the test to master I get:
thread 'eval::tests::safe_access_to_empty' panicked at 'Output was not Ok(_): ConversionError(ConversionError { value: "", target_type: "json-value", source: Some(Error("EOF while parsing a value", line: 1, column: 0)) })', crates/simplexpr/src/eval.rs:392:5
I'm not sure if this is actually desirable behavior, as empty strings will be valid values in many situations -- having a differentiation between empty strings and null does seem quite useful. However, I guess it also depends on how empty strings are treated elsewhere, which to be honest, I'm not 100% sure at this point -- and it miiiiiight be useful to then provide some easier way to convert empty strings into null... easier than (foo == "") ? "null" : foo
I'm not sure if this is actually desirable behavior, as empty strings will be valid values in many situations
yes, but ""?.test
would be an error otherwise so this is not breaking anything.
the only situation where behavior wouldn't be unambiguous would be ""?:
.
I think we should only make an empty string ""
and not an empty json string '""'
null.
If you use a ?
operator you actively decide to use the value as json, so as long as it is documented it is fine to add this behavior.
That i'd be more fine with, sure! Although this behavior should probably be documented very well