search

knadh / koanf

Simple, extremely lightweight, extensible, configuration management library for Go. Support for JSON, TOML, YAML, env, command line, file, S3 etc. Alternative to viper.
MIT License
2.71k stars 150 forks source link

Parser: KDL #266

Closed dezren39 closed 9 months ago

knadh commented 9 months ago

Thanks for the PR @dezren39. Could you move kdl references and tests from tests/koanf_test.go? All the tests can be self-contained in kdl/kdl_test.go.

dezren39 commented 9 months ago

I can! I wanted to clarify, should I not add a KDL case to koanf_test? The main reason I have changes there is because kdl-go treats empty kdl node values as 'nil' instead of an empty map, and so if I add the KDL case, I need to handle the tests involving 'empty' slightly differently. I did raise an issue in the kdl-go library confirming whether this behavior was intended and noting that it differs from other libraries somewhat. So, I treated the behavior as an intended difference in the data specs between the formats. I tried my best to generically add the 'emptyIsNil' concept so that kdl isn't being directly referenced in tests. I'm open to solving this however works best for you. What do you think?

knadh commented 9 months ago

The main reason I have changes there is because kdl-go treats empty kdl node values as 'nil' instead of an empty map.

This can be handled in the provider. You can iterate, type-check, and set it to an empty map.

I can! I wanted to clarify, should I not add a KDL case to koanf_test?

That's not necessary. All the KDL tests can be in the KDL package.

dezren39 commented 9 months ago

that makes things much easier! i've removed the changes in tests now. I'll look about the kdl tests within the provider I had mostly focused on ensuring the existing tests worked. 😅

dezren39 commented 9 months ago

ok i've cleaned up and fixed the tests. :-)