Re: [PATCH] emacs: split-window-sensibly in tree mode with open message
Daniel Kahn Gillmor writes: > I like this proposal, and the simplification that it gives to the > notmuch-emacs codebase. However, this thread is the first place i've > learned about split-window-sensibly, so i'm probably not eligible to > really judge the merits here. For some historical context, split-window-sensibly was introduced in 2009 [1] and the (split-window-vertically (/ (window-height) 4)) code in 2012 [2]. The two functions seem pretty interchangeable. > As far as testing goes, a test would be nice -- is this something you > could add to test/T460-emacs-tree.sh ? Testing UI/UX issues is always > pretty tough though, and it's not clear to me that we're actually > already testing the existing "(/ (window-height) 4)" business anyway. I did look at that file, and there is no tests for the current split functionality. All of the tests use the test-output function which outputs the contents of a buffer -- we need the whole frame. This might need to be done at a layer above Emacs and may complicate the test. I took a "screenshot" by copying my terminal contents with tmux, but I'm not sure how that would look like in a test. If I manually do the split on the test case [3], the fourth line does cut off some of the text since it goes beyond 160 columns, the default threshold. It is still viewable by pressing C-e, like anything too long. So I agree that testing would be nice but it seems to be an existing problem here. I'm not sure how, or if, other Emacs packages do character-perfect tests like these. -Radu [1]: git.savannah.gnu.org/cgit/emacs.git/commit/?id=8b10a2d19895041340296a703ba956c77541ec88 [2]: git.notmuchmail.org/git?p=notmuch;a=commit;h=3d92a257c8adbb36615bc61be9e668c8188006dc [3]: https://gist.githubusercontent.com/rbutoi/78c5b30fc5e64585bda18abbc5bf1fbf/raw/cba7df184f3cd534c92a6fb9bf39aa121c19ecd7/gistfile1.txt ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
python-cffi and ruby test suites fail in out-of-tree builds
Hey folks-- I just did a bit of testing and cleanup for out-of-tree builds (see the minor patches that should have landed on the list in the last hour or two). For me, "make check" in an out-of-tree build works fine now, with the exception of T391-python-cffi.sh and T395-ruby.sh. I'm afraid i don't know enough about either python or ruby to see the obvious fix for these builds, but i do note that they're the only builds which depend on copying their source into the out-of-tree location (assuming no one proposes an improvement for id:20200522010359.715688-1-...@fifthhorseman.net). Just thought i'd flag this as an outstanding problem that hopefully someone clever feels like digging into. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] tests: fix test_json_nodes() in out-of-tree builds
In out-of-tree builds, $TEST_DIRECTORY doesn't contain
json_check_nodes.py. This caused 27 tests to fail in such an
environment.
Signed-off-by: Daniel Kahn Gillmor
---
test/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 58972339..792b1cb9 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -556,7 +556,7 @@ test_json_nodes () {
if ! test_skip "$test_subtest_name"
then
- output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON
"$TEST_DIRECTORY"/json_check_nodes.py "$@")
+ output=$(PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON
"$NOTMUCH_SRCDIR"/test/json_check_nodes.py "$@")
if [ "$?" = 0 ]
then
test_ok_
--
2.26.2
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/5] build: optionally build python-cffi bindings
On Mon 2019-11-04 23:26:25 +0200, Tomi Ollila wrote: > how bad does out-of-tree build break with this -- do we need to do > the same as with ruby bindings (copy sources -- do we still do so)? or does > python provide better alternative..? I've just posted id:20200522010359.715688-1-...@fifthhorseman.net to apply the same hack as the ruby bindings here. I welcome any better proposals, but think we should at least have the build complete! --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] python-cffi: enable out-of-tree builds
This is a simple hack to enable out-of-tree builds, a concern raised by Tomi in id:m24kzjib9a@guru.guru-group.fi This change at least enables "make check" to complete without error, but I'm sure it could be improved. I am not expert enough in setuptools to know how. Signed-off-by: Daniel Kahn Gillmor --- configure | 8 1 file changed, 8 insertions(+) diff --git a/configure b/configure index 37368bda..f0472e8e 100755 --- a/configure +++ b/configure @@ -70,6 +70,14 @@ if [ "$srcdir" != "." ]; then mkdir bindings/ruby cp -a "$srcdir"/bindings/ruby/*.[ch] bindings/ruby cp -a "$srcdir"/bindings/ruby/extconf.rb bindings/ruby + +# Use the same hack to replicate python-cffi source for +# out-of-tree builds (again, not ideal). +mkdir bindings/python-cffi +cp -a "$srcdir"/bindings/python-cffi/tests \ + "$srcdir"/bindings/python-cffi/notmuch2 \ + "$srcdir"/bindings/python-cffi/setup.py \ + bindings/python-cffi/ fi # Set several defaults (optionally specified by the user in -- 2.26.2 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: test suite: FIXED messages are misordered with tests
On Fri 2020-05-22 00:57:14 +0300, Tomi Ollila wrote: > We've accumulated quite a bit of mess during these years to the test > system, which makes it harder to do larger adjustments (and not (yet) > mentioning even larger refactorings...). The last significant refactoring was probably when jrollins and i got the test suite running in parallel, and that might not even qualify as "major". Of course, the most effective tool for encouraging any refactoring is having a robust and reliable test suite to make sure that nothing broke. But now we're talking about a test suite for the test suite… i dunno how far we want to go down the rabbit hole! --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2 v3] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
When checking cryptographic signatures, Notmuch relies on GMime to
tell it whether the certificate that signs a message has a valid User
ID or not.
If the User ID is not valid, then notmuch does not report the signer's
User ID to the user. This means that the consumer of notmuch's
cryptographic summary of a message (or of its protected headers) can
be confident in relaying the reported identity to the user.
However, some versions of GMime before 3.2.7 cannot report Certificate
validity for X.509 certificates. This is resolved upstream in GMime
at https://github.com/jstedfast/gmime/pull/90.
We adapt to this by marking tests of reported User IDs for
S/MIME-signed messages as known-broken if GMime is older than 3.2.7
and has not been patched.
If GMime >= 3.2.7 and certificate validity still doesn't work for
X.509 certs, then there has likely been a regression in GMime and we
should fail early, during ./configure.
To break out these specific User ID checks from other checks, i had to
split some tests into two parts, and reuse $output across the two
subtests.
Signed-off-by: Daniel Kahn Gillmor
---
configure | 83 +-
test/T355-smime.sh | 17 ---
test/T356-protected-headers.sh | 13 +-
3 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/configure b/configure
index 0cfdaa6f..37368bda 100755
--- a/configure
+++ b/configure
@@ -494,7 +494,7 @@ int main () {
if (error) return !! fprintf (stderr, "failed to instantiate parser with
test/corpora/crypto/basic-encrypted.eml\n");
body = GMIME_MULTIPART_ENCRYPTED(g_mime_message_get_mime_part
(g_mime_parser_construct_message (parser, NULL)));
-if (body == NULL) return !!fprintf (stderr, "did not find a
multipart encrypted message\n");
+if (body == NULL) return !! fprintf (stderr, "did not find a multipart
encrypted message\n");
output = g_mime_multipart_encrypted_decrypt (body,
GMIME_DECRYPT_EXPORT_SESSION_KEY, NULL, _result, );
if (error || output == NULL) return !! fprintf (stderr, "decryption
failed\n");
@@ -536,6 +536,82 @@ EOF
if [ -n "$TEMP_GPG" -a -d "$TEMP_GPG" ]; then
rm -rf "$TEMP_GPG"
fi
+
+# see https://github.com/jstedfast/gmime/pull/90
+# should be fixed in GMime in 3.2.7, but some distros might patch
+printf "Checking for GMime X.509 certificate validity... "
+
+cat > _check_x509_validity.c <
+#include
+
+int main () {
+GError *error = NULL;
+GMimeParser *parser = NULL;
+GMimeApplicationPkcs7Mime *body = NULL;
+GMimeSignatureList *sig_list = NULL;
+GMimeSignature *sig = NULL;
+GMimeCertificate *cert = NULL;
+GMimeObject *output = NULL;
+GMimeValidity validity = GMIME_VALIDITY_UNKNOWN;
+int len;
+
+g_mime_init ();
+parser = g_mime_parser_new ();
+g_mime_parser_init_with_stream (parser,
g_mime_stream_file_open("$srcdir/test/corpora/pkcs7/smime-onepart-signed.eml",
"r", ));
+if (error) return !! fprintf (stderr, "failed to instantiate parser with
test/corpora/pkcs7/smime-onepart-signed.eml\n");
+
+body = GMIME_APPLICATION_PKCS7_MIME(g_mime_message_get_mime_part
(g_mime_parser_construct_message (parser, NULL)));
+if (body == NULL) return !!fprintf (stderr, "did not find a
application/pkcs7 message\n");
+
+sig_list = g_mime_application_pkcs7_mime_verify (body, GMIME_VERIFY_NONE,
, );
+if (error || output == NULL) return !! fprintf (stderr, "verify failed\n");
+
+if (sig_list == NULL) return !! fprintf (stderr, "no GMimeSignatureList
found\n");
+len = g_mime_signature_list_length (sig_list);
+if (len != 1) return !! fprintf (stderr, "expected 1 signature, got %d\n",
len);
+sig = g_mime_signature_list_get_signature (sig_list, 0);
+if (sig == NULL) return !! fprintf (stderr, "no GMimeSignature found at
position 0\n");
+cert = g_mime_signature_get_certificate (sig);
+if (cert == NULL) return !! fprintf (stderr, "no GMimeCertificate
found\n");
+validity = g_mime_certificate_get_id_validity (cert);
+if (validity != GMIME_VALIDITY_FULL) return !! fprintf (stderr, "Got
validity %d, expected %d\n", validity, GMIME_VALIDITY_FULL);
+
+return 0;
+}
+EOF
+if ! TEMP_GPG=$(mktemp -d "${TMPDIR:-/tmp}/notmuch.XX"); then
+printf 'No.\nCould not make tempdir for testing X.509 certificate
validity support.\n'
+errors=$((errors + 1))
+elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c
${gmime_ldflags} -o _check_x509_validity \
+&& echo disable-crl-checks > "$TEMP_GPG/gpgsm.conf" \
+&& echo
"4D:E0:FF:63:C0:E9:EC:01:29:11:C8:7A:EE:DA:3A:9A:7F:6E:C1:0D S" >>
"$TEMP_GPG/trustlist.txt" \
+&& GNUPGHOME=${TEMP_GPG} gpgsm --batch --quiet --import <
"$srcdir"/test/smime/ca.crt
+then
+if GNUPGHOME=${TEMP_GPG} ./_check_x509_validity; then
+gmime_x509_cert_validity=1
+printf "Yes.\n"
Re: [PATCH 2/2 v2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
Thanks for the review, and for the poke about out-of-tree builds on IRC,
Bremner. Another revision is coming in a minute. Notes below…
On Thu 2020-05-21 20:29:05 -0300, David Bremner wrote:
> I find these long lines with !! in the middle pretty surprising. Is
> there some reason for this style? It doesn't seem to fit with the usual
> conventions.
Hm, do we even have conventions for inlined C in ./configure?
If you'd rather i expand these to something more verbose, i can do so,
but i was under the impression that we wanted to keep these C
interstitials fairly compact so that ./configure is still (somewhat)
readable as a shell script.
The "return !! fprintf…" idiom is a compact way to get a non-zero
process error code and an error message to stderr without introducing a
code block. The only way for fprintf to return 0 (which would result in
the process returning 0) is if 0 bytes are written and no error
occurred, which isn't possible with any of the format strings supplied
here.
Another approach would be to pull the C entirely out of ./configure, but
that could have a problem when dealing with out-of-tree builds. (i just
noticed a problem for this test with out-of-tree builds, which i'll
revise in a minute)
> This line in particular has a tab in the middle.
i dunno how that got there, i'll have it fixed in the upcoming revision.
>> +elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c
>> ${gmime_ldflags} -o _check_x509_validity \
>
> The other test files are cleaned up in configure (source and binary)
> once we are done with them.
good point, i'll ensure that they get cleaned up alongside
_check_session_keys* in the upcoming revision.
> As far as I could follow, the changes to the tests themselves look
> reasonable.
thanks for the thoughtful review!
--dkg
signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2 v2] smime: tests of X.509 certificate validity are known-broken on GMime < 3.2.7
Daniel Kahn Gillmor writes:
> When checking cryptographic signatures, Notmuch relies on GMime to
> tell it whether the certificate that signs a message has a valid User
> ID or not.
>
> If the User ID is not valid, then notmuch does not report the signer's
> User ID to the user. This means that the consumer of notmuch's
> cryptographic summary of a message (or of its protected headers) can
> be confident in relaying the reported identity to the user.
>
> However, some versions of GMime before 3.2.7 cannot report Certificate
> validity for X.509 certificates. This is resolved upstream in GMime
> at https://github.com/jstedfast/gmime/pull/90.
>
> We adapt to this by marking tests of reported User IDs for
> S/MIME-signed messages as known-broken if GMime is older than 3.2.7
> and has not been patched.
>
> If GMime >= 3.2.7 and certificate validity still doesn't work for
> X.509 certs, then there has likely been a regression in GMime and we
> should fail early, during ./configure.
>
> To break out these specific User ID checks from other checks, i had to
> split some tests into two parts, and reuse $output across the two
> subtests.
>
> Signed-off-by: Daniel Kahn Gillmor
> ---
> configure | 79 ++
> test/T355-smime.sh | 17 +---
> test/T356-protected-headers.sh | 13 +-
> 3 files changed, 100 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index 0cfdaa6f..92e5bd1b 100755
> --- a/configure
> +++ b/configure
> @@ -536,6 +536,82 @@ EOF
> if [ -n "$TEMP_GPG" -a -d "$TEMP_GPG" ]; then
> rm -rf "$TEMP_GPG"
> fi
> +
> +# see https://github.com/jstedfast/gmime/pull/90
> +# should be fixed in GMime in 3.2.7, but some distros might patch
> +printf "Checking for GMime X.509 certificate validity... "
> +
> +cat > _check_x509_validity.c < +#include
> +#include
> +
> +int main () {
> +GError *error = NULL;
> +GMimeParser *parser = NULL;
> +GMimeApplicationPkcs7Mime *body = NULL;
> +GMimeSignatureList *sig_list = NULL;
> +GMimeSignature *sig = NULL;
> +GMimeCertificate *cert = NULL;
> +GMimeObject *output = NULL;
> +GMimeValidity validity = GMIME_VALIDITY_UNKNOWN;
> +int len;
> +
> +g_mime_init ();
> +parser = g_mime_parser_new ();
> +g_mime_parser_init_with_stream (parser,
> g_mime_stream_file_open("test/corpora/pkcs7/smime-onepart-signed.eml", "r",
> ));
> +if (error) return !! fprintf (stderr, "failed to instantiate parser with
> test/corpora/pkcs7/smime-onepart-signed.eml\n");
> +
> +body = GMIME_APPLICATION_PKCS7_MIME(g_mime_message_get_mime_part
> (g_mime_parser_construct_message (parser, NULL)));
> +if (body == NULL) return !! fprintf (stderr, "did not find a
> application/pkcs7 message\n");
I find these long lines with !! in the middle pretty surprising. Is
there some reason for this style? It doesn't seem to fit with the usual
conventions.
This line in particular has a tab in the middle.
> +elif ${CC} ${CFLAGS} ${gmime_cflags} _check_x509_validity.c
> ${gmime_ldflags} -o _check_x509_validity \
The other test files are cleaned up in configure (source and binary)
once we are done with them.
As far as I could follow, the changes to the tests themselves look
reasonable.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] python/notmuch2: add bindings for the database config strings
Thanks for adding more of the API!
This mostly is fine as well, again I'd mainly ask to add tests however.
At least the things which are implemented directly I guess: setitem,
getitem, iter and len.
On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:
> ---
> bindings/python-cffi/notmuch2/_build.py| 17 +
> bindings/python-cffi/notmuch2/_config.py | 84 ++
> bindings/python-cffi/notmuch2/_database.py | 23 ++
> 3 files changed, 124 insertions(+)
> create mode 100644 bindings/python-cffi/notmuch2/_config.py
>
> diff --git a/bindings/python-cffi/notmuch2/_build.py
> b/bindings/python-cffi/notmuch2/_build.py
> index 5e1fcac1..f269f2a1 100644
> --- a/bindings/python-cffi/notmuch2/_build.py
> +++ b/bindings/python-cffi/notmuch2/_build.py
> @@ -314,6 +314,23 @@ ffibuilder.cdef(
> notmuch_indexopts_get_decrypt_policy (const notmuch_indexopts_t
> *indexopts);
> void
> notmuch_indexopts_destroy (notmuch_indexopts_t *options);
> +
> +notmuch_status_t
> +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_bool_t
> +notmuch_config_list_valid (notmuch_config_list_t *config_list);
> +const char *
> +notmuch_config_list_key (notmuch_config_list_t *config_list);
> +const char *
> +notmuch_config_list_value (notmuch_config_list_t *config_list);
> +void
> +notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
> +void
> +notmuch_config_list_destroy (notmuch_config_list_t *config_list);
> """
> )
>
> diff --git a/bindings/python-cffi/notmuch2/_config.py
> b/bindings/python-cffi/notmuch2/_config.py
> new file mode 100644
> index ..58383c16
> --- /dev/null
> +++ b/bindings/python-cffi/notmuch2/_config.py
> @@ -0,0 +1,84 @@
> +import collections.abc
> +
> +import notmuch2._base as base
> +import notmuch2._capi as capi
> +import notmuch2._errors as errors
> +
> +__all__ = ['ConfigMapping']
> +
Same style thing about 2 blank lines
> +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)
> +
> +def __next__(self):
> +item = super().__next__()
> +return base.BinString.from_cffi(item)
> +
> +class ConfigMapping(base.NotmuchObject, collections.abc.MutableMapping):
> +"""The config key/value pairs stored in the database.
> +
> +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.
> +
> +: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):
> +self._parent = parent
> +self._ptr = lambda: getattr(parent, ptr_name)
> +
> +@property
> +def alive(self):
> +return self._parent.alive
> +
> +def _destroy(self):
> +pass
> +
> +def __getitem__(self, key):
> +if isinstance(key, str):
> +key = key.encode('utf-8')
> +val_pp = capi.ffi.new('char**')
> +ret = capi.lib.notmuch_database_get_config(self._ptr(), key, val_pp)
> +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +raise errors.NotmuchError(ret)
> +if val_pp[0] == "":
> +capi.lib.free(val_pp[0])
> +raise KeyError
> +val = base.BinString.from_cffi(val_pp[0])
> +capi.lib.free(val_pp[0])
> +return val
> +
> +def __setitem__(self, key, val):
> +if isinstance(key, str):
> +key = key.encode('utf-8')
> +if isinstance(val, str):
> +val = val.encode('utf-8')
> +ret = capi.lib.notmuch_database_set_config(self._ptr(), key, val)
> +if ret != capi.lib.NOTMUCH_STATUS_SUCCESS:
> +raise errors.NotmuchError(ret)
> +
> +def __delitem__(self, key):
> +self[key] = ""
> +
> +def __iter__(self):
> +"""Return an iterator over the config items.
> +
> +:raises NullPointerError: If the iterator can not be created.
> +"""
> +configlist_pp = capi.ffi.new('notmuch_config_list_t**')
> +ret =
Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query
Hi Anton,
Thanks for improving the bindings! Any my apologies for the late
response, I failed to spot this mail the first time round.
Also, this is a pretty serious bug, thanks for finding it.
This looks pretty solid, a few small style comments that aren't very
important notwithstanding. Though I do think you should be able to add
a test for this in the pytest tests. I think just triggering the
use-after-free you demonstrate in the commit message is fine as it will
work with your patch. The fact it will crash rather then fail without
is unfortunate but probably fine.
On Sat 09 May 2020 at 07:05 +0200, Anton Khirnov wrote:
> Any messages retrieved from a query - either directly via
> search_messages() or indirectly via thread objects - are owned by that
> query. Retrieving the same message (i.e. corresponding to the same
> message ID / database object) several times will always yield the same
> C object.
>
> The caller is allowed to destroy message objects owned by a query before
> the query itself - which can save memory for long-lived queries.
> However, that message must then never be retrieved again from that
> query.
>
> The python-notmuch2 bindings will currently destroy every message object
> in Message._destroy(), which will lead to an invalid free if the same
> message is then retrieved again. E.g. the following python program leads
> to libtalloc abort()ing:
>
> import notmuch2
> db = notmuch2.Database(mode = notmuch2.Database.MODE.READ_ONLY)
> t= next(db.threads('*'))
> msgs = list(zip(t.toplevel(), t.toplevel()))
> msgs = list(zip(t.toplevel(), t.toplevel()))
>
> Fix this issue by creating a subclass of Message, which is used for
> "standalone" message which have to be freed by the caller. Message class
> is then used only for messages descended from a query, which do not need
> to be freed by the caller.
> ---
> bindings/python-cffi/notmuch2/_database.py | 6 ++--
> bindings/python-cffi/notmuch2/_message.py | 42 ++
> 2 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_database.py
> b/bindings/python-cffi/notmuch2/_database.py
> index 95f59ca0..f14eac78 100644
> --- a/bindings/python-cffi/notmuch2/_database.py
> +++ b/bindings/python-cffi/notmuch2/_database.py
> @@ -399,7 +399,7 @@ class Database(base.NotmuchObject):
>capi.lib.NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID]
> if ret not in ok:
> raise errors.NotmuchError(ret)
> -msg = message.Message(self, msg_pp[0], db=self)
> +msg = message.StandaloneMessage(self, msg_pp[0], db=self)
> if sync_flags:
> msg.tags.from_maildir_flags()
> return self.AddedMessage(
> @@ -468,7 +468,7 @@ class Database(base.NotmuchObject):
> msg_p = msg_pp[0]
> if msg_p == capi.ffi.NULL:
> raise LookupError
> -msg = message.Message(self, msg_p, db=self)
> +msg = message.StandaloneMessage(self, msg_p, db=self)
> return msg
>
> def get(self, filename):
> @@ -501,7 +501,7 @@ class Database(base.NotmuchObject):
> msg_p = msg_pp[0]
> if msg_p == capi.ffi.NULL:
> raise LookupError
> -msg = message.Message(self, msg_p, db=self)
> +msg = message.StandaloneMessage(self, msg_p, db=self)
> return msg
>
> @property
> diff --git a/bindings/python-cffi/notmuch2/_message.py
> b/bindings/python-cffi/notmuch2/_message.py
> index c5fdbf6d..416ce7ca 100644
> --- a/bindings/python-cffi/notmuch2/_message.py
> +++ b/bindings/python-cffi/notmuch2/_message.py
> @@ -14,7 +14,7 @@ __all__ = ['Message']
>
>
> class Message(base.NotmuchObject):
> -"""An email message stored in the notmuch database.
> +"""An email message stored in the notmuch database retrieved via a query.
>
> This should not be directly created, instead it will be returned
> by calling methods on :class:`Database`. A message keeps a
> @@ -61,22 +61,10 @@ class Message(base.NotmuchObject):
>
> @property
> def alive(self):
> -if not self._parent.alive:
> -return False
> -try:
> -self._msg_p
> -except errors.ObjectDestroyedError:
> -return False
> -else:
> -return True
> -
> -def __del__(self):
> -self._destroy()
> +return self._parent.alive
>
> def _destroy(self):
> -if self.alive:
> -capi.lib.notmuch_message_destroy(self._msg_p)
> -self._msg_p = None
> +pass
I feel like some more description from the commit message could live
here to explain why this isn't destroying the message rather than having
to find it in the commit message.
>
> @property
> def messageid(self):
> @@ -375,6 +363,30 @@ class Message(base.NotmuchObject):
> if isinstance(other, self.__class__):
> return self.messageid == other.messageid
>
> +class
Re: [PATCH] emacs: split-window-sensibly in tree mode with open message
On Sat 2020-05-02 20:11:09 -0400, Radu Butoi wrote: > This uses the standard Emacs function `split-window-sensibly` to split a > window horizontally or vertically depending on space when opening a > message in tree view. By default, split-width-threshold is 160 columns > (and -height- is nil), so screens wider than 160 will be split > horizontally. > > This is based on an older proposal [1] which manually did the > calculation of width. The main issues there were (1) lack of > configurability and (2) lack of testing. I don't have an answer for > testing, but this allows users to configure two thresholds using > built-in variables, an improvement. I like this proposal, and the simplification that it gives to the notmuch-emacs codebase. However, this thread is the first place i've learned about split-window-sensibly, so i'm probably not eligible to really judge the merits here. As far as testing goes, a test would be nice -- is this something you could add to test/T460-emacs-tree.sh ? Testing UI/UX issues is always pretty tough though, and it's not clear to me that we're actually already testing the existing "(/ (window-height) 4)" business anyway. --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs docstrings: consistent indentation, newlines, periods
On Thu, May 21 2020, Daniel Kahn Gillmor wrote:
> On Mon 2020-05-04 00:21:36 +0300, Tomi Ollila wrote:
>> Fixed emacs docstrings to be consistent. No functional change.
>>
>> - removed some (accidental) indentation
>> - removed some trailing newlines
>> - added trailing periods where missing (some exclusions)
>
> This all looks good to me, except for the following changes.
>
> I think if the last line of the docstring is an example, deliberately
> indented to indicate that it is verbatim code, adding a trailing period
> is a mistake, because it encourages naive users to make the obvious
> error of including the period in their text.
>
> These were the examples i noticed in a skim of the changeset:
>
>> --- a/emacs/notmuch-draft.el
>> +++ b/emacs/notmuch-draft.el
>> @@ -45,7 +45,7 @@ (defcustom notmuch-draft-tags '("+draft")
>> For example, if you wanted to give the message a \"draft\" tag
>> but not the (normally added by default) \"inbox\" tag, you would
>> set:
>> -(\"+draft\" \"-inbox\")"
>> +(\"+draft\" \"-inbox\")."
>>:type '(repeat string)
>>:group 'notmuch-draft)
>>
>
>
>> --- a/emacs/notmuch-lib.el
>> +++ b/emacs/notmuch-lib.el
>> @@ -145,7 +145,7 @@ (defcustom notmuch-archive-tags '("-inbox")
>>
>> For example, if you wanted to remove an \"inbox\" tag and add an
>> \"archived\" tag, you would set:
>> -(\"-inbox\" \"+archived\")"
>> +(\"-inbox\" \"+archived\")."
>>:type '(repeat string)
>>:group 'notmuch-search
>>:group 'notmuch-show)
>
>
>> --- a/emacs/notmuch-message.el
>> +++ b/emacs/notmuch-message.el
>> @@ -33,7 +33,7 @@ (defcustom notmuch-message-replied-tags '("+replied")
>>
>> For example, if you wanted to add a \"replied\" tag and remove
>> the \"inbox\" and \"todo\" tags, you would set:
>> -(\"+replied\" \"-inbox\" \"-todo\")"
>> +(\"+replied\" \"-inbox\" \"-todo\")."
>>:type '(repeat string)
>>:group 'notmuch-send)
>>
>> @@ -46,7 +46,7 @@ (defcustom notmuch-message-forwarded-tags '("+forwarded")
>>
>> For example, if you wanted to add a \"forwarded\" tag and remove
>> the \"inbox\" tag, you would set:
>> -(\"+forwarded\" \"-inbox\")"
>> +(\"+forwarded\" \"-inbox\")."
>>:type '(repeat string)
>>:group 'notmuch-send)
>
>
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -237,7 +237,7 @@ (defcustom notmuch-show-mark-read-tags '("-unread")
>>
>> For example, if you wanted to remove an \"unread\" tag and add a
>> \"read\" tag (which would make little sense), you would set:
>> -(\"-unread\" \"+read\")"
>> +(\"-unread\" \"+read\")."
>>:type '(repeat string)
>>:group 'notmuch-show)
>
>
> I'd prefer this changeset to not have trailing periods when the
> docstring ends in an example. (and maybe also to clean up any trailing
> periods that might already be in such a docstring, if they exist)
>
> I recognize this is something of an aesthetic position with no
> objectively correct answer, but so is the question of trailing periods,
> i suppose. I'm not going to die on this hill, though.
I agree with your point. Like I mentioned I excluded trailing period at
least in one case where missing, probably similar to this. I have to
reconsider also to re-indent some content after For example:
How I did the changeset I used David's rstdoc.el to get long list of all
docstrings and then looked through that and went editing original .el
files -- monotonic changes makes one blind of all kinds of considerations.
>
> Regards,
>
> --dkg
Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: test suite: FIXED messages are misordered with tests
On Thu, May 21 2020, Daniel Kahn Gillmor wrote: > On Thu 2020-05-21 00:16:48 +0300, Tomi Ollila wrote: >> (just tested this latest works) > > Thanks for looking into this, Tomi! > > Do you have a patch to propose? Looked a bit (now). Somewhat complicated to make perfect (enemy of good) change. print_test_description could be added as first line in test_known_broken_ok_ (then inconsistent with test_known_broken_failure_) or in test_ok_ just before calling test_known_broken_ok_ (test_known_broken_ok_ is called only once in test-lib.sh) but it looks like test_check_missing_external_prereqs_ and test_report_skip_ may be called before print_test_description is called (and there may be more...) We've accumulated quite a bit of mess during these years to the test system, which makes it harder to do larger adjustments (and not (yet) mentioning even larger refactorings...). > >--dkg Tomi ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: add notmuch-expr, sexp-style queries
On Wed 2020-05-13 20:00:24 +1000, Tom Fitzhenry wrote: > notmuch-expr allows you to write notmuch search queries in sexp style like: > > (notmuch-expr > '(and > (to "emacs-devel") > "info manual" > (or > (not (is "spam")) > (is "important" > > which will generate the textual query: > > "to:emacs-devel AND (NOT is:spam OR is:important) AND \"info manual\"" I like this idea! > +(defun notmuch-expr--quote (s) > + ;; FIXME Escape s. > + (concat "\"" s "\"")) Shouldn't this FIXME be resolved before we consider merging? --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs docstrings: consistent indentation, newlines, periods
On Mon 2020-05-04 00:21:36 +0300, Tomi Ollila wrote:
> Fixed emacs docstrings to be consistent. No functional change.
>
> - removed some (accidental) indentation
> - removed some trailing newlines
> - added trailing periods where missing (some exclusions)
This all looks good to me, except for the following changes.
I think if the last line of the docstring is an example, deliberately
indented to indicate that it is verbatim code, adding a trailing period
is a mistake, because it encourages naive users to make the obvious
error of including the period in their text.
These were the examples i noticed in a skim of the changeset:
> --- a/emacs/notmuch-draft.el
> +++ b/emacs/notmuch-draft.el
> @@ -45,7 +45,7 @@ (defcustom notmuch-draft-tags '("+draft")
> For example, if you wanted to give the message a \"draft\" tag
> but not the (normally added by default) \"inbox\" tag, you would
> set:
> -(\"+draft\" \"-inbox\")"
> +(\"+draft\" \"-inbox\")."
>:type '(repeat string)
>:group 'notmuch-draft)
>
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -145,7 +145,7 @@ (defcustom notmuch-archive-tags '("-inbox")
>
> For example, if you wanted to remove an \"inbox\" tag and add an
> \"archived\" tag, you would set:
> -(\"-inbox\" \"+archived\")"
> +(\"-inbox\" \"+archived\")."
>:type '(repeat string)
>:group 'notmuch-search
>:group 'notmuch-show)
> --- a/emacs/notmuch-message.el
> +++ b/emacs/notmuch-message.el
> @@ -33,7 +33,7 @@ (defcustom notmuch-message-replied-tags '("+replied")
>
> For example, if you wanted to add a \"replied\" tag and remove
> the \"inbox\" and \"todo\" tags, you would set:
> -(\"+replied\" \"-inbox\" \"-todo\")"
> +(\"+replied\" \"-inbox\" \"-todo\")."
>:type '(repeat string)
>:group 'notmuch-send)
>
> @@ -46,7 +46,7 @@ (defcustom notmuch-message-forwarded-tags '("+forwarded")
>
> For example, if you wanted to add a \"forwarded\" tag and remove
> the \"inbox\" tag, you would set:
> -(\"+forwarded\" \"-inbox\")"
> +(\"+forwarded\" \"-inbox\")."
>:type '(repeat string)
>:group 'notmuch-send)
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -237,7 +237,7 @@ (defcustom notmuch-show-mark-read-tags '("-unread")
>
> For example, if you wanted to remove an \"unread\" tag and add a
> \"read\" tag (which would make little sense), you would set:
> -(\"-unread\" \"+read\")"
> +(\"-unread\" \"+read\")."
>:type '(repeat string)
>:group 'notmuch-show)
I'd prefer this changeset to not have trailing periods when the
docstring ends in an example. (and maybe also to clean up any trailing
periods that might already be in such a docstring, if they exist)
I recognize this is something of an aesthetic position with no
objectively correct answer, but so is the question of trailing periods,
i suppose. I'm not going to die on this hill, though.
Regards,
--dkg
signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: waiting tag
On Mon 2020-05-04 09:25:59 +0200, Gregor Zattler wrote:
> * Keegan Carruthers-Smith [2020-05-03; 22:37]:
>> notmuch tag -waiting -- tag:waiting and 'thread:{tag:new}'
>
> but this removes the waiting tag if there is some response
> to some message in the thread, not necessary to the message
> tagged waiting, isn't it?
Yes, you're right, this is not exactly the thing you were asking for.
fwiw, i don't know how to provide the thing you were asking for either
:(
--dkg
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: test suite: FIXED messages are misordered with tests
On Thu 2020-05-21 00:16:48 +0300, Tomi Ollila wrote: > (just tested this latest works) Thanks for looking into this, Tomi! Do you have a patch to propose? --dkg signature.asc Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query
Anton Khirnov writes: > ping I'm hoping Floris will find a chance to review your patches. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] python/notmuch2: do not destroy messages owned by a query
ping -- Anton Khirnov ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
