update toml library to v2

[GreyXor]

I've revised the TOML parser to make it compatible with the v2 of go-toml.

It appears that this updated version parses differently; specifically, it no longer includes spaces in subgroups and only returns nil if the map is empty(look at the tests to see the difference).

Please tell me if there are any additions or modifications required.

[knadh]

Thatnks @GreyXor, but if the parses have different behaviour and the existing tests fail, that'll break backwards compatibility. If that's the case, then the parsers/toml package should be versioned parsers/toml/v2. Can you confirm whether it is indeed backwards incompatible?

[GreyXor]

Thatnks @GreyXor, but if the parses have different behaviour and the existing tests fail, that'll break backwards compatibility. If that's the case, then the parsers/toml package should be versioned parsers/toml/v2. Can you confirm whether it is indeed backwards incompatible?

Thanks, I can confirm that this breaks backward compatibility. Should I move it into a subdirectory called v2 ?

[knadh]

Yep, in that case, it should be /v2. In your PR, you should update parsers/toml/go.mod to have the v2 suffix (module github.com/knadh/koanf/parsers/toml/v2).

koanf/parsers/toml/go.mod

Line 1 in 6c6c498

module github.com/knadh/koanf/parsers/toml

[GreyXor]

Yep, in that case, it should be /v2. In your PR, you should update parsers/toml/go.mod to have the v2 suffix (module github.com/knadh/koanf/parsers/toml/v2).

koanf/parsers/toml/go.mod

Line 1 in 6c6c498

module github.com/knadh/koanf/parsers/toml

Here it is, toml/v2 in a subdirectory

[knadh]

There should be no v2 subdirectory. Go versioning works based on the paths defined in go.mod and not directory structures. Please refer to this: https://github.com/knadh/koanf/tree/master/providers/consul

You should make the necessary changes to the toml provider and simply upgrade the version path in its go.mod to have the /v2 suffix. Once it's merged, I'll push a tag that makes it go gettable.

[GreyXor]

I understood that:
We want to keep both versions of the TOML parser, the current one for compatibility needs, and a new "v2", mine. To do this, I:

• Added v2 to the go.mod file (I did this here)
• Moved v2 to a subfolder (for proper organization)

If a v2 subfolder doesn't suit you, where should I put it?

[knadh]

The current version already exists in the repo with a specific tag. You update the files to the new version and add /v2 and I will push a v2 tag for your commit. Thus, both versions will co exist in the repository's git history.

In the directory structure, there should also be the latest "version" (state) of the file. There shouldn't be subdirectories with different versions.

[GreyXor]

Thanks, I've deleted the v2 subfolder and parser/toml is now the v2.
@knadh, it's all good for you?

[knadh]

Thanks @GreyXor. Will review and merge by this weekend.

[rhnvrm]

Also, let's update the following:

• examples/read-commandline/main.go which uses the v1 package; to the /v2 version.
• Add /v2 to README
• Add a test to tests package

[GreyXor]

Also, let's update the following:

* `examples/read-commandline/main.go` which uses the v1 package; to the /v2 version.

* Add /v2 to README

* Add a test to `tests` package

Thank you, i've done the two first bullets points from your list. Would you like a complete new test in a new file ? Or just a test directly in koanf_test.go ?

[rhnvrm]

Thank you, i've done the two first bullets points from your list. Would you like a complete new test in a new file ? Or just a test directly in koanf_test.go ?

https://github.com/knadh/koanf/blob/master/tests/koanf_test.go#L282-L288

Maybe we can just add a new case with tomlv2 as the parser?

import(
        ...
	tomlv2 "github.com/knadh/koanf/parsers/toml/v2"
        ...
)

[knadh]

I've added a fix for the go.work and tests not running in the workflow. Can you rebase your PR on master?

Once this is done, we can go ahead and merge.

[GreyXor]

I've added a fix for the go.work and tests not running in the workflow. Can you rebase your PR on master?

Once this is done, we can go ahead and merge.

Thanks, I rebased on master

Resolved

[knadh]

This is merged in #286. Thanks @GreyXor. Had to create a new PR out of your commits as I'd one minor change to make which couldn't be pushed to your PR (as it's from your master and not a feature branch).

[GreyXor]

This is merged in #286. Thanks @GreyXor. Had to create a new PR out of your commits as I'd one minor change to make which couldn't be pushed to your PR (as it's from your master and not a feature branch).

Thanks 👍
I understand, next time i'll push in a feature/foo branch.