search

pmderodat / ada-toml

TOML parser for Ada
Other
31 stars 5 forks source link

TOML.Merge: recursively merge nested tables #9

Closed mosteo closed 3 years ago

mosteo commented 3 years ago

The existing check on duplicate keys is too strict, as nested tables can be further merged recursively.

pmderodat commented 3 years ago

Hello Alex!

Without more context, I have the feeling that both the previous behavior and the one you introduce could be useful. Could you add a new argument? (for instance Recursive : Boolean := False, for backward compatibility). Also, could you add tests for both cases?

Thank you!

mosteo commented 3 years ago

Will do, stay tuned.

pmderodat commented 3 years ago

Hello @mosteo,

I’ve tried a different approach: add a Merge overloading function that takes a callback to customize conflicts handling (https://github.com/pmderodat/ada-toml/commit/2a671ffb1039a036f2bb68bdc88afc8d3dc68c10): could you check if that’s okay for you?

mosteo commented 3 years ago

Ouch, I had forgotten totally about this one, sorry. OK, I will try that.

pmderodat commented 3 years ago

No problem. :-) However note that I’ve implemented this on master after I’ve updated the library to comply with TOML 1.0.0… which required a little API change! I hope it’s okay for you, I suppose Alire will need to transition, eventually. Please let me know how things go.

mosteo commented 3 years ago

Yup, I was just doing the porting and I realized. Fortunately we weren't relying much on homogeneous arrays.

Our test suite is passing with the new Merge profile so I think it is safe to dismiss this one now.

pmderodat commented 3 years ago

Ok great, thanks! I’ll go ahead and create a new release, then. :-)