* use new config API in python-cffi config iterator @ 2022-02-11 13:04 David Bremner 2022-02-11 13:04 ` [PATCH 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner 2022-02-11 13:04 ` [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 0 siblings, 2 replies; 9+ messages in thread From: David Bremner @ 2022-02-11 13:04 UTC (permalink / raw) To: notmuch See [1/2] for for the rationale. This obsoletes id:20220210122531.898466-1-david@tethera.net ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] test: known broken test for list(db.config) in python-cffi bindings 2022-02-11 13:04 use new config API in python-cffi config iterator David Bremner @ 2022-02-11 13:04 ` David Bremner 2022-02-11 13:04 ` [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 1 sibling, 0 replies; 9+ messages in thread From: David Bremner @ 2022-02-11 13:04 UTC (permalink / raw) To: notmuch As of notmuch 0.34.2 [1], the python-cffi bindings make available the configuration from both a config file and the database when accessing Database.config like a dictionary. It is therefore confusing that the iterator operations only work on the configuration information stored in the database. [1]: d7f95724132bf658fd151630185899737e2ed829 --- test/T055-path-config.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh index 1df240dd..71823039 100755 --- a/test/T055-path-config.sh +++ b/test/T055-path-config.sh @@ -293,6 +293,27 @@ user.primary_email=test_suite@notmuchmail.org EOF test_expect_equal_file EXPECTED OUTPUT + test_begin_subtest "Config list from python ($config)" + test_subtest_known_broken + test_python <<EOF > OUTPUT +from notmuch2 import Database +db=Database(config=Database.CONFIG.SEARCH) +for key in list(db.config): + print(key) +EOF + cat <<EOF > EXPECTED +database.autocommit +database.backup_dir +database.hook_dir +database.mail_root +database.path +maildir.synchronize_flags +new.tags +user.name +user.other_email +user.primary_email +EOF + test_expect_equal_file EXPECTED OUTPUT case $config in XDG*) test_begin_subtest "Set shadowed config value in database ($config)" -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator 2022-02-11 13:04 use new config API in python-cffi config iterator David Bremner 2022-02-11 13:04 ` [PATCH 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner @ 2022-02-11 13:04 ` David Bremner 2022-02-16 20:52 ` Floris Bruynooghe 1 sibling, 1 reply; 9+ messages in thread From: David Bremner @ 2022-02-11 13:04 UTC (permalink / raw) To: notmuch This returns all of the config keys with non-empty values, not just those that happen to be stored in the database. --- bindings/python-cffi/notmuch2/_build.py | 16 +++++----- bindings/python-cffi/notmuch2/_config.py | 37 +++++++++++++++-------- bindings/python-cffi/tests/test_config.py | 22 ++++++++------ test/T055-path-config.sh | 1 - 4 files changed, 44 insertions(+), 32 deletions(-) diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py index a55b484f..349bb79d 100644 --- a/bindings/python-cffi/notmuch2/_build.py +++ b/bindings/python-cffi/notmuch2/_build.py @@ -97,7 +97,7 @@ ffibuilder.cdef( typedef struct _notmuch_string_map_iterator notmuch_message_properties_t; typedef struct _notmuch_directory notmuch_directory_t; typedef struct _notmuch_filenames notmuch_filenames_t; - typedef struct _notmuch_config_list notmuch_config_list_t; + typedef struct _notmuch_config_pairs notmuch_config_pairs_t; typedef struct _notmuch_indexopts notmuch_indexopts_t; const char * @@ -325,18 +325,18 @@ ffibuilder.cdef( notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value); notmuch_status_t notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value); - notmuch_status_t - notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out); + notmuch_config_pairs_t * + notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix); notmuch_bool_t - notmuch_config_list_valid (notmuch_config_list_t *config_list); + notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list); const char * - notmuch_config_list_key (notmuch_config_list_t *config_list); + notmuch_config_pairs_key (notmuch_config_pairs_t *config_list); const char * - notmuch_config_list_value (notmuch_config_list_t *config_list); + notmuch_config_pairs_value (notmuch_config_pairs_t *config_list); void - notmuch_config_list_move_to_next (notmuch_config_list_t *config_list); + notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list); void - notmuch_config_list_destroy (notmuch_config_list_t *config_list); + notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list); """ ) diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py index 29de6495..d73539c5 100644 --- a/bindings/python-cffi/notmuch2/_config.py +++ b/bindings/python-cffi/notmuch2/_config.py @@ -13,27 +13,39 @@ class ConfigIter(base.NotmuchIter): def __init__(self, parent, iter_p): super().__init__( parent, iter_p, - fn_destroy=capi.lib.notmuch_config_list_destroy, - fn_valid=capi.lib.notmuch_config_list_valid, - fn_get=capi.lib.notmuch_config_list_key, - fn_next=capi.lib.notmuch_config_list_move_to_next) + fn_destroy=capi.lib.notmuch_config_pairs_destroy, + fn_valid=capi.lib.notmuch_config_pairs_valid, + fn_get=capi.lib.notmuch_config_pairs_key, + fn_next=capi.lib.notmuch_config_pairs_move_to_next) def __next__(self): - item = super().__next__() - return base.BinString.from_cffi(item) - + # skip pairs whose value is NULL + while capi.lib.notmuch_config_pairs_valid (super()._iter_p): + val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p) + key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p) + key = base.BinString.from_cffi(key_p) + capi.lib.notmuch_config_pairs_move_to_next (super()._iter_p) + if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "": + return key + self._destroy() + raise StopIteration class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): - """The config key/value pairs stored in the database. + """The config key/value pairs loaded from the database, config file, + and and/or defaults. The entries are exposed as a :class:`collections.abc.MutableMapping` object. Note that setting a value to an empty string is the same as deleting it. + Mutating (deleting or updating values) in the map persists only in + the database, which can be shadowed by config files. + :param parent: the parent object :param ptr_name: the name of the attribute on the parent which will return the memory pointer. This allows this object to access the pointer via the parent's descriptor and thus trigger :class:`MemoryPointer`'s memory safety. + """ def __init__(self, parent, ptr_name): @@ -77,11 +89,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): :raises NullPointerError: If the iterator can not be created. """ - configlist_pp = capi.ffi.new('notmuch_config_list_t**') - ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp) - if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: - raise errors.NotmuchError(ret) - return ConfigIter(self._parent, configlist_pp[0]) + config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'') + if config_pairs_p == capi.ffi.NULL: + raise KeyError + return ConfigIter(self._parent, config_pairs_p) def __len__(self): return sum(1 for t in self) diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py index 67b0dea4..c62aeb98 100644 --- a/bindings/python-cffi/tests/test_config.py +++ b/bindings/python-cffi/tests/test_config.py @@ -34,20 +34,22 @@ class TestIter: print(repr(val)) def test_iter(self, db): - assert list(db.config) == [] - db.config['spam'] = 'ham' - db.config['eggs'] = 'bacon' - assert set(db.config) == {'spam', 'eggs'} - assert set(db.config.keys()) == {'spam', 'eggs'} - assert set(db.config.values()) == {'ham', 'bacon'} - assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')} + import re + prefix = re.compile(r"^TEST[.]") + assert [ x for x in list(db.config) if prefix.match(x) ] == [] + db.config['TEST.spam'] = 'TEST.ham' + db.config['TEST.eggs'] = 'TEST.bacon' + assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'} + assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'} + assert { x for x in set(db.config.values()) if prefix.match (x) } == {'TEST.ham', 'TEST.bacon'} + assert { (x, y) for (x,y) in set(db.config.items()) if prefix.match(x) } == {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')} def test_len(self, db): - assert len(db.config) == 0 + defaults = len(db.config) db.config['spam'] = 'ham' - assert len(db.config) == 1 + assert len(db.config) == defaults + 1 db.config['eggs'] = 'bacon' - assert len(db.config) == 2 + assert len(db.config) == defaults + 2 def test_del(self, db): db.config['spam'] = 'ham' diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh index 71823039..4897c814 100755 --- a/test/T055-path-config.sh +++ b/test/T055-path-config.sh @@ -294,7 +294,6 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "Config list from python ($config)" - test_subtest_known_broken test_python <<EOF > OUTPUT from notmuch2 import Database db=Database(config=Database.CONFIG.SEARCH) -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator 2022-02-11 13:04 ` [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner @ 2022-02-16 20:52 ` Floris Bruynooghe 2022-02-17 2:08 ` David Bremner 0 siblings, 1 reply; 9+ messages in thread From: Floris Bruynooghe @ 2022-02-16 20:52 UTC (permalink / raw) To: David Bremner, notmuch On Fri 11 Feb 2022 at 09:04 -0400, David Bremner wrote: > This returns all of the config keys with non-empty values, not just > those that happen to be stored in the database. > --- > bindings/python-cffi/notmuch2/_build.py | 16 +++++----- > bindings/python-cffi/notmuch2/_config.py | 37 +++++++++++++++-------- > bindings/python-cffi/tests/test_config.py | 22 ++++++++------ > test/T055-path-config.sh | 1 - > 4 files changed, 44 insertions(+), 32 deletions(-) > > diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py > index a55b484f..349bb79d 100644 > --- a/bindings/python-cffi/notmuch2/_build.py > +++ b/bindings/python-cffi/notmuch2/_build.py > @@ -97,7 +97,7 @@ ffibuilder.cdef( > typedef struct _notmuch_string_map_iterator notmuch_message_properties_t; > typedef struct _notmuch_directory notmuch_directory_t; > typedef struct _notmuch_filenames notmuch_filenames_t; > - typedef struct _notmuch_config_list notmuch_config_list_t; > + typedef struct _notmuch_config_pairs notmuch_config_pairs_t; > typedef struct _notmuch_indexopts notmuch_indexopts_t; > > const char * > @@ -325,18 +325,18 @@ ffibuilder.cdef( > notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value); > notmuch_status_t > notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value); > - notmuch_status_t > - notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out); > + notmuch_config_pairs_t * > + notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix); > notmuch_bool_t > - notmuch_config_list_valid (notmuch_config_list_t *config_list); > + notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list); > const char * > - notmuch_config_list_key (notmuch_config_list_t *config_list); > + notmuch_config_pairs_key (notmuch_config_pairs_t *config_list); > const char * > - notmuch_config_list_value (notmuch_config_list_t *config_list); > + notmuch_config_pairs_value (notmuch_config_pairs_t *config_list); > void > - notmuch_config_list_move_to_next (notmuch_config_list_t *config_list); > + notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list); > void > - notmuch_config_list_destroy (notmuch_config_list_t *config_list); > + notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list); > """ > ) > > diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py > index 29de6495..d73539c5 100644 > --- a/bindings/python-cffi/notmuch2/_config.py > +++ b/bindings/python-cffi/notmuch2/_config.py > @@ -13,27 +13,39 @@ class ConfigIter(base.NotmuchIter): > def __init__(self, parent, iter_p): > super().__init__( > parent, iter_p, > - fn_destroy=capi.lib.notmuch_config_list_destroy, > - fn_valid=capi.lib.notmuch_config_list_valid, > - fn_get=capi.lib.notmuch_config_list_key, > - fn_next=capi.lib.notmuch_config_list_move_to_next) > + fn_destroy=capi.lib.notmuch_config_pairs_destroy, > + fn_valid=capi.lib.notmuch_config_pairs_valid, > + fn_get=capi.lib.notmuch_config_pairs_key, > + fn_next=capi.lib.notmuch_config_pairs_move_to_next) > > def __next__(self): > - item = super().__next__() > - return base.BinString.from_cffi(item) > - > + # skip pairs whose value is NULL > + while capi.lib.notmuch_config_pairs_valid (super()._iter_p): FWIW having spaces between the function name and parentheses is rather uncommon for python style. Though of course complaining about style without using an auto-formatter is pretty meh these days :) > + val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p) > + key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p) > + key = base.BinString.from_cffi(key_p) does key_p need a NULL check first or can it never be NULL? > + capi.lib.notmuch_config_pairs_move_to_next (super()._iter_p) > + if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "": > + return key > + self._destroy() > + raise StopIteration > > class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): > - """The config key/value pairs stored in the database. > + """The config key/value pairs loaded from the database, config file, > + and and/or defaults. > > The entries are exposed as a :class:`collections.abc.MutableMapping` object. > Note that setting a value to an empty string is the same as deleting it. > > + Mutating (deleting or updating values) in the map persists only in > + the database, which can be shadowed by config files. > + > :param parent: the parent object > :param ptr_name: the name of the attribute on the parent which will > return the memory pointer. This allows this object to > access the pointer via the parent's descriptor and thus > trigger :class:`MemoryPointer`'s memory safety. > + > """ > > def __init__(self, parent, ptr_name): > @@ -77,11 +89,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): > > :raises NullPointerError: If the iterator can not be created. > """ > - configlist_pp = capi.ffi.new('notmuch_config_list_t**') > - ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp) > - if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: > - raise errors.NotmuchError(ret) > - return ConfigIter(self._parent, configlist_pp[0]) > + config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'') > + if config_pairs_p == capi.ffi.NULL: > + raise KeyError > + return ConfigIter(self._parent, config_pairs_p) > > def __len__(self): > return sum(1 for t in self) > diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py > index 67b0dea4..c62aeb98 100644 > --- a/bindings/python-cffi/tests/test_config.py > +++ b/bindings/python-cffi/tests/test_config.py > @@ -34,20 +34,22 @@ class TestIter: > print(repr(val)) > > def test_iter(self, db): > - assert list(db.config) == [] > - db.config['spam'] = 'ham' > - db.config['eggs'] = 'bacon' > - assert set(db.config) == {'spam', 'eggs'} > - assert set(db.config.keys()) == {'spam', 'eggs'} > - assert set(db.config.values()) == {'ham', 'bacon'} > - assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')} > + import re > + prefix = re.compile(r"^TEST[.]") > + assert [ x for x in list(db.config) if prefix.match(x) ] == [] `x.startswith('TEST.')` is probably lighter weight here, maybe easier to read too that's subjective i guess You can even try something like this to further make it more readable: has_prefix = lambda x: x.startswith('TEST.') > + db.config['TEST.spam'] = 'TEST.ham' > + db.config['TEST.eggs'] = 'TEST.bacon' > + assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'} > + assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'} I'm not sure why you need to wrap `db.config.keys()` in `set()`? This explicitly creates a set out of things before turning it back into an interator while you're fine with the original iterator I think? > + assert { x for x in set(db.config.values()) if prefix.match (x) } == {'TEST.ham', 'TEST.bacon'} > + assert { (x, y) for (x,y) in set(db.config.items()) if prefix.match(x) } == {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')} > > def test_len(self, db): > - assert len(db.config) == 0 > + defaults = len(db.config) > db.config['spam'] = 'ham' > - assert len(db.config) == 1 > + assert len(db.config) == defaults + 1 > db.config['eggs'] = 'bacon' > - assert len(db.config) == 2 > + assert len(db.config) == defaults + 2 > > def test_del(self, db): > db.config['spam'] = 'ham' > diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh > index 71823039..4897c814 100755 > --- a/test/T055-path-config.sh > +++ b/test/T055-path-config.sh > @@ -294,7 +294,6 @@ EOF > test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "Config list from python ($config)" > - test_subtest_known_broken > test_python <<EOF > OUTPUT > from notmuch2 import Database > db=Database(config=Database.CONFIG.SEARCH) ^ permalink raw reply [flat|nested] 9+ messages in thread
* (no subject) 2022-02-16 20:52 ` Floris Bruynooghe @ 2022-02-17 2:08 ` David Bremner 2022-02-17 2:08 ` [PATCH v2 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner 2022-02-17 2:08 ` [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 0 siblings, 2 replies; 9+ messages in thread From: David Bremner @ 2022-02-17 2:08 UTC (permalink / raw) To: Floris Bruynooghe, David Bremner, notmuch Floris wrote: > FWIW having spaces between the function name and parentheses is rather >uncommon for python style. Though of course complaining about style >without using an auto-formatter is pretty meh these days :) > Yeah fair enough, it's the default in the C code, but we pretty clearly have different styles going on in different languages. >> + val_p = capi.lib.notmuch_config_pairs_value (super()._iter_p) >> + key_p = capi.lib.notmuch_config_pairs_key (super()._iter_p) >> + key = base.BinString.from_cffi(key_p) > >does key_p need a NULL check first or can it never be NULL? I think it can never be NULL, but if it is, better to raise an exception I think. >> def test_iter(self, db): >> - assert list(db.config) == [] >> - db.config['spam'] = 'ham' >> - db.config['eggs'] = 'bacon' >> - assert set(db.config) == {'spam', 'eggs'} >> - assert set(db.config.keys()) == {'spam', 'eggs'} >> - assert set(db.config.values()) == {'ham', 'bacon'} >> - assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')} >> + import re >> + prefix = re.compile(r"^TEST[.]") >> + assert [ x for x in list(db.config) if prefix.match(x) ] == [] > >`x.startswith('TEST.')` is probably lighter weight here, maybe easier to >read too that's subjective i guess > >You can even try something like this to further make it more readable: > >has_prefix = lambda x: x.startswith('TEST.') I did variation on this, defining an inner function instead of using a lambda. > >> + db.config['TEST.spam'] = 'TEST.ham' >> + db.config['TEST.eggs'] = 'TEST.bacon' >> + assert { x for x in set(db.config) if prefix.match(x) } == {'TEST.spam', 'TEST.eggs'} >> + assert { x for x in set(db.config.keys()) if prefix.match (x) } == {'TEST.spam', 'TEST.eggs'} > >I'm not sure why you need to wrap `db.config.keys()` in `set()`? This >explicitly creates a set out of things before turning it back into an >interator while you're fine with the original iterator I think? Good question. That seems to apply to list(db.config) above also? ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] test: known broken test for list(db.config) in python-cffi bindings 2022-02-17 2:08 ` David Bremner @ 2022-02-17 2:08 ` David Bremner 2022-02-17 2:08 ` [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 1 sibling, 0 replies; 9+ messages in thread From: David Bremner @ 2022-02-17 2:08 UTC (permalink / raw) To: Floris Bruynooghe, David Bremner, notmuch As of notmuch 0.34.2 [1], the python-cffi bindings make available the configuration from both a config file and the database when accessing Database.config like a dictionary. It is therefore confusing that the iterator operations only work on the configuration information stored in the database. [1]: d7f95724132bf658fd151630185899737e2ed829 --- test/T055-path-config.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh index 1df240dd..71823039 100755 --- a/test/T055-path-config.sh +++ b/test/T055-path-config.sh @@ -293,6 +293,27 @@ user.primary_email=test_suite@notmuchmail.org EOF test_expect_equal_file EXPECTED OUTPUT + test_begin_subtest "Config list from python ($config)" + test_subtest_known_broken + test_python <<EOF > OUTPUT +from notmuch2 import Database +db=Database(config=Database.CONFIG.SEARCH) +for key in list(db.config): + print(key) +EOF + cat <<EOF > EXPECTED +database.autocommit +database.backup_dir +database.hook_dir +database.mail_root +database.path +maildir.synchronize_flags +new.tags +user.name +user.other_email +user.primary_email +EOF + test_expect_equal_file EXPECTED OUTPUT case $config in XDG*) test_begin_subtest "Set shadowed config value in database ($config)" -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator 2022-02-17 2:08 ` David Bremner 2022-02-17 2:08 ` [PATCH v2 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner @ 2022-02-17 2:08 ` David Bremner 2022-02-17 21:30 ` Floris Bruynooghe 1 sibling, 1 reply; 9+ messages in thread From: David Bremner @ 2022-02-17 2:08 UTC (permalink / raw) To: Floris Bruynooghe, David Bremner, notmuch This returns all of the config keys with non-empty values, not just those that happen to be stored in the database. --- bindings/python-cffi/notmuch2/_build.py | 16 ++++----- bindings/python-cffi/notmuch2/_config.py | 40 +++++++++++++++-------- bindings/python-cffi/tests/test_config.py | 24 ++++++++------ test/T055-path-config.sh | 1 - 4 files changed, 49 insertions(+), 32 deletions(-) diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py index a55b484f..349bb79d 100644 --- a/bindings/python-cffi/notmuch2/_build.py +++ b/bindings/python-cffi/notmuch2/_build.py @@ -97,7 +97,7 @@ ffibuilder.cdef( typedef struct _notmuch_string_map_iterator notmuch_message_properties_t; typedef struct _notmuch_directory notmuch_directory_t; typedef struct _notmuch_filenames notmuch_filenames_t; - typedef struct _notmuch_config_list notmuch_config_list_t; + typedef struct _notmuch_config_pairs notmuch_config_pairs_t; typedef struct _notmuch_indexopts notmuch_indexopts_t; const char * @@ -325,18 +325,18 @@ ffibuilder.cdef( notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value); notmuch_status_t notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value); - notmuch_status_t - notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out); + notmuch_config_pairs_t * + notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix); notmuch_bool_t - notmuch_config_list_valid (notmuch_config_list_t *config_list); + notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list); const char * - notmuch_config_list_key (notmuch_config_list_t *config_list); + notmuch_config_pairs_key (notmuch_config_pairs_t *config_list); const char * - notmuch_config_list_value (notmuch_config_list_t *config_list); + notmuch_config_pairs_value (notmuch_config_pairs_t *config_list); void - notmuch_config_list_move_to_next (notmuch_config_list_t *config_list); + notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list); void - notmuch_config_list_destroy (notmuch_config_list_t *config_list); + notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list); """ ) diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py index 29de6495..603fdcbf 100644 --- a/bindings/python-cffi/notmuch2/_config.py +++ b/bindings/python-cffi/notmuch2/_config.py @@ -13,27 +13,42 @@ class ConfigIter(base.NotmuchIter): def __init__(self, parent, iter_p): super().__init__( parent, iter_p, - fn_destroy=capi.lib.notmuch_config_list_destroy, - fn_valid=capi.lib.notmuch_config_list_valid, - fn_get=capi.lib.notmuch_config_list_key, - fn_next=capi.lib.notmuch_config_list_move_to_next) + fn_destroy=capi.lib.notmuch_config_pairs_destroy, + fn_valid=capi.lib.notmuch_config_pairs_valid, + fn_get=capi.lib.notmuch_config_pairs_key, + fn_next=capi.lib.notmuch_config_pairs_move_to_next) def __next__(self): - item = super().__next__() - return base.BinString.from_cffi(item) - + # skip pairs whose value is NULL + while capi.lib.notmuch_config_pairs_valid(super()._iter_p): + val_p = capi.lib.notmuch_config_pairs_value(super()._iter_p) + key_p = capi.lib.notmuch_config_pairs_key(super()._iter_p) + if key_p == capi.ffi.NULL: + # this should never happen + raise errors.NullPointerError + key = base.BinString.from_cffi(key_p) + capi.lib.notmuch_config_pairs_move_to_next(super()._iter_p) + if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "": + return key + self._destroy() + raise StopIteration class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): - """The config key/value pairs stored in the database. + """The config key/value pairs loaded from the database, config file, + and and/or defaults. The entries are exposed as a :class:`collections.abc.MutableMapping` object. Note that setting a value to an empty string is the same as deleting it. + Mutating (deleting or updating values) in the map persists only in + the database, which can be shadowed by config files. + :param parent: the parent object :param ptr_name: the name of the attribute on the parent which will return the memory pointer. This allows this object to access the pointer via the parent's descriptor and thus trigger :class:`MemoryPointer`'s memory safety. + """ def __init__(self, parent, ptr_name): @@ -77,11 +92,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): :raises NullPointerError: If the iterator can not be created. """ - configlist_pp = capi.ffi.new('notmuch_config_list_t**') - ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp) - if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: - raise errors.NotmuchError(ret) - return ConfigIter(self._parent, configlist_pp[0]) + config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'') + if config_pairs_p == capi.ffi.NULL: + raise KeyError + return ConfigIter(self._parent, config_pairs_p) def __len__(self): return sum(1 for t in self) diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py index 67b0dea4..2a7f42f0 100644 --- a/bindings/python-cffi/tests/test_config.py +++ b/bindings/python-cffi/tests/test_config.py @@ -34,20 +34,24 @@ class TestIter: print(repr(val)) def test_iter(self, db): - assert list(db.config) == [] - db.config['spam'] = 'ham' - db.config['eggs'] = 'bacon' - assert set(db.config) == {'spam', 'eggs'} - assert set(db.config.keys()) == {'spam', 'eggs'} - assert set(db.config.values()) == {'ham', 'bacon'} - assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')} + def has_prefix(x): + return x.startswith('TEST.') + + assert [ x for x in db.config if has_prefix(x) ] == [] + db.config['TEST.spam'] = 'TEST.ham' + db.config['TEST.eggs'] = 'TEST.bacon' + assert { x for x in db.config if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'} + assert { x for x in db.config.keys() if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'} + assert { x for x in db.config.values() if has_prefix(x) } == {'TEST.ham', 'TEST.bacon'} + assert { (x, y) for (x,y) in db.config.items() if has_prefix(x) } == \ + {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')} def test_len(self, db): - assert len(db.config) == 0 + defaults = len(db.config) db.config['spam'] = 'ham' - assert len(db.config) == 1 + assert len(db.config) == defaults + 1 db.config['eggs'] = 'bacon' - assert len(db.config) == 2 + assert len(db.config) == defaults + 2 def test_del(self, db): db.config['spam'] = 'ham' diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh index 71823039..4897c814 100755 --- a/test/T055-path-config.sh +++ b/test/T055-path-config.sh @@ -294,7 +294,6 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "Config list from python ($config)" - test_subtest_known_broken test_python <<EOF > OUTPUT from notmuch2 import Database db=Database(config=Database.CONFIG.SEARCH) -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator 2022-02-17 2:08 ` [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner @ 2022-02-17 21:30 ` Floris Bruynooghe 2022-02-18 0:27 ` David Bremner 0 siblings, 1 reply; 9+ messages in thread From: Floris Bruynooghe @ 2022-02-17 21:30 UTC (permalink / raw) To: David Bremner, David Bremner, notmuch lgtm i think :) On Wed 16 Feb 2022 at 22:08 -0400, David Bremner wrote: > This returns all of the config keys with non-empty values, not just > those that happen to be stored in the database. > --- > bindings/python-cffi/notmuch2/_build.py | 16 ++++----- > bindings/python-cffi/notmuch2/_config.py | 40 +++++++++++++++-------- > bindings/python-cffi/tests/test_config.py | 24 ++++++++------ > test/T055-path-config.sh | 1 - > 4 files changed, 49 insertions(+), 32 deletions(-) > > diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py > index a55b484f..349bb79d 100644 > --- a/bindings/python-cffi/notmuch2/_build.py > +++ b/bindings/python-cffi/notmuch2/_build.py > @@ -97,7 +97,7 @@ ffibuilder.cdef( > typedef struct _notmuch_string_map_iterator notmuch_message_properties_t; > typedef struct _notmuch_directory notmuch_directory_t; > typedef struct _notmuch_filenames notmuch_filenames_t; > - typedef struct _notmuch_config_list notmuch_config_list_t; > + typedef struct _notmuch_config_pairs notmuch_config_pairs_t; > typedef struct _notmuch_indexopts notmuch_indexopts_t; > > const char * > @@ -325,18 +325,18 @@ ffibuilder.cdef( > notmuch_database_set_config (notmuch_database_t *db, const char *key, const char *value); > notmuch_status_t > notmuch_database_get_config (notmuch_database_t *db, const char *key, char **value); > - notmuch_status_t > - notmuch_database_get_config_list (notmuch_database_t *db, const char *prefix, notmuch_config_list_t **out); > + notmuch_config_pairs_t * > + notmuch_config_get_pairs (notmuch_database_t *db, const char *prefix); > notmuch_bool_t > - notmuch_config_list_valid (notmuch_config_list_t *config_list); > + notmuch_config_pairs_valid (notmuch_config_pairs_t *config_list); > const char * > - notmuch_config_list_key (notmuch_config_list_t *config_list); > + notmuch_config_pairs_key (notmuch_config_pairs_t *config_list); > const char * > - notmuch_config_list_value (notmuch_config_list_t *config_list); > + notmuch_config_pairs_value (notmuch_config_pairs_t *config_list); > void > - notmuch_config_list_move_to_next (notmuch_config_list_t *config_list); > + notmuch_config_pairs_move_to_next (notmuch_config_pairs_t *config_list); > void > - notmuch_config_list_destroy (notmuch_config_list_t *config_list); > + notmuch_config_pairs_destroy (notmuch_config_pairs_t *config_list); > """ > ) > > diff --git a/bindings/python-cffi/notmuch2/_config.py b/bindings/python-cffi/notmuch2/_config.py > index 29de6495..603fdcbf 100644 > --- a/bindings/python-cffi/notmuch2/_config.py > +++ b/bindings/python-cffi/notmuch2/_config.py > @@ -13,27 +13,42 @@ class ConfigIter(base.NotmuchIter): > def __init__(self, parent, iter_p): > super().__init__( > parent, iter_p, > - fn_destroy=capi.lib.notmuch_config_list_destroy, > - fn_valid=capi.lib.notmuch_config_list_valid, > - fn_get=capi.lib.notmuch_config_list_key, > - fn_next=capi.lib.notmuch_config_list_move_to_next) > + fn_destroy=capi.lib.notmuch_config_pairs_destroy, > + fn_valid=capi.lib.notmuch_config_pairs_valid, > + fn_get=capi.lib.notmuch_config_pairs_key, > + fn_next=capi.lib.notmuch_config_pairs_move_to_next) > > def __next__(self): > - item = super().__next__() > - return base.BinString.from_cffi(item) > - > + # skip pairs whose value is NULL > + while capi.lib.notmuch_config_pairs_valid(super()._iter_p): > + val_p = capi.lib.notmuch_config_pairs_value(super()._iter_p) > + key_p = capi.lib.notmuch_config_pairs_key(super()._iter_p) > + if key_p == capi.ffi.NULL: > + # this should never happen > + raise errors.NullPointerError > + key = base.BinString.from_cffi(key_p) > + capi.lib.notmuch_config_pairs_move_to_next(super()._iter_p) > + if val_p != capi.ffi.NULL and base.BinString.from_cffi(val_p) != "": > + return key > + self._destroy() > + raise StopIteration > > class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): > - """The config key/value pairs stored in the database. > + """The config key/value pairs loaded from the database, config file, > + and and/or defaults. > > The entries are exposed as a :class:`collections.abc.MutableMapping` object. > Note that setting a value to an empty string is the same as deleting it. > > + Mutating (deleting or updating values) in the map persists only in > + the database, which can be shadowed by config files. > + > :param parent: the parent object > :param ptr_name: the name of the attribute on the parent which will > return the memory pointer. This allows this object to > access the pointer via the parent's descriptor and thus > trigger :class:`MemoryPointer`'s memory safety. > + > """ > > def __init__(self, parent, ptr_name): > @@ -77,11 +92,10 @@ class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping): > > :raises NullPointerError: If the iterator can not be created. > """ > - configlist_pp = capi.ffi.new('notmuch_config_list_t**') > - ret = capi.lib.notmuch_database_get_config_list(self._ptr(), b'', configlist_pp) > - if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: > - raise errors.NotmuchError(ret) > - return ConfigIter(self._parent, configlist_pp[0]) > + config_pairs_p = capi.lib.notmuch_config_get_pairs(self._ptr(), b'') > + if config_pairs_p == capi.ffi.NULL: > + raise KeyError > + return ConfigIter(self._parent, config_pairs_p) > > def __len__(self): > return sum(1 for t in self) > diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py > index 67b0dea4..2a7f42f0 100644 > --- a/bindings/python-cffi/tests/test_config.py > +++ b/bindings/python-cffi/tests/test_config.py > @@ -34,20 +34,24 @@ class TestIter: > print(repr(val)) > > def test_iter(self, db): > - assert list(db.config) == [] > - db.config['spam'] = 'ham' > - db.config['eggs'] = 'bacon' > - assert set(db.config) == {'spam', 'eggs'} > - assert set(db.config.keys()) == {'spam', 'eggs'} > - assert set(db.config.values()) == {'ham', 'bacon'} > - assert set(db.config.items()) == {('spam', 'ham'), ('eggs', 'bacon')} > + def has_prefix(x): > + return x.startswith('TEST.') > + > + assert [ x for x in db.config if has_prefix(x) ] == [] > + db.config['TEST.spam'] = 'TEST.ham' > + db.config['TEST.eggs'] = 'TEST.bacon' > + assert { x for x in db.config if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'} > + assert { x for x in db.config.keys() if has_prefix(x) } == {'TEST.spam', 'TEST.eggs'} > + assert { x for x in db.config.values() if has_prefix(x) } == {'TEST.ham', 'TEST.bacon'} > + assert { (x, y) for (x,y) in db.config.items() if has_prefix(x) } == \ > + {('TEST.spam', 'TEST.ham'), ('TEST.eggs', 'TEST.bacon')} > > def test_len(self, db): > - assert len(db.config) == 0 > + defaults = len(db.config) > db.config['spam'] = 'ham' > - assert len(db.config) == 1 > + assert len(db.config) == defaults + 1 > db.config['eggs'] = 'bacon' > - assert len(db.config) == 2 > + assert len(db.config) == defaults + 2 > > def test_del(self, db): > db.config['spam'] = 'ham' > diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh > index 71823039..4897c814 100755 > --- a/test/T055-path-config.sh > +++ b/test/T055-path-config.sh > @@ -294,7 +294,6 @@ EOF > test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "Config list from python ($config)" > - test_subtest_known_broken > test_python <<EOF > OUTPUT > from notmuch2 import Database > db=Database(config=Database.CONFIG.SEARCH) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator 2022-02-17 21:30 ` Floris Bruynooghe @ 2022-02-18 0:27 ` David Bremner 0 siblings, 0 replies; 9+ messages in thread From: David Bremner @ 2022-02-18 0:27 UTC (permalink / raw) To: Floris Bruynooghe, notmuch Floris Bruynooghe <flub@devork.be> writes: > lgtm i think :) Applied to master. d ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-18 0:27 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-11 13:04 use new config API in python-cffi config iterator David Bremner 2022-02-11 13:04 ` [PATCH 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner 2022-02-11 13:04 ` [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 2022-02-16 20:52 ` Floris Bruynooghe 2022-02-17 2:08 ` David Bremner 2022-02-17 2:08 ` [PATCH v2 1/2] test: known broken test for list(db.config) in python-cffi bindings David Bremner 2022-02-17 2:08 ` [PATCH v2 2/2] python-cffi: use config_pairs API in ConfigIterator David Bremner 2022-02-17 21:30 ` Floris Bruynooghe 2022-02-18 0:27 ` David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).