Config access uses `impl Key` to allow type-safe keys

[bittrance]

This PR changes the API of gix_config::File to use a trait (gix_config::Key) as key when requesting or setting configuration. This will allow gix to use its tree of structured and validating typed keys (gix::config::tree::sections::*) as keys directly, and also means that consumers of the gix crate to use type-safe config access. gix-config also provides an implementation for &str and &bstr so that constant strings can be used as keys. This is part of the work addressing #1125.

The PR is draft, as there remains various question marks, see specific comments in code below.

gix contains a lot of strings and format!() operations that could be changed to use type-safe keys, but that is left for a separate PR.

[bittrance]

…the Key related changes in gix-config are in one commit, whose subject is prefixed with fix!: or feat!: . Please write the subject and body so they can go into the changelog.

Is the purpose simply to have a feat! commit in the history? If so, I'd just split out adding the Key trait into a separate commit, no problem. The current commit looks like it does because I'm always striving for every commit to be viable (i.e. buildable and git-bisect:able) and still be meaningful.

There is also this one big request of only modifying the string_by_key based access with the new trait. After all, the natural way of accessing the API is by passing triplets of section, subsection and 'key' (definitely open to a better term here). That way, only string_by_key() has to change at all and that change would probably be compatible to most existing uses depending on the available implementations of Key for &BStr for example. I don't think there is a need to break the world here, and hope this could even work without making any breaking change.

I was thinking we should keep the API surface small by removing all the _by_key method so that only the impl Key versions remained (I prepared https://github.com/bittrance/gitoxide/commit/c54bb3addfa9e8a2fbcd0fad82bf1f1b3fe62d28 for this purpose). My thought is that gix config API should encourage using type-safe access and I think config.string(format!("foo.{submod}.bar")) is more readable than config.string("foo", submod, "bar"), but I defer to your judgement on when it is worth making a breaking API change.

[Byron]

Is the purpose simply to have a feat! commit in the history? If so, I'd just split out adding the Key trait into a separate commit, no problem. The current commit looks like it does because I'm always striving for every commit to be viable (i.e. buildable and git-bisect:able) and still be meaningful.

No, the purpose is to have a changelog that applies to the gix-config crate exclusively - otherwise cargo smart-release would think there are breaking changes in all of the crates whose code was adjusted, and it will claim new features in their changelogs, too.

That takes priority over each commit being green on CI, it's all trade-offs.

I was thinking we should keep the API surface small by removing all the _by_key method so that only the impl Key versions remained (I prepared bittrance@c54bb3a for this purpose). My thought is that gix config API should encourage using type-safe access and I think config.string(format!("foo.{submod}.bar")) is more readable than config.string("foo", submod, "bar"), but I defer to your judgement on when it is worth making a breaking API change.

Thanks for sharing your thoughts.

For a plumbing crate, it would already be more than fine. However, it's exposed in gix and thus it needs to provide more support for a nicer API, but that shouldn't supersede the 'API that is natural for the data', i.e. triplets as it's without overhead.

But since you would be touching all of these methods anyway, let's encourage the use of the potentially typesafe and nicer API by making their methods the shorter ones. This means, for example, to keep File::string(key: impl Key) as is, but have a longer method that takes the triplet, like File::string_by(section, subsection, key). The _by suffix is an example but something that seems to work nicely, and it seems better than my initial thought _by_triplet() which is punishingly long. Please do feel free to experiment (and bikeshed) with the suffix though. Finally, this also means that string() would call string_by().

I hope all that makes sense, please do let me know about any concern you might have as well.

[Byron]

As a note, I plan to finish this PR myself once I get a chance.

[Byron]

The content of this PR was merged to main, but I forgot to push before changing the commit message, hence GitHub doesn't see the merge happened.

Thanks again for getting this started!