Re: [PATCH] test: report summary even when aborting
On Sat, May 25 2019, Daniel Kahn Gillmor wrote: > In certain cases of test suite failure, the summary report was not > being printed. In particular, any failure on the parallel test suite, > and any aborted test in the serialized test suite would end up hiding > the summary. > > It's better to always show the summary where we can (while preserving > the return code). This is an improvement, but it looks like it is possible aggregate-results.sh prints everything good (like all 3 tests passed) since it may be that there are only good logs in test-results/. That might, in some cases, be confusing (exit value of notmuch-test is not always visible). One option could be giving $RES to aggregate-results.sh and if it is not zero then that could print different text -- just that it requires a bit more coding... Tomi > > Signed-off-by: Daniel Kahn Gillmor > --- > test/notmuch-test | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/test/notmuch-test b/test/notmuch-test > index 50ed8721..d835e152 100755 > --- a/test/notmuch-test > +++ b/test/notmuch-test > @@ -45,6 +45,8 @@ else > fi > > trap 'e=$?; kill $!; exit $e' HUP INT TERM > + > +RES=0 > # Run the tests > if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then > test -t 1 && export COLORS_WITHOUT_TTY=t || : > @@ -58,7 +60,6 @@ if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel > >/dev/null ; then > RES=$? > if [[ $RES != 0 ]]; then > echo "parallel test suite returned error code $RES" > -exit $RES > fi > else > for test in $TESTS; do > @@ -69,7 +70,7 @@ else > RES=$? > testname=$(basename $test .sh) > if [[ $RES != 0 && ! -e > "$NOTMUCH_BUILDDIR/test/test-results/$testname" ]]; then > -exit $RES > +echo "Aborting on $testname (returned $RES)" > fi > done > fi > @@ -78,7 +79,11 @@ trap - HUP INT TERM > # Report results > echo > $NOTMUCH_SRCDIR/test/aggregate-results.sh > $NOTMUCH_BUILDDIR/test/test-results/* > -ev=$? > +if [ "$RES" = 0 ]; then > +ev=$? > +else > +ev=$RES > +fi > > # Clean up > rm -rf $NOTMUCH_BUILDDIR/test/test-results > -- > 2.20.1 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > https://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 2/4] cli: expose message-wide crypto status from mime-node
The mime node context (a per-message context) gains a cryptographic
status object, and the mime_node_t object itself can return a view on
that status to an interested party.
The status is not yet populated, and for now we can keep that view
read-only, so that it can only be populated/modified during MIME tree
traversal.
---
mime-node.c | 7 +++
notmuch-client.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/mime-node.c b/mime-node.c
index e6bb..66ff7446 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -34,6 +34,7 @@ typedef struct mime_node_context {
GMimeStream *stream;
GMimeParser *parser;
GMimeMessage *mime_message;
+_notmuch_message_crypto_t *msg_crypto;
/* Context provided by the caller. */
_notmuch_crypto_t *crypto;
@@ -54,6 +55,12 @@ _mime_node_context_free (mime_node_context_t *res)
return 0;
}
+const _notmuch_message_crypto_t*
+mime_node_get_message_crypto_status (mime_node_t *node)
+{
+return node->ctx->msg_crypto;
+}
+
notmuch_status_t
mime_node_open (const void *ctx, notmuch_message_t *message,
_notmuch_crypto_t *crypto, mime_node_t **root_out)
diff --git a/notmuch-client.h b/notmuch-client.h
index d762d3cc..a82cb431 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -439,6 +439,9 @@ mime_node_child (mime_node_t *parent, int child);
mime_node_t *
mime_node_seek_dfs (mime_node_t *node, int n);
+const _notmuch_message_crypto_t*
+mime_node_get_message_crypto_status (mime_node_t *node);
+
typedef enum dump_formats {
DUMP_FORMAT_AUTO,
DUMP_FORMAT_BATCH_TAG,
--
2.20.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
revision 3: easing access to the cryptographic envelope
This is the third revision of the series originally posted at id:20190424183113.29242-1-...@fifthhorseman.net (revision 2 was at id:20190520032228.27420-1-...@fifthhorseman.net) This series addresses comments raised by David Bremner in his review. Thanks, Bremner! The most significant change here is that notmuch-show in --format=json or --format=sexp now always emits a "crypto" member for every message, regardless of whether there is any cryptographic envelope. In the case where there is no cryptographic envelope, the "crypto" member will be empty. -- E-mail structures are potentially arbitrarily complicated. Cryptographic protection standards like S/MIME and OpenPGP or PGP/MIME are often applicable to some elements of some messages. Last year's "E-Fail" attacks made it clear that trying to provide normal users with cryptographic protections on piecemeal parts of an e-mail message is a recipe for disaster, both from an implementation perspective and a user experience perspective. I've argued in more detail at [0] about the need to treat cryptographic protections at the message level, rather than at the subpart level. [0] https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html This series makes "notmuch show" track and emit message-wide cryptographic state, providing an interface that simple clients that use "notmuch show" can rely on for their UI and UX. It doesn't yet apply this layer to the emacs interface, because at the moment many users of the emacs interface are nerds who are as likely to understand the intricacies of MIME structure as anyone, and for the moment, just augmenting the notmuch show schemata in a sensible way is enough of a chunk to bite off. (though i'd be happy to review and support the use of this per-message cryptographic state in notmuch-emacs if/when this lands!) I'd appreciate any review and feedback! Regards, --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 3/4] mime-node: track whole-message crypto state while walking the tree
Deliberately populate the message's cryptographic status while walking
the MIME tree from the CLI.
Note that the additional numchild argument added to _mime_node_create
is a passthrough needed to be able to adequately populate the crypto
state object.
---
mime-node.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/mime-node.c b/mime-node.c
index 66ff7446..48b45247 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -135,6 +135,8 @@ mime_node_open (const void *ctx, notmuch_message_t *message,
goto DONE;
}
+mctx->msg_crypto = _notmuch_message_crypto_new (mctx);
+
mctx->crypto = crypto;
/* Create the root node */
@@ -180,6 +182,7 @@ static void
node_verify (mime_node_t *node, GMimeObject *part)
{
GError *err = NULL;
+notmuch_status_t status;
node->verify_attempted = true;
node->sig_list = g_mime_multipart_signed_verify
@@ -193,6 +196,10 @@ node_verify (mime_node_t *node, GMimeObject *part)
if (err)
g_error_free (err);
+
+status = _notmuch_message_crypto_potential_sig_list(node->ctx->msg_crypto,
node->sig_list);
+if (status) /* this is a warning, not an error */
+ fprintf (stderr, "Warning: failed to note signature status: %s.\n",
notmuch_status_to_string (status));
}
/* Decrypt and optionally verify an encrypted mime node */
@@ -201,6 +208,7 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject
*part)
{
GError *err = NULL;
GMimeDecryptResult *decrypt_result = NULL;
+notmuch_status_t status;
GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
notmuch_message_t *message = NULL;
@@ -223,6 +231,9 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject
*part)
}
node->decrypt_success = true;
+status = _notmuch_message_crypto_successful_decryption
(node->ctx->msg_crypto);
+if (status) /* this is a warning, not an error */
+ fprintf (stderr, "Warning: failed to note decryption status: %s.\n",
notmuch_status_to_string (status));
if (decrypt_result) {
/* This may be NULL if the part is not signed. */
@@ -231,6 +242,9 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject
*part)
node->verify_attempted = true;
g_object_ref (node->sig_list);
set_signature_list_destructor (node);
+ status =
_notmuch_message_crypto_potential_sig_list(node->ctx->msg_crypto,
node->sig_list);
+ if (status) /* this is a warning, not an error */
+ fprintf (stderr, "Warning: failed to note signature status:
%s.\n", notmuch_status_to_string (status));
}
if (node->ctx->crypto->decrypt == NOTMUCH_DECRYPT_TRUE && message) {
@@ -251,9 +265,10 @@ node_decrypt_and_verify (mime_node_t *node, GMimeObject
*part)
}
static mime_node_t *
-_mime_node_create (mime_node_t *parent, GMimeObject *part)
+_mime_node_create (mime_node_t *parent, GMimeObject *part, int numchild)
{
mime_node_t *node = talloc_zero (parent, mime_node_t);
+notmuch_status_t status;
/* Set basic node properties */
node->part = part;
@@ -305,6 +320,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
} else {
node_verify (node, part);
}
+} else {
+ status = _notmuch_message_crypto_potential_payload
(node->ctx->msg_crypto, part, parent ? parent->part : NULL, numchild);
+ if (status)
+ fprintf (stderr, "Warning: failed to record potential crypto
payload (%s).\n", notmuch_status_to_string (status));
}
return node;
@@ -332,7 +351,7 @@ mime_node_child (mime_node_t *parent, int child)
INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
g_type_name (G_OBJECT_TYPE (parent->part)));
}
-node = _mime_node_create (parent, sub);
+node = _mime_node_create (parent, sub, child);
if (child == parent->next_child && parent->next_part_num != -1) {
/* We're traversing in depth-first order. Record the child's
--
2.20.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 4/4] cli/show: emit new whole-message crypto status output
This allows MUAs that don't want to think about per-mime-part
cryptographic status to have a simple high-level overview of the
message's cryptographic state.
Sensibly structured encrypted and/or signed messages will work fine
with this. The only requirement for the simplest encryption + signing
is that the message have all of its encryption and signing protection
(the "cryptographic envelope") in a contiguous set of MIME layers at
the very outside of the message itself.
This is because messages with some subparts signed or encrypted, but
with other subparts with no cryptographic protection is very difficult
to reason about, and even harder for the user to make sense of or work
with.
For further characterization of the Cryptographic Envelope and some of
the usability tradeoffs, see here:
https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html#cryptographic-envelope
---
devel/schemata | 18 ++
notmuch-show.c | 29 +
test/T070-insert.sh | 1 +
test/T160-json.sh| 11 ++-
test/T170-sexp.sh| 10 +-
test/T190-multipart.sh | 4 +++-
test/T220-reply.sh | 1 +
test/T340-maildir-sync.sh| 1 +
test/T350-crypto.sh | 19 +++
test/T355-smime.sh | 5 +++--
test/T470-missing-headers.sh | 2 ++
test/T510-thread-replies.sh | 11 +++
test/T670-duplicate-mid.sh | 1 +
13 files changed, 96 insertions(+), 17 deletions(-)
diff --git a/devel/schemata b/devel/schemata
index 42b1bcf3..72feb7b7 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -33,6 +33,8 @@ v3
v4
- replace signature error integer bitmask with a set of flags for
individual errors.
+- (notmuch 0.29) added message.crypto to identify overall message
+ cryptographic state
Common non-terminals
@@ -73,9 +75,25 @@ message = {
tags: [string*],
headers:headers,
+crypto: crypto,
body?: [part]# omitted if --body=false
}
+# when showing the message, was any or all of it decrypted?
+msgdecstatus: "full"|"partial"
+
+# The overall cryptographic state of the message as a whole:
+crypto = {
+signed?:{
+ status: sigstatus,
+ # was the set of signatures described under encrypted cover?
+ encrypted: bool,
+},
+decrypted?: {
+ status: msgdecstatus,
+}
+}
+
# A MIME part (format_part_sprinter)
part = {
id: int|string, # part id (currently DFS part number)
diff --git a/notmuch-show.c b/notmuch-show.c
index b95fc389..c6a7a10a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -628,6 +628,35 @@ format_part_sprinter (const void *ctx, sprinter_t *sp,
mime_node_t *node,
format_part_sprinter (ctx, sp, mime_node_child (node, 0), true,
include_html);
sp->end (sp);
}
+
+ if (notmuch_format_version >= 4) {
+ const _notmuch_message_crypto_t *msg_crypto =
mime_node_get_message_crypto_status (node);
+ sp->map_key (sp, "crypto");
+ sp->begin_map (sp);
+ if (msg_crypto->sig_list ||
+ msg_crypto->decryption_status !=
NOTMUCH_MESSAGE_DECRYPTED_NONE) {
+ if (msg_crypto->sig_list) {
+ sp->map_key (sp, "signed");
+ sp->begin_map (sp);
+ sp->map_key (sp, "status");
+ format_part_sigstatus_sprinter (sp, msg_crypto->sig_list);
+ if (msg_crypto->signature_encrypted) {
+ sp->map_key (sp, "encrypted");
+ sp->boolean (sp, msg_crypto->signature_encrypted);
+ }
+ sp->end (sp);
+ }
+ if (msg_crypto->decryption_status !=
NOTMUCH_MESSAGE_DECRYPTED_NONE) {
+ sp->map_key (sp, "decrypted");
+ sp->begin_map (sp);
+ sp->map_key (sp, "status");
+ sp->string (sp, msg_crypto->decryption_status ==
NOTMUCH_MESSAGE_DECRYPTED_FULL ? "full" : "partial");
+ sp->end (sp);
+ }
+ }
+ sp->end (sp);
+ }
+
sp->end (sp);
return;
}
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 05be473a..48165caa 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -48,6 +48,7 @@ test_begin_subtest "Insert message adds default tags"
output=$(notmuch show --format=json "subject:insert-subject")
expected='[[[{
"id": "'"${gen_msg_id}"'",
+ "crypto": {},
"match": true,
"excluded": false,
"filename": ["'"${cur_msg_filename}"'"],
diff --git a/test/T160-json.sh b/test/T160-json.sh
index 91b98e5d..004adb4e 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -5,16 +5,16 @@ test_description="--format=json output"
test_begin_subtest "Show
[PATCH v3 1/4] util/crypto: _notmuch_message_crypto: tracks message-wide crypto state
E-mail encryption and signatures reported by notmuch are at the MIME
part level. This makes sense in the dirty details, but for users we
need to have a per-message conception of the cryptographic state of
the e-mail. (see
https://dkg.fifthhorseman.net/blog/e-mail-cryptography.html for more
discussion of why this is important).
The object created in this patch is a useful for tracking the
cryptographic state of the underlying message as a whole, based on a
depth-first search of the message's MIME structure.
This object stores a signature list of the message, but we don't
handle it yet. Further patches in this series will make use of the
signature list.
---
util/crypto.c | 95 +++
util/crypto.h | 65 +++
2 files changed, 160 insertions(+)
diff --git a/util/crypto.c b/util/crypto.c
index 99104e78..d895932d 100644
--- a/util/crypto.c
+++ b/util/crypto.c
@@ -82,3 +82,98 @@ _notmuch_crypto_decrypt (bool *attempted,
decrypt_result, err);
return ret;
}
+
+
+static int
+_notmuch_message_crypto_destructor (_notmuch_message_crypto_t *msg_crypto)
+{
+if (!msg_crypto)
+ return 0;
+if (msg_crypto->sig_list)
+ g_object_unref (msg_crypto->sig_list);
+return 0;
+}
+
+_notmuch_message_crypto_t *
+_notmuch_message_crypto_new (void *ctx)
+{
+_notmuch_message_crypto_t *ret = talloc_zero (ctx,
_notmuch_message_crypto_t);
+talloc_set_destructor (ret, _notmuch_message_crypto_destructor);
+return ret;
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_potential_sig_list (_notmuch_message_crypto_t
*msg_crypto, GMimeSignatureList *sigs)
+{
+if (!msg_crypto)
+ return NOTMUCH_STATUS_NULL_POINTER;
+
+/* Signatures that arrive after a payload part during DFS are not
+ * part of the cryptographic envelope: */
+if (msg_crypto->payload_encountered)
+ return NOTMUCH_STATUS_SUCCESS;
+
+if (msg_crypto->sig_list)
+ g_object_unref (msg_crypto->sig_list);
+
+/* This signature list needs to persist as long as the _n_m_crypto
+ * object survives. Increasing its reference counter prevents
+ * garbage-collection until after _n_m_crypto_destroy is
+ * called. */
+msg_crypto->sig_list = sigs;
+if (sigs)
+ g_object_ref (sigs);
+
+if (msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_FULL)
+ msg_crypto->signature_encrypted = true;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_potential_payload (_notmuch_message_crypto_t
*msg_crypto, GMimeObject *payload, GMimeObject *parent, int childnum)
+{
+if (!msg_crypto || !payload)
+ return NOTMUCH_STATUS_NULL_POINTER;
+
+/* only fire on the first payload part encountered */
+if (msg_crypto->payload_encountered)
+ return NOTMUCH_STATUS_SUCCESS;
+
+/* the first child of multipart/encrypted that matches the
+ * encryption protocol should be "control information" metadata,
+ * not payload. So we skip it. (see
+ * https://tools.ietf.org/html/rfc1847#page-8) */
+if (parent && GMIME_IS_MULTIPART_ENCRYPTED (parent) && childnum ==
GMIME_MULTIPART_ENCRYPTED_VERSION) {
+ const char *enc_type = g_mime_object_get_content_type_parameter
(parent, "protocol");
+ GMimeContentType *ct = g_mime_object_get_content_type (payload);
+ if (ct && enc_type) {
+ const char *part_type = g_mime_content_type_get_mime_type (ct);
+ if (part_type && strcmp (part_type, enc_type) == 0)
+ return NOTMUCH_STATUS_SUCCESS;
+ }
+}
+
+msg_crypto->payload_encountered = true;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+
+notmuch_status_t
+_notmuch_message_crypto_successful_decryption (_notmuch_message_crypto_t
*msg_crypto)
+{
+if (!msg_crypto)
+ return NOTMUCH_STATUS_NULL_POINTER;
+
+/* see the rationale for different values of
+ * _notmuch_message_decryption_status_t in util/crypto.h */
+if (!msg_crypto->payload_encountered)
+ msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_FULL;
+else if (msg_crypto->decryption_status == NOTMUCH_MESSAGE_DECRYPTED_NONE)
+ msg_crypto->decryption_status = NOTMUCH_MESSAGE_DECRYPTED_PARTIAL;
+
+return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/util/crypto.h b/util/crypto.h
index af3998e8..c6fa7f4b 100644
--- a/util/crypto.h
+++ b/util/crypto.h
@@ -25,6 +25,71 @@ _notmuch_crypto_decrypt (bool *attempted,
void
_notmuch_crypto_cleanup (_notmuch_crypto_t *crypto);
+/* The user probably wants to know if the entire message was in the
+ * clear. When replying, the MUA probably wants to know whether there
+ * was any part decrypted in the message. And when displaying to the
+ * user, we probably only want to display "encrypted message" if the
+ * entire message was covered by encryption. */
+typedef enum {
+NOTMUCH_MESSAGE_DECRYPTED_NONE = 0,
+
[PATCH] test: report summary even when aborting
In certain cases of test suite failure, the summary report was not being printed. In particular, any failure on the parallel test suite, and any aborted test in the serialized test suite would end up hiding the summary. It's better to always show the summary where we can (while preserving the return code). Signed-off-by: Daniel Kahn Gillmor --- test/notmuch-test | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/notmuch-test b/test/notmuch-test index 50ed8721..d835e152 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -45,6 +45,8 @@ else fi trap 'e=$?; kill $!; exit $e' HUP INT TERM + +RES=0 # Run the tests if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then test -t 1 && export COLORS_WITHOUT_TTY=t || : @@ -58,7 +60,6 @@ if test -z "$NOTMUCH_TEST_SERIALIZE" && command -v parallel >/dev/null ; then RES=$? if [[ $RES != 0 ]]; then echo "parallel test suite returned error code $RES" -exit $RES fi else for test in $TESTS; do @@ -69,7 +70,7 @@ else RES=$? testname=$(basename $test .sh) if [[ $RES != 0 && ! -e "$NOTMUCH_BUILDDIR/test/test-results/$testname" ]]; then -exit $RES +echo "Aborting on $testname (returned $RES)" fi done fi @@ -78,7 +79,11 @@ trap - HUP INT TERM # Report results echo $NOTMUCH_SRCDIR/test/aggregate-results.sh $NOTMUCH_BUILDDIR/test/test-results/* -ev=$? +if [ "$RES" = 0 ]; then +ev=$? +else +ev=$RES +fi # Clean up rm -rf $NOTMUCH_BUILDDIR/test/test-results -- 2.20.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
Daniel Kahn Gillmor writes: > When we have not been able to evaluate the signature status of a given > MIME part, showing a content-free (and interaction-free) "[ Unknown > signature status ]" button doesn't really help the user at all, and > takes up valuable screen real-estate. > > A visual reminder that a given message is *not* signed isn't helpful > unless it is always present, in which case we'd want to see "[ Unknown > signature status ]" buttons on all messages, even ones that don't have > a signing structure, but i don't think we want that. amended version pushed to master d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button
On Fri 2019-05-24 22:38:12 -0300, David Bremner wrote: > Daniel Kahn Gillmor writes: > >> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote: >>> Daniel Kahn Gillmor writes: >>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el index 353f721e..68171153 100644 --- a/emacs/notmuch-crypto.el +++ b/emacs/notmuch-crypto.el @@ -93,6 +93,7 @@ mode." (defun notmuch-crypto-insert-sigstatus-button (sigstatus from) (let* ((status (plist-get sigstatus :status)) (help-msg nil) + (show-button t) (label "Signature not processed") >>> >>> This should probably be nil, since that particular value is never used, >>> iiuc. I can amend it if you agree. >> > [snip] >> >> If i've misunderstood the e-lisp (entirely possible!) i would be happy >> to be corrected. > > No, you've just misunderstood my reply. I refer to the existing initialization > of "label", which now seems obsolete to me. Ah, sorry! i understand now, and yes, i agree that label shold be initialized to nil. If you're willing to amend it that sounds good to me. Thanks for catching this. --dkg ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Add notmuch-tree-toggle-order?
Pierre Neidhardt writes: > Hi, > > We have notmuch-search-toggle-order but surprisingly there is no > notmuch-tree-toggle-order. > > Besides, I find the default tree order somewhat counter-intuitive: the > threads are sorted newest at the top, but the messages within the thread > are sorted oldest at the top (which quite expected from a tree display). I guess you're asking if it's reasonable to add? I think so, and the key 'o' is also free in tree mode. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] emacs: Allow tagging regions in notmuch-tree
Pierre Neidhardt writes: > David Bremner writes: > >>> --8<---cut here---start->8--- guix environment notmuch -- /home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh >>> guix environment: error: execlp: No such file or directory: >>> "/home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh" >>> --8<---cut here---end--->8--- >> >> I can't really help you with Guix, but I suggest setting up some >> environment where you can run things in an interactive shell. >> >> In particular that error seems to be claiming the test file doesn't >> exist, which would be easy to debug in an interactive shell. > > Yup, that's what I did with "guix environment notmuch": it sets up a > build environment (and an interactive shell) for notmuch. And I really > wonder why it can't find the file, it's there in the interactive shell. > It's possible that the error message is a red herring though, I'll look > into it. I case it helps, attached is the output from % cd test && ./T460-emacs-tree.sh Note that the tests do need to be run from that directory. The two new tests that actually use emacs are failing with output: *ERROR*: Wrong number of arguments: (1 . 1), 0 Running the tests interactively (just eval testl-lib.el first) suggests that is output from set-mark-command (in emacs 26.1, it demands at least one argument). test.out Description: Binary data > > So would you like me to patch the namespacing along with these changes > or leave it for another patch? I'd leave namespacing of existing code for a new series if you're motivated to work on that. For your new code, I think it makes most sense to have it in the patch that introduces the code. > >> I assume you are not using git-send-email because it's difficult for >> you; it's not that a big of a deal, although we do prefer series >> generated git-send-email for reviewing. > > I'm using git-send-email on a regular basis, no problem with that. (I > wonder why you would think it's difficult for me :p) My mistake, I assume everyone read https://notmuchmail.org/contributing/#index11h2 ;). > git-send-email comes with different workflows though, I'm not sure which > one Notmuch follows. Do you prefer versioned patch series > (e.g. [PATCHv2], etc.) or patch updates sent with > "--in-reply-to="? For series, probably the former. For single patches, or updates to the single patches in the series, the latter. Thanks! d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] test: allow disabling timeout with NOTMUCH_TEST_TIMEOUT=0
Daniel Kahn Gillmor writes: > On Wed 2019-05-22 08:52:55 -0300, David Bremner wrote: >> Daniel Kahn Gillmor writes: >> >>> Tests appear to be hanging when run under GNU timeout on debian >>> stretch. To aid in diagnosing this or similar problems, it's handy to >>> be able to disable timeout from the command line at will. >>> >> >> 1) Do we still need / want this? > > Yes, i think it's useful if you want rule out coreutils when debugging > the test suite. > >> 2) care to reword the commit message? > > Sure, keeping the same subject line, i'd change the body to: Pushed, d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] emacs: Allow tagging regions in notmuch-tree
Pierre Neidhardt writes: Thanks for the updated patches. > >> 3) We generally want at least one test for a new >>feature. test/T460-emacs-tree.sh has the existing tree related >>tests. Again, if you need help with the test suite, let us know. > > I've added a test, but I wasn't able to run it on Guix > (https://guix.info). It fails like this: > > --8<---cut here---start->8--- >> guix environment notmuch -- >> /home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh > guix environment: error: execlp: No such file or directory: > "/home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh" > --8<---cut here---end--->8--- I can't really help you with Guix, but I suggest setting up some environment where you can run things in an interactive shell. In particular that error seems to be claiming the test file doesn't exist, which would be easy to debug in an interactive shell. > >> 4) Please use notmuch-tree-- as a prefix for new private symbols that >>users should not rely on. I'm not sure about which symbols that applies >>to here. > > I've mirrored the implementation of search-mode. > There are indeed a few functions that are only used locally, but so are > they in notmuch.el, so I guess this should be part of another patch. > For better or for worse, our standards are higher for new code. Since (as you have just observed) minor style problems in existing code tend to linger unfixed. I assume you are not using git-send-email because it's difficult for you; it's not that a big of a deal, although we do prefer series generated git-send-email for reviewing. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: Index user defined headers
David Bremner writes: > David Bremner writes: > >> This obsoletes [1]. Compared to the previous version the main change >> is that it imposes the restriction that user defined prefixes may not >> start with [a-z], and must consist of "unicode word characters". This >> assumes a utf8 input encoding. People that don't like utf8 are welcome >> to use ASCII :P >> >> [1]: id:20190302154133.25642-1-da...@tethera.net >> [2]: https://salsa.debian.org/bremner/notmuch/commits/wip/user-headers > > Last call for review or other feedback. I'll merge this in the next week > if I don't hear anything. > > d Next week turned into next month, but these this series is merged to master now. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
