Update Config.__iter__ to match the documentation #776
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The
Config.__iter__
docs say the method iterates over all entries, where "[e]ach element is a tuple containing the name and the value", which could be problematic as the method "may return multiple versions of each entry if they are set multiple times." However, the (final)next
method on the iterator only returns thename
of the entry. There's a very good chance it's never returned a tuple, evidenced by theblame
.Example
Pull Request
I updated
__next__
's return to also include the value. This matches what the docs describe, returning a tuple of name and value. Since the change only affects the iterator, the default value return isn't affected.I also updated the test to consume the tuple. There's probably more that could go there, but I'm not quite sure what yet.
libgit2
I think
libgit2
supports my interpretation of things.git_config_iterator
iterates overgit_config_entry
, which contains both the name and the value (as well as some other information that might be useful to use).Extension
Looking at my example above, it doesn't seem like
(key, value)
is all that useful. It might be a good idea to also returnlevel
. It's ingit_config_entry
, so it's available to the iterator.Alternative
As I pointed out, I don't think this has worked as intended for years. An equally simple solution would be to update the docs to match the current behavior. I'd be more than happy to do that if this PR is rejected.