feat(git-config): add GitConfig::from_paths

[caspervonb]

This adds GitConfig::from_paths which constructs a config from all the sections of the given files.
Sections are additive so that multi-value semantics are preserved.

Towards #191

[caspervonb]

Intent here is to follow-up with a from_known_paths variant to resolve from typical system paths.

As far as I understood multi-values, the following will be true as it is up to the caller to decide if they want sample all sections or just a single section:

The files are read in the order given above, with last value found taking precedence over values read earlier. When multiple values are taken then all values of a key from all files will be used.

PTAL @edward-shen 🙂

[edward-shen]

Thanks for the PR!

Re: multivar behavior: can you run/add a test that includes sections with the same key? I'd like to document that behavior in our tests.

Honestly that wording from the git-config doc is a little ambiguous. Can you also model a test after the behavior of git config?

[caspervonb]

Honestly that wording from the git-config doc is a little ambiguous.

Yeah it is, I interpreted it as collapsing at first but came around about 15 minutes later.
For git-config really (seems to) just depends on if you call it with --get or --get-all.

It will return all values across sections and files (say ~/.gitconfig and ./git/config) with git-config --get-all.
I'll dig up the implementation and see if there's any obvious caveats.

Did you have anything specific in mind for said test tho @edward-shen? 🙂

[caspervonb]

Digging into it little bit more, seems that git-config calls git_config_from_file repeatedly which just keeps pushing and parsing sections so this seems to be the way to go.

[edward-shen]

Not particularly! I just want to have this behavior documented somewhere in the code, so if there was a difference in behavior from git-config then I just wanted a test to make sure it was noted. Since git-config is doing the same, there's no real reason to add a test (although adding a comment saying this matches the behavior of git-config would be nice while you clean up the potential lints).

If you have a quick sec I'd appreciate fixing the lints before merging, but not going to block you on that if you don't.

Again, thanks for the PR :)

[caspervonb]

Not particularly! I just want to have this behavior documented somewhere in the code, so if there was a difference in behavior from git-config then I just wanted a test to make sure it was noted. Since git-config is doing the same, there's no real reason to add a test (although adding a comment saying this matches the behavior of git-config would be nice while you clean up the potential lints).

If you have a quick sec I'd appreciate fixing the lints before merging, but not going to block you on that if you don't.

Again, thanks for the PR :)

Oops, had assumed CI would catch that.
Amended slightly, will be revising the docs as I go here 😄

[edward-shen]

Let me know if you'd like another review for docs, but otherwise the newly added docs looks good as is.

[Byron]

Thanks a lot, looks great to me, too.