From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id sOkfIxBkDWIdGAAAgWs5BA (envelope-from ) for ; Wed, 16 Feb 2022 21:52:32 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id iKHPHxBkDWIQFwEAauVa8A (envelope-from ) for ; Wed, 16 Feb 2022 21:52:32 +0100 Received: from mail.notmuchmail.org (yantan.tethera.net [IPv6:2a01:4f9:c011:7a79::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 425043DAD5 for ; Wed, 16 Feb 2022 21:52:32 +0100 (CET) Received: from yantan.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id CF8B65F709; Wed, 16 Feb 2022 20:52:29 +0000 (UTC) Received: from mail-ed1-x530.google.com (mail-ed1-x530.google.com [IPv6:2a00:1450:4864:20::530]) by mail.notmuchmail.org (Postfix) with ESMTPS id 91D365F5EB for ; Wed, 16 Feb 2022 20:52:26 +0000 (UTC) Received: by mail-ed1-x530.google.com with SMTP id i11so4385021eda.9 for ; Wed, 16 Feb 2022 12:52:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:from:to:subject:in-reply-to:references:date:message-id :mime-version; bh=GooaU8ygKE/fLx+faSIBqW6zU0OeVjvHMU4tFkqYhFk=; b=mCpivnWeau2KYu9jk0LPD2664yw6xsmlWgOvMr/vJJ8H/2N6fYodBs0loGX2inBMIq s3XovwsXOAu4k3YX1/H1poC4l+viE/Q0tx7atbYtl8BY5To0uWQ9ISD+6InvTZb0H+/w pEfLV/46S1Ml4KPHn+LROWOWmZltZqmv4RjXKRAKKRYxG/utvw7lG/0n5cY2LX7uZCux uikope6KczDCJ5DpidF0cL8AuqN3ttVzW7i0v0rx+CBABiLn5yaSuMFlxRCFboin+VPe MDGvdLR7Zs/LEEioC6AeouDDBTQGyjRBMCUj+LXn41u3pN6lQqxEeODKziCPRNftzZiq g4fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:from:to:subject:in-reply-to:references :date:message-id:mime-version; bh=GooaU8ygKE/fLx+faSIBqW6zU0OeVjvHMU4tFkqYhFk=; b=yO5J4+WrY/X6swpF5WC7m51kg7Uc/xWcVBRrrGK55mqtf2LreMUPlRRjexvzDmLlX9 AQpoq4wCLJX7JKJJVdrDkhA09ztiinc2fEyp3bO8U9XpIh//ASpFy+f5aZldQdLhCP5r AlOFbPaYSnrTG9WEm7yOMns9L9MO8ccj1BuNEuuc8DYCGrjhGYoXh270ob+1rbXNIYCh 05ZRFAql0IGX9zqRSR/q/mBP1qi/n/4GblI3ePjEysxGmyoWldEyjxEEl+ATxKMPHgwQ xjuXBc/Y9oDJQDwRjklXrof/CK5u5anqxntbiNlaJkCe9J1nwRlq+tqhEThG2zscN5V2 t7Kg== X-Gm-Message-State: AOAM531kWn18je7StiuGtAU0BY/Wr/kvow0D0zTdfZNuNoNhEASc3xrV DTpXXLGzBRBu5iSt4PJH8o8= X-Google-Smtp-Source: ABdhPJwIvSWj12Sv0zUntjajjc7BB3cqCy0BW+TEIZ3CQ2UvRP4BXxg/FCEPdgjkRxeu25W8NtdVcw== X-Received: by 2002:aa7:c041:0:b0:410:ee83:3c91 with SMTP id k1-20020aa7c041000000b00410ee833c91mr5007337edo.407.1645044745883; Wed, 16 Feb 2022 12:52:25 -0800 (PST) Received: from powell.devork.be (62-178-226-148.cable.dynamic.surfer.at. [62.178.226.148]) by smtp.gmail.com with ESMTPSA id v5sm2288672edb.15.2022.02.16.12.52.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Feb 2022 12:52:25 -0800 (PST) Sender: Floris Bruynooghe Received: (nullmailer pid 235800 invoked by uid 1000); Wed, 16 Feb 2022 20:52:24 -0000 From: Floris Bruynooghe To: David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH 2/2] python-cffi: use config_pairs API in ConfigIterator In-Reply-To: <20220211130412.1416990-3-david@tethera.net> References: <20220211130412.1416990-1-david@tethera.net> <20220211130412.1416990-3-david@tethera.net> Date: Wed, 16 Feb 2022 21:52:24 +0100 Message-ID: <8735kipm9j.fsf@powell.devork.be> MIME-Version: 1.0 Message-ID-Hash: 6576C7YLBWIEZP2VOJJ5HCQHGFFGZJAH X-Message-ID-Hash: 6576C7YLBWIEZP2VOJJ5HCQHGFFGZJAH X-MailFrom: floris.bruynooghe@gmail.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-notmuch.notmuchmail.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.3 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_IN X-Migadu-Country: DE ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1645044752; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-owner:list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=Z7NrjLXlcaYdYvfhlSBTum5kSumzg8jn/tJgZXRYPyI=; b=oNyPVGd37qMIsK45o0HCUMfWtk7O0Oq6XEiZYBJnc75miPuXXXA69aQ7qyRdUO56/DW2uj YMSJkPs7pdWGu3BXPh1BS5PCEmL40IfQ4oZvPKsaYOaIuzQDYgUmHglQ7gGckis4+maRt9 BHgl0xivIaCweNhJdjhmT/7B2ZMoAhWNb0IvrOdNkHq0TzKgKEiTzMeA74F0VO5vJF0ntx GEeajs9vsZh6VbjDgPilzPYHPyVc54LRRmOWlTHvWK3X6jxsQVfQwJl/kzhKK7r1MVgEmy 1PtCt3YGgBqJIwkJ0XO+lUjkPRUPp5qCd0I7Jf7Yss8q5389cwb4QuVzTG5RGA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1645044752; a=rsa-sha256; cv=none; b=aP5x85d6/tvOZSUpTOIvOYQVsioSEUynDsFuOS6PrzE3kPU+4MDDixTH1tOKWMprVHlppc r1nlswr6wte7NJ4vHXeYlShf+xRrmhv4qLgTwuzIkpDZSirCwNn1LPo3C0bAaz6n1y81lq r/2nMTpPFrfq9+pLMQl86jv2vLlQfBZHuLY1DMaaNuCoSj6HoOS8ojxm9DEJywHAiDvLCy 5lIpQGddioZP4NG7NcTwOGMtG331qKdnGHa8qOrsL4YmkeuSzn5tgpuzIqQwL004BA739G 2FTZ3Brg8t+ovcitOgOOgVDg/NDNwQjtf71FjX/fCjiQXbNvqPQjPX+DVhethQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=gmail.com header.s=20210112 header.b=mCpivnWe; dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 2a01:4f9:c011:7a79::1 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Spam-Score: -2.35 Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=gmail.com header.s=20210112 header.b=mCpivnWe; dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 2a01:4f9:c011:7a79::1 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Queue-Id: 425043DAD5 X-Spam-Score: -2.35 X-Migadu-Scanner: scn0.migadu.com X-TUID: xGtB+7oqWyRe 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 < OUTPUT > from notmuch2 import Database > db=Database(config=Database.CONFIG.SEARCH)