v

v copied to clipboard

PR adds direct encoding and decoding in toml.

Let me know about considerations and nitpicks to make this good as it should be.

Imho the support for custom .from_toml and .to_toml methods should stay - if they are defined, they should be tried first, and if they are not, the toml module could fall back to the automatic behavior.

Yep, that would give most power to the user and ensure backwards compatibility. Would prorbaly make this PR mergable earlier as well. I'll look into it. Thanks @spytheman.

I have looked into this a bit. Calling these methods when they are defined, but keeping them optional to allow automatic handling adds some complexity. The only way I can see doing this is to go via ast. But I'm still new to V. So maybe(and hopefully) there is a simple way, that I'm just not aware of, to fall back to automatic handling when .from_toml / .to_toml is not present.

Please let me know if there is something or if I should go the ast way.

To throw in some thoughts: Is a customized decoding/encoding inside the modules toml.decode / toml.enocode even a desired thing? It would still be possible after all. Users could define toml maps to their liking, which would make it more like json2 raw_decode instead of having it integrated in the toml modules methods.

The only disadvantage I see in using automated handling as default are potential interference with methods like from_toml that contain advanced login. For those, a user would need to not call them via toml.decode.

This also reads like decoding with a mandatory struct method was meant to be transient:

// Currently encode expects the method .to_toml() exists on T.

... Also remember that TOML is not a format for serializing data. It's a config file format. Nothing more nothing less.

As an example; It'll be impossible to know what format to decode to. Does the user want:

[foo]
  bar = "moo"

or

foo.bar = "koo"

I've always hoped for stronger compiletime support in the module. I haven't had time to checkout the PR locally

Imho the support for custom .from_toml and .to_toml methods should stay - if they are defined, they should be tried first, and if they are not, the toml module could fall back to the automatic behavior.

I fully agree with this.

Imho the support for custom .from_toml and .to_toml methods should stay - if they are defined, they should be tried first, and if they are not, the toml module could fall back to the automatic behavior.

Should work now. 🙂👍

I'll dig into nested struct handling and also wanted to check if there are good ways to go about the different time format toml types. Please let me know if there are other things.

Nothing is calling .from_toml at all anymore in 6d535ee .

To much wine yesterday. Thanks @spytheman. I updated the method test to include custom encoding logic to cause failiure if the methods won't be used.

As an example; It'll be impossible to know what format to decode to. Does the user want:

Agreed @Larpon. I won't try to crack the nut of satisfying custom serialization needs. While automated encoding is a nice to have, especially when formatting doesn't matter, I'd say having deserialization automated can add quality of life for all users of the toml module.

Regarding nested structs: Encoding should work already. For decoding, one of you might can give me the missing link.

Using the decode_struct fn will fail to infer the type. (PR changes #L51)

} $else $if field.is_struct {
	// typ.$(field.name) = decode_struct(doc.value(field.name))

So I'm struggling to get it work for e.g., something like:

struct Person {
	name    string
	address Address
}

struct Address {
	street string
	city   string
}
toml_str := 'name = "John"
address = { street = "Elm Street", city = "Springwood" }'
decode[Person](toml_str)

Is it possible to help decode_struct to infer that type or to pass the nested type as generic to have it resolved in: decode_struct[Address](doc.value(field.name))?

@Larpon en-/decoding of nested structs and arrays was added.

Now it already got hacky. So if a moment in your schedule frees up, I'd like to know your opinion on how to proceed and if/where things can be done better to satisfy the projects needs.

Things that seem in need of a fix:

• [x] Parsing float arrays~~ #18058.
• [x] Parsing bool arrays doesn't seem to work as well.~~ #18068
• [ ] Generic struct access at comptime #18110
• Probably not a blocker: In the encode function: I couldn't find a way to use the array map method an get the correct values. #18069

Is it possible to help decode_struct to infer that type or to pass the nested type as generic to have it resolved in: decode_struct[Address](doc.value(field.name))?

I think it's something that needs to be improved in the compiler.

However the PR in general looks good to me now. @spytheman what do you think?

Thanks for the review. I think I also had some improvements in mind. I will have another look at it tomorrow as well 👍 Have a good evening!

When there is no need to wait until all dependent issues are resolved this one might be ready for a merge. I think it can be of use in it's current state (I'm actually currently encountering such a situation). We could add further improvements in a new clean PR when the other necessities are implemented.

Since the last review I'm just rebasing this on master and adding the problem with multiple nested struct as comments.