eww icon indicating copy to clipboard operation
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: `'""'`.

Open ModProg opened this issue 2 years ago • 6 comments

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

ModProg avatar Nov 27 '22 11:11 ModProg

What was the error you were seeing when indexing an empty string?

oldwomanjosiah avatar Dec 12 '22 20:12 oldwomanjosiah

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

ModProg avatar Dec 12 '22 21:12 ModProg

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

elkowar avatar Feb 25 '23 09:02 elkowar

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 ""?:.

ModProg avatar Feb 25 '23 10:02 ModProg

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.

ModProg avatar Feb 25 '23 10:02 ModProg

That i'd be more fine with, sure! Although this behavior should probably be documented very well

elkowar avatar Feb 25 '23 12:02 elkowar