search

kinnison / marked-data

A YAML representation library which explicitly retains provenance data
MIT License
12 stars 5 forks source link

Some types are lost #13

Closed wolfv closed 2 months ago

wolfv commented 3 months ago

I think in marked-yaml, some types are currently "lost".

For example, the following:

foo: 5
# or
foo_str: "5"  # (the `"` are stripped in the YAML parsing)

are different types in YAML. However, in marked-yaml they end up being the same (ScalarNode). The conversion from foo_str.as_i64 works fine - which IMO it shouldn't since it's a "forced" string.

Not sure if that's something you'd consider. It would mean we have to track the types as they come from the Yaml loader.

kinnison commented 3 months ago

Hmm, we could track the TScalarStyle and mark if a scalar node is a forced string or not (plain == not, otherwise == forced I guess)

That might (a) be a good way to do it and (b) well worth doing - opinions?

wolfv commented 3 months ago

I like it! We could then pretty easily fail the conversion to integer/float/bool when there was anything other than Plain.

kinnison commented 3 months ago

We'd need to track that in the Deserializer implementations too though, since that supports conversions using FromStr

wolfv commented 3 months ago

Although there is one more problem. We might want to constrain the input types (e.g. not accept floats, ever).

That would still be problematic (without having to parse the string again).

kinnison commented 3 months ago

Given this would be a breaking change, we should try and decide all the niggles at once

Also we could roll #12 into the change.

kinnison commented 3 months ago

(I'm thinking we'd need a LoaderOptions boolean for whether or not to honour forced strings

kinnison commented 3 months ago

I've put a prototype of this onto main. I think that this is the approximately right answer. If you want us to expose if the scalar kind was plain or not as well then that'd be easy to add.

kinnison commented 2 months ago

@wolfv When you have a chance, could you look at what I've put on the main branch and tell me (a) if you think it looks plausible based on the discusison above, and (b) if you also want scalars to expose if they're forced to be plain strings?

kinnison commented 2 months ago

If what's on main looks good, I'd like to make a release.

/ping @wolfv

wolfv commented 2 months ago

I just checked it out, looks good! I'm happy to update once the release is out and will report back if I find issues.

kinnison commented 2 months ago

@wolfv Sorry for the delay, I've been unwell. 0.6.0 published.

wolfv commented 2 months ago

Get better soon :)

Thanks!

kinnison commented 2 months ago

@wolfv I just pushed 0.6.1 because I managed to miss a dbg!() call which would have made your output very messy.

wolfv commented 2 months ago

:) 🚀