Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Config.__iter__ to iterate over ConfigEntry objects #778

Merged

Conversation

thecjharries
Copy link
Contributor

@thecjharries thecjharries commented Mar 17, 2018

This PR supersedes #776.

Summary

Config.__iter__ iterates over a list of ConfigEntry objects instead of a list of names, matching the behavior of git_config_iterator.

Changes

ConfigEntry

  • Added a new staticmethod, decode_as_string, to simplify all the ffi.string(value).decode('utf-8') calls.
  • Added a new flag to _from_c, from_iterator=False, to track the origin of the entry.
  • Changed __del__ behavior to only git_config_entry_free ConfigEntrys created from git_config_get_entry. Attempting to free entries while iterating results in a seg fault. (If anyone can explain that to me, I'm curious. My C is pretty weak.)
  • Exposed level, the git_config_level_t value. (It looks like pygit2 has a shortened list; I didn't play with this too much because that's a different feature.)
  • Exposed name, the name of the entry, as a string (using decode_as_string).
  • Exposed value_string, a convenience accessor for decode_as_string(self.value).

All of these changes make it possible to create a ConfigEntry both from git_config_next and from git_config_get_entry. They expose some useful properties (level, name) which make it easier to track what an entry is and where it came from. value is the raw value that must be interpreted; value_string is conveniently a string that can be used immediately.

ConfigIterator

ConfigMultivarIterator

  • __next__ returns entry.value_string instead of calling ffi.string(entry.value).decode('utf-8')

Config

  • __getitem__ returns entry.value_string instead of calling ffi.string(entry.value).decode('utf-8')

Tests

test_iterator was refactored to check that each entry's level > -1 (again, pygit2 seems to have a reduced set of items; this seemed like a safe check). It still checks that core.bare is in the list of entries and that it is set, which is what the original tests did.

ConfigEntry didn't gain any tests. My knowledge of cffi is pretty limited so I'm not quite sure how to set up mocks for that.

Docs

  • Updated Config.__iter__ description
  • Added ConfigEntry section

Example

from __future__ import print_function

from tempfile import NamedTemporaryFile

from pygit2 import Config, GIT_CONFIG_LEVEL_GLOBAL, GIT_CONFIG_LEVEL_LOCAL

blank = Config()

with NamedTemporaryFile() as config_file:
    config_file.write("[push]\n\tdefault = %s" % 'simple')
    config_file.seek(0)
    blank.add_file(config_file.name, GIT_CONFIG_LEVEL_GLOBAL)

with NamedTemporaryFile() as config_file:
    config_file.write("[push]\n\tdefault = %s\n[core]\n\tbare = no" % 'matching')
    config_file.seek(0)
    blank.add_file(config_file.name, GIT_CONFIG_LEVEL_LOCAL)

for entry in blank:
    print(entry.name, entry.value_string, entry.level)

# push.default simple 4
# push.default matching 5
# core.bare no 5

print(blank['push.default'])

# matching

print(blank['core.bare'])

# no

print(blank.get_bool('core.bare'))

# False

@jdavid
Copy link
Member

jdavid commented Mar 18, 2018

Thanks. Could you please squash the commits to simplify reviewing.

Also I think .value should return the string, then have something like .raw_value. I think most people are not interested in the raw value, and a string is what they want most of the time.

@thecjharries thecjharries force-pushed the feature/return-config-entry-from-iter branch from 5a4203b to e0e89bc Compare March 18, 2018 13:34
@thecjharries
Copy link
Contributor Author

I had the same thought when writing it. The raw value is used a couple of times in Config, which made me wary about changing it. The tests ran just fine locally, so it doesn't look like anything else was depending on that.

  • I squashed everything to a single commit (I actually didn't know how to do that until today)
  • ConfigEntry.value is now the decoded string value
  • ConfigEntry.raw_value is the raw C value, which is used for get_bool and get_int

@thecjharries
Copy link
Contributor Author

It looks like I squashed too much. I'll have that fixed in a few minutes. Sorry about that.

commit f618e30b21401aca34f77c7b7e05c710528697ae
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Sun Mar 18 08:23:37 2018 -0500

    Pass decoded value via valua and raw value via raw_value

commit 5a4203b
Merge: 8ca94fc 0c1f2d8
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:08:12 2018 -0500

    Merge branch 'feature/update-docs' into feature/return-config-entry-from-iter

commit 0c1f2d8
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:06:51 2018 -0500

    Add ConfigEntry docs

commit 67701ed
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:05:59 2018 -0500

    Update __iter__ docs

commit cdd9566
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:04:24 2018 -0500

    Document properties

commit 8f74752
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:02:51 2018 -0500

    Document decode_as_string

commit f938b49
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 21:01:59 2018 -0500

    Document _from_c

commit 8ca94fc
Merge: 835e711 52de16a
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 20:15:49 2018 -0500

    Merge branch 'feature/iterate-over-config-entries' into feature/return-config-entry-from-iter

commit 52de16a
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 20:15:08 2018 -0500

    Update test_iterator to consume ConfigEntr[ies]

commit e7798bb
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 20:03:39 2018 -0500

    Return entry, a ConfigEntry from ConfigIterator.__next__

commit 835e711
Merge: bbcb464 55604ca
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 20:02:45 2018 -0500

    Merge branch 'feature/decode-name' into feature/return-config-entry-from-iter

commit 55604ca
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:57:25 2018 -0500

    Return properly decoded entry.name

commit 44326fa
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:57:06 2018 -0500

    Decode ConfigEntry name without prompting

commit bbcb464
Merge: 74544d6 1c47ece
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:56:29 2018 -0500

    Merge branch 'feature/decode-value' into feature/return-config-entry-from-iter

commit 1c47ece
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:54:45 2018 -0500

    Return entry.value_string from __getitem__

commit cb873be
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:52:14 2018 -0500

    Return entry.value_string from ConfigMultivarIterator

commit 2e14065
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:51:53 2018 -0500

    Add value_string property, decode_as_string(entry.value)

commit 9d10675
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:51:13 2018 -0500

    Add decode_as_string method

commit 74544d6
Merge: 3d76e44 ae6eead
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:28:08 2018 -0500

    Merge branch 'feature/expose-entry-properties' into feature/return-config-entry-from-iter

commit ae6eead
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:26:07 2018 -0500

    Return simplified name from __next__

commit 6ca5789
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:25:11 2018 -0500

    Add name property

commit c9fdd27
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:24:47 2018 -0500

    Add level property

commit 3d76e44
Merge: 0f64ac4 f471b6d
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:23:54 2018 -0500

    Merge branch 'feature/return-config-entry-name-from-iter' into feature/return-config-entry-from-iter

commit f471b6d
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:20:30 2018 -0500

    Expose the proper name (entry._entry.name)

commit 6ef0abb
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:20:17 2018 -0500

    Return a ConfigEntry from _next_entry

commit f48d0d8
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:19:27 2018 -0500

    Only free the entry if it did not come from an iterator

commit df0e8b6
Author: CJ Harries <cj@wizardsoftheweb.pro>
Date:   Fri Mar 16 19:19:00 2018 -0500

    Add from_iterator creation flag
@thecjharries thecjharries force-pushed the feature/return-config-entry-from-iter branch from e0e89bc to 5026f9c Compare March 18, 2018 13:45
@jdavid jdavid merged commit 5026f9c into libgit2:master Mar 18, 2018
@jdavid
Copy link
Member

jdavid commented Mar 18, 2018

Good job, thanks!

@thecjharries thecjharries deleted the feature/return-config-entry-from-iter branch March 18, 2018 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants