* [PATCH v3 01/15] test: make it possible to have multiple corpora
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 02/15] test: add known broken test for reply to message with multiple Cc headers Jani Nikula
` (15 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
We largely use the corpus under test/corpus for
testing. Unfortunately, many of our tests have grown to depend on
having exactly this set of messages, making it hard to add new message
files for testing specific cases.
We do use a lot of add_message from within the tests, but it's not
possible to use that for adding broken messages, and adding several
messages at once can get unwieldy.
Move the basic corpus under tests/corpora/default, and make it
possible to add new, independent corpora along its side. This means
tons of renames with a few tweaks to add_email_corpus function in
test-lib.sh to let tests specify which corpus to use.
---
test/{corpus => corpora/default}/01:2, | 0
test/{corpus => corpora/default}/02:2, | 0
test/{corpus => corpora/default}/bar/17:2, | 0
test/{corpus => corpora/default}/bar/18:2, | 0
test/{corpus => corpora/default}/bar/baz/05:2, | 0
test/{corpus => corpora/default}/bar/baz/23:2, | 0
test/{corpus => corpora/default}/bar/baz/24:2, | 0
test/{corpus => corpora/default}/bar/baz/cur/25:2, | 0
test/{corpus => corpora/default}/bar/baz/cur/26:2, | 0
test/{corpus => corpora/default}/bar/baz/new/27:2, | 0
test/{corpus => corpora/default}/bar/baz/new/28:2, | 0
test/{corpus => corpora/default}/bar/cur/19:2, | 0
test/{corpus => corpora/default}/bar/cur/20:2, | 0
test/{corpus => corpora/default}/bar/new/21:2, | 0
test/{corpus => corpora/default}/bar/new/22:2, | 0
test/{corpus => corpora/default}/cur/29:2, | 0
test/{corpus => corpora/default}/cur/30:2, | 0
test/{corpus => corpora/default}/cur/31:2, | 0
test/{corpus => corpora/default}/cur/32:2, | 0
test/{corpus => corpora/default}/cur/33:2, | 0
test/{corpus => corpora/default}/cur/34:2, | 0
test/{corpus => corpora/default}/cur/35:2, | 0
test/{corpus => corpora/default}/cur/36:2, | 0
test/{corpus => corpora/default}/cur/37:2, | 0
test/{corpus => corpora/default}/cur/38:2, | 0
test/{corpus => corpora/default}/cur/39:2, | 0
test/{corpus => corpora/default}/cur/40:2, | 0
test/{corpus => corpora/default}/cur/41:2, | 0
test/{corpus => corpora/default}/cur/42:2, | 0
test/{corpus => corpora/default}/cur/43:2, | 0
test/{corpus => corpora/default}/cur/44:2, | 0
test/{corpus => corpora/default}/cur/45:2, | 0
test/{corpus => corpora/default}/cur/46:2, | 0
test/{corpus => corpora/default}/cur/47:2, | 0
test/{corpus => corpora/default}/cur/48:2, | 0
test/{corpus => corpora/default}/cur/49:2, | 0
test/{corpus => corpora/default}/cur/50:2, | 0
test/{corpus => corpora/default}/cur/51:2, | 0
test/{corpus => corpora/default}/cur/52:2, | 0
test/{corpus => corpora/default}/cur/53:2, | 0
test/{corpus => corpora/default}/foo/06:2, | 0
test/{corpus => corpora/default}/foo/baz/11:2, | 0
test/{corpus => corpora/default}/foo/baz/12:2, | 0
test/{corpus => corpora/default}/foo/baz/cur/13:2, | 0
test/{corpus => corpora/default}/foo/baz/cur/14:2, | 0
test/{corpus => corpora/default}/foo/baz/new/15:2, | 0
test/{corpus => corpora/default}/foo/baz/new/16:2, | 0
test/{corpus => corpora/default}/foo/cur/07:2, | 0
test/{corpus => corpora/default}/foo/cur/08:2, | 0
test/{corpus => corpora/default}/foo/new/03:2, | 0
test/{corpus => corpora/default}/foo/new/09:2, | 0
test/{corpus => corpora/default}/foo/new/10:2, | 0
test/{corpus => corpora/default}/new/04:2, | 0
test/test-lib.sh | 19 ++++++++++++-------
54 files changed, 12 insertions(+), 7 deletions(-)
rename test/{corpus => corpora/default}/01:2, (100%)
rename test/{corpus => corpora/default}/02:2, (100%)
rename test/{corpus => corpora/default}/bar/17:2, (100%)
rename test/{corpus => corpora/default}/bar/18:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/05:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/23:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/24:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/cur/25:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/cur/26:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/new/27:2, (100%)
rename test/{corpus => corpora/default}/bar/baz/new/28:2, (100%)
rename test/{corpus => corpora/default}/bar/cur/19:2, (100%)
rename test/{corpus => corpora/default}/bar/cur/20:2, (100%)
rename test/{corpus => corpora/default}/bar/new/21:2, (100%)
rename test/{corpus => corpora/default}/bar/new/22:2, (100%)
rename test/{corpus => corpora/default}/cur/29:2, (100%)
rename test/{corpus => corpora/default}/cur/30:2, (100%)
rename test/{corpus => corpora/default}/cur/31:2, (100%)
rename test/{corpus => corpora/default}/cur/32:2, (100%)
rename test/{corpus => corpora/default}/cur/33:2, (100%)
rename test/{corpus => corpora/default}/cur/34:2, (100%)
rename test/{corpus => corpora/default}/cur/35:2, (100%)
rename test/{corpus => corpora/default}/cur/36:2, (100%)
rename test/{corpus => corpora/default}/cur/37:2, (100%)
rename test/{corpus => corpora/default}/cur/38:2, (100%)
rename test/{corpus => corpora/default}/cur/39:2, (100%)
rename test/{corpus => corpora/default}/cur/40:2, (100%)
rename test/{corpus => corpora/default}/cur/41:2, (100%)
rename test/{corpus => corpora/default}/cur/42:2, (100%)
rename test/{corpus => corpora/default}/cur/43:2, (100%)
rename test/{corpus => corpora/default}/cur/44:2, (100%)
rename test/{corpus => corpora/default}/cur/45:2, (100%)
rename test/{corpus => corpora/default}/cur/46:2, (100%)
rename test/{corpus => corpora/default}/cur/47:2, (100%)
rename test/{corpus => corpora/default}/cur/48:2, (100%)
rename test/{corpus => corpora/default}/cur/49:2, (100%)
rename test/{corpus => corpora/default}/cur/50:2, (100%)
rename test/{corpus => corpora/default}/cur/51:2, (100%)
rename test/{corpus => corpora/default}/cur/52:2, (100%)
rename test/{corpus => corpora/default}/cur/53:2, (100%)
rename test/{corpus => corpora/default}/foo/06:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/11:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/12:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/cur/13:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/cur/14:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/new/15:2, (100%)
rename test/{corpus => corpora/default}/foo/baz/new/16:2, (100%)
rename test/{corpus => corpora/default}/foo/cur/07:2, (100%)
rename test/{corpus => corpora/default}/foo/cur/08:2, (100%)
rename test/{corpus => corpora/default}/foo/new/03:2, (100%)
rename test/{corpus => corpora/default}/foo/new/09:2, (100%)
rename test/{corpus => corpora/default}/foo/new/10:2, (100%)
rename test/{corpus => corpora/default}/new/04:2, (100%)
diff --git a/test/corpus/01:2, b/test/corpora/default/01:2,
similarity index 100%
rename from test/corpus/01:2,
rename to test/corpora/default/01:2,
diff --git a/test/corpus/02:2, b/test/corpora/default/02:2,
similarity index 100%
rename from test/corpus/02:2,
rename to test/corpora/default/02:2,
diff --git a/test/corpus/bar/17:2, b/test/corpora/default/bar/17:2,
similarity index 100%
rename from test/corpus/bar/17:2,
rename to test/corpora/default/bar/17:2,
diff --git a/test/corpus/bar/18:2, b/test/corpora/default/bar/18:2,
similarity index 100%
rename from test/corpus/bar/18:2,
rename to test/corpora/default/bar/18:2,
diff --git a/test/corpus/bar/baz/05:2, b/test/corpora/default/bar/baz/05:2,
similarity index 100%
rename from test/corpus/bar/baz/05:2,
rename to test/corpora/default/bar/baz/05:2,
diff --git a/test/corpus/bar/baz/23:2, b/test/corpora/default/bar/baz/23:2,
similarity index 100%
rename from test/corpus/bar/baz/23:2,
rename to test/corpora/default/bar/baz/23:2,
diff --git a/test/corpus/bar/baz/24:2, b/test/corpora/default/bar/baz/24:2,
similarity index 100%
rename from test/corpus/bar/baz/24:2,
rename to test/corpora/default/bar/baz/24:2,
diff --git a/test/corpus/bar/baz/cur/25:2, b/test/corpora/default/bar/baz/cur/25:2,
similarity index 100%
rename from test/corpus/bar/baz/cur/25:2,
rename to test/corpora/default/bar/baz/cur/25:2,
diff --git a/test/corpus/bar/baz/cur/26:2, b/test/corpora/default/bar/baz/cur/26:2,
similarity index 100%
rename from test/corpus/bar/baz/cur/26:2,
rename to test/corpora/default/bar/baz/cur/26:2,
diff --git a/test/corpus/bar/baz/new/27:2, b/test/corpora/default/bar/baz/new/27:2,
similarity index 100%
rename from test/corpus/bar/baz/new/27:2,
rename to test/corpora/default/bar/baz/new/27:2,
diff --git a/test/corpus/bar/baz/new/28:2, b/test/corpora/default/bar/baz/new/28:2,
similarity index 100%
rename from test/corpus/bar/baz/new/28:2,
rename to test/corpora/default/bar/baz/new/28:2,
diff --git a/test/corpus/bar/cur/19:2, b/test/corpora/default/bar/cur/19:2,
similarity index 100%
rename from test/corpus/bar/cur/19:2,
rename to test/corpora/default/bar/cur/19:2,
diff --git a/test/corpus/bar/cur/20:2, b/test/corpora/default/bar/cur/20:2,
similarity index 100%
rename from test/corpus/bar/cur/20:2,
rename to test/corpora/default/bar/cur/20:2,
diff --git a/test/corpus/bar/new/21:2, b/test/corpora/default/bar/new/21:2,
similarity index 100%
rename from test/corpus/bar/new/21:2,
rename to test/corpora/default/bar/new/21:2,
diff --git a/test/corpus/bar/new/22:2, b/test/corpora/default/bar/new/22:2,
similarity index 100%
rename from test/corpus/bar/new/22:2,
rename to test/corpora/default/bar/new/22:2,
diff --git a/test/corpus/cur/29:2, b/test/corpora/default/cur/29:2,
similarity index 100%
rename from test/corpus/cur/29:2,
rename to test/corpora/default/cur/29:2,
diff --git a/test/corpus/cur/30:2, b/test/corpora/default/cur/30:2,
similarity index 100%
rename from test/corpus/cur/30:2,
rename to test/corpora/default/cur/30:2,
diff --git a/test/corpus/cur/31:2, b/test/corpora/default/cur/31:2,
similarity index 100%
rename from test/corpus/cur/31:2,
rename to test/corpora/default/cur/31:2,
diff --git a/test/corpus/cur/32:2, b/test/corpora/default/cur/32:2,
similarity index 100%
rename from test/corpus/cur/32:2,
rename to test/corpora/default/cur/32:2,
diff --git a/test/corpus/cur/33:2, b/test/corpora/default/cur/33:2,
similarity index 100%
rename from test/corpus/cur/33:2,
rename to test/corpora/default/cur/33:2,
diff --git a/test/corpus/cur/34:2, b/test/corpora/default/cur/34:2,
similarity index 100%
rename from test/corpus/cur/34:2,
rename to test/corpora/default/cur/34:2,
diff --git a/test/corpus/cur/35:2, b/test/corpora/default/cur/35:2,
similarity index 100%
rename from test/corpus/cur/35:2,
rename to test/corpora/default/cur/35:2,
diff --git a/test/corpus/cur/36:2, b/test/corpora/default/cur/36:2,
similarity index 100%
rename from test/corpus/cur/36:2,
rename to test/corpora/default/cur/36:2,
diff --git a/test/corpus/cur/37:2, b/test/corpora/default/cur/37:2,
similarity index 100%
rename from test/corpus/cur/37:2,
rename to test/corpora/default/cur/37:2,
diff --git a/test/corpus/cur/38:2, b/test/corpora/default/cur/38:2,
similarity index 100%
rename from test/corpus/cur/38:2,
rename to test/corpora/default/cur/38:2,
diff --git a/test/corpus/cur/39:2, b/test/corpora/default/cur/39:2,
similarity index 100%
rename from test/corpus/cur/39:2,
rename to test/corpora/default/cur/39:2,
diff --git a/test/corpus/cur/40:2, b/test/corpora/default/cur/40:2,
similarity index 100%
rename from test/corpus/cur/40:2,
rename to test/corpora/default/cur/40:2,
diff --git a/test/corpus/cur/41:2, b/test/corpora/default/cur/41:2,
similarity index 100%
rename from test/corpus/cur/41:2,
rename to test/corpora/default/cur/41:2,
diff --git a/test/corpus/cur/42:2, b/test/corpora/default/cur/42:2,
similarity index 100%
rename from test/corpus/cur/42:2,
rename to test/corpora/default/cur/42:2,
diff --git a/test/corpus/cur/43:2, b/test/corpora/default/cur/43:2,
similarity index 100%
rename from test/corpus/cur/43:2,
rename to test/corpora/default/cur/43:2,
diff --git a/test/corpus/cur/44:2, b/test/corpora/default/cur/44:2,
similarity index 100%
rename from test/corpus/cur/44:2,
rename to test/corpora/default/cur/44:2,
diff --git a/test/corpus/cur/45:2, b/test/corpora/default/cur/45:2,
similarity index 100%
rename from test/corpus/cur/45:2,
rename to test/corpora/default/cur/45:2,
diff --git a/test/corpus/cur/46:2, b/test/corpora/default/cur/46:2,
similarity index 100%
rename from test/corpus/cur/46:2,
rename to test/corpora/default/cur/46:2,
diff --git a/test/corpus/cur/47:2, b/test/corpora/default/cur/47:2,
similarity index 100%
rename from test/corpus/cur/47:2,
rename to test/corpora/default/cur/47:2,
diff --git a/test/corpus/cur/48:2, b/test/corpora/default/cur/48:2,
similarity index 100%
rename from test/corpus/cur/48:2,
rename to test/corpora/default/cur/48:2,
diff --git a/test/corpus/cur/49:2, b/test/corpora/default/cur/49:2,
similarity index 100%
rename from test/corpus/cur/49:2,
rename to test/corpora/default/cur/49:2,
diff --git a/test/corpus/cur/50:2, b/test/corpora/default/cur/50:2,
similarity index 100%
rename from test/corpus/cur/50:2,
rename to test/corpora/default/cur/50:2,
diff --git a/test/corpus/cur/51:2, b/test/corpora/default/cur/51:2,
similarity index 100%
rename from test/corpus/cur/51:2,
rename to test/corpora/default/cur/51:2,
diff --git a/test/corpus/cur/52:2, b/test/corpora/default/cur/52:2,
similarity index 100%
rename from test/corpus/cur/52:2,
rename to test/corpora/default/cur/52:2,
diff --git a/test/corpus/cur/53:2, b/test/corpora/default/cur/53:2,
similarity index 100%
rename from test/corpus/cur/53:2,
rename to test/corpora/default/cur/53:2,
diff --git a/test/corpus/foo/06:2, b/test/corpora/default/foo/06:2,
similarity index 100%
rename from test/corpus/foo/06:2,
rename to test/corpora/default/foo/06:2,
diff --git a/test/corpus/foo/baz/11:2, b/test/corpora/default/foo/baz/11:2,
similarity index 100%
rename from test/corpus/foo/baz/11:2,
rename to test/corpora/default/foo/baz/11:2,
diff --git a/test/corpus/foo/baz/12:2, b/test/corpora/default/foo/baz/12:2,
similarity index 100%
rename from test/corpus/foo/baz/12:2,
rename to test/corpora/default/foo/baz/12:2,
diff --git a/test/corpus/foo/baz/cur/13:2, b/test/corpora/default/foo/baz/cur/13:2,
similarity index 100%
rename from test/corpus/foo/baz/cur/13:2,
rename to test/corpora/default/foo/baz/cur/13:2,
diff --git a/test/corpus/foo/baz/cur/14:2, b/test/corpora/default/foo/baz/cur/14:2,
similarity index 100%
rename from test/corpus/foo/baz/cur/14:2,
rename to test/corpora/default/foo/baz/cur/14:2,
diff --git a/test/corpus/foo/baz/new/15:2, b/test/corpora/default/foo/baz/new/15:2,
similarity index 100%
rename from test/corpus/foo/baz/new/15:2,
rename to test/corpora/default/foo/baz/new/15:2,
diff --git a/test/corpus/foo/baz/new/16:2, b/test/corpora/default/foo/baz/new/16:2,
similarity index 100%
rename from test/corpus/foo/baz/new/16:2,
rename to test/corpora/default/foo/baz/new/16:2,
diff --git a/test/corpus/foo/cur/07:2, b/test/corpora/default/foo/cur/07:2,
similarity index 100%
rename from test/corpus/foo/cur/07:2,
rename to test/corpora/default/foo/cur/07:2,
diff --git a/test/corpus/foo/cur/08:2, b/test/corpora/default/foo/cur/08:2,
similarity index 100%
rename from test/corpus/foo/cur/08:2,
rename to test/corpora/default/foo/cur/08:2,
diff --git a/test/corpus/foo/new/03:2, b/test/corpora/default/foo/new/03:2,
similarity index 100%
rename from test/corpus/foo/new/03:2,
rename to test/corpora/default/foo/new/03:2,
diff --git a/test/corpus/foo/new/09:2, b/test/corpora/default/foo/new/09:2,
similarity index 100%
rename from test/corpus/foo/new/09:2,
rename to test/corpora/default/foo/new/09:2,
diff --git a/test/corpus/foo/new/10:2, b/test/corpora/default/foo/new/10:2,
similarity index 100%
rename from test/corpus/foo/new/10:2,
rename to test/corpora/default/foo/new/10:2,
diff --git a/test/corpus/new/04:2, b/test/corpora/default/new/04:2,
similarity index 100%
rename from test/corpus/new/04:2,
rename to test/corpora/default/new/04:2,
diff --git a/test/test-lib.sh b/test/test-lib.sh
index aac0343ba7bf..e2e26e6f274a 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -541,21 +541,26 @@ emacs_fcc_message ()
notmuch new >/dev/null
}
-# Generate a corpus of email and add it to the database.
+# Add an existing, fixed corpus of email to the database.
#
-# This corpus is fixed, (it happens to be 50 messages from early in
-# the history of the notmuch mailing list), which allows for reliably
+# $1 is the corpus dir under corpora to add, using "default" if unset.
+#
+# The default corpus is based on about 50 messages from early in the
+# history of the notmuch mailing list, which allows for reliably
# testing commands that need to operate on a not-totally-trivial
# number of messages.
add_email_corpus ()
{
+ corpus=${1:-default}
+
rm -rf ${MAIL_DIR}
- if [ -d $TEST_DIRECTORY/corpus.mail ]; then
- cp -a $TEST_DIRECTORY/corpus.mail ${MAIL_DIR}
+ if [ -d $TEST_DIRECTORY/corpora.mail/$corpus ]; then
+ cp -a $TEST_DIRECTORY/corpora.mail/$corpus ${MAIL_DIR}
else
- cp -a $TEST_DIRECTORY/corpus ${MAIL_DIR}
+ cp -a $TEST_DIRECTORY/corpora/$corpus ${MAIL_DIR}
notmuch new >/dev/null || die "'notmuch new' failed while adding email corpus"
- cp -a ${MAIL_DIR} $TEST_DIRECTORY/corpus.mail
+ mkdir -p $TEST_DIRECTORY/corpora.mail
+ cp -a ${MAIL_DIR} $TEST_DIRECTORY/corpora.mail/$corpus
fi
}
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 02/15] test: add known broken test for reply to message with multiple Cc headers
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
2016-09-13 17:14 ` [PATCH v3 01/15] test: make it possible to have multiple corpora Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 03/15] cli/reply: push notmuch reply format abstraction lower in the stack Jani Nikula
` (14 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
As Daniel Kahn Gillmor <dkg@fifthhorseman.net> reports in
id:87d1ngv95p.fsf@alice.fifthhorseman.net, notmuch show combines
multiple Cc: fields into one, while notmuch reply does not. While such
messages are in violation of RFC 5322, it would be reasonable to
expect notmuch to be consistent. Add a known broken test to document
this expectation.
This also starts a new "broken" corpus for messages which are broken.
Details:
The original message is formatted using the message printing in
notmuch-show.c. For Cc:, it uses g_mime_message_get_recipients(),
which apparently combines all Cc: fields into one internally.
The addresses in the reply headers, OTOH, are based on headers queried
through libnotmuch. It boils down to g_mime_object_get_header() in
lib/message-file.c, which returns only the first occurence of header.
---
test/T220-reply.sh | 13 +++++++++++++
test/corpora/README | 11 +++++++++++
test/corpora/broken/broken-cc | 9 +++++++++
3 files changed, 33 insertions(+)
create mode 100644 test/corpora/README
create mode 100644 test/corpora/broken/broken-cc
diff --git a/test/T220-reply.sh b/test/T220-reply.sh
index 30b78f679d97..a72068ddde95 100755
--- a/test/T220-reply.sh
+++ b/test/T220-reply.sh
@@ -253,5 +253,18 @@ test_expect_equal_json "$output" '
}
}'
+test_begin_subtest "Reply to a message with multiple Cc headers"
+test_subtest_known_broken
+add_email_corpus broken
+output=$(notmuch reply id:multiple-cc@example.org)
+test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: wowsers!
+To: Alice <alice@example.org>, Daniel <daniel@example.org>
+Cc: Bob <bob@example.org>, Charles <charles@example.org>
+In-Reply-To: <multiple-cc@example.org>
+References: <multiple-cc@example.org>
+
+On Thu, 16 Jun 2016 22:14:41 -0400, Alice <alice@example.org> wrote:
+> Note the Cc: and cc: headers."
test_done
diff --git a/test/corpora/README b/test/corpora/README
new file mode 100644
index 000000000000..77c48e6e7138
--- /dev/null
+++ b/test/corpora/README
@@ -0,0 +1,11 @@
+This directory contains email corpora for testing.
+
+default
+ The default corpus is based on about 50 messages from early in the
+ history of the notmuch mailing list, which allows for reliably
+ testing commands that need to operate on a not-totally-trivial
+ number of messages.
+
+broken
+ The broken corpus contains messages that are broken and/or RFC
+ non-compliant, ensuring we deal with them in a sane way.
diff --git a/test/corpora/broken/broken-cc b/test/corpora/broken/broken-cc
new file mode 100644
index 000000000000..57ae9ba9742c
--- /dev/null
+++ b/test/corpora/broken/broken-cc
@@ -0,0 +1,9 @@
+From: Alice <alice@example.org>
+To: Daniel <daniel@example.org>
+Cc: Bob <bob@example.org>
+Subject: wowsers!
+cc: Charles <charles@example.org>
+Message-Id: <multiple-cc@example.org>
+Date: Thu, 16 Jun 2016 22:14:41 -0400
+
+Note the Cc: and cc: headers.
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 03/15] cli/reply: push notmuch reply format abstraction lower in the stack
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
2016-09-13 17:14 ` [PATCH v3 01/15] test: make it possible to have multiple corpora Jani Nikula
2016-09-13 17:14 ` [PATCH v3 02/15] test: add known broken test for reply to message with multiple Cc headers Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 04/15] cli/reply: reuse show_reply_headers() in headers-only format Jani Nikula
` (13 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
There's quite a bit of duplication, and some consequent deviation,
between the various notmuch reply format code paths. Perform the query
and message iteration in common code, and make the format specific
functions operate on single messages.
There should be no functional changes.
---
notmuch-reply.c | 216 ++++++++++++++++++++++++++------------------------------
1 file changed, 101 insertions(+), 115 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 49513732e620..847e306f94d2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -598,82 +598,42 @@ create_reply_message(void *ctx,
static int
notmuch_reply_format_default(void *ctx,
notmuch_config_t *config,
- notmuch_query_t *query,
+ notmuch_message_t *message,
notmuch_show_params_t *params,
notmuch_bool_t reply_all,
unused (sprinter_t *sp))
{
GMimeMessage *reply;
- notmuch_messages_t *messages;
- notmuch_message_t *message;
mime_node_t *root;
- notmuch_status_t status;
- status = notmuch_query_search_messages_st (query, &messages);
- if (print_status_query ("notmuch reply", query, status))
+ reply = create_reply_message (ctx, config, message, reply_all);
+ if (!reply)
return 1;
- for (;
- notmuch_messages_valid (messages);
- notmuch_messages_move_to_next (messages))
- {
- message = notmuch_messages_get (messages);
+ show_reply_headers (reply);
- reply = create_reply_message (ctx, config, message, reply_all);
-
- /* If reply creation failed, we're out of memory, so don't
- * bother trying any more messages.
- */
- if (!reply) {
- notmuch_message_destroy (message);
- return 1;
- }
-
- show_reply_headers (reply);
-
- g_object_unref (G_OBJECT (reply));
- reply = NULL;
-
- if (mime_node_open (ctx, message, &(params->crypto), &root) == NOTMUCH_STATUS_SUCCESS) {
- format_part_reply (root);
- talloc_free (root);
- }
+ g_object_unref (G_OBJECT (reply));
- notmuch_message_destroy (message);
+ if (mime_node_open (ctx, message, ¶ms->crypto, &root) == NOTMUCH_STATUS_SUCCESS) {
+ format_part_reply (root);
+ talloc_free (root);
}
+
return 0;
}
static int
notmuch_reply_format_sprinter(void *ctx,
notmuch_config_t *config,
- notmuch_query_t *query,
+ notmuch_message_t *message,
notmuch_show_params_t *params,
notmuch_bool_t reply_all,
sprinter_t *sp)
{
GMimeMessage *reply;
- notmuch_messages_t *messages;
- notmuch_message_t *message;
mime_node_t *node;
- unsigned count;
- notmuch_status_t status;
-
- status = notmuch_query_count_messages_st (query, &count);
- if (print_status_query ("notmuch reply", query, status))
- return 1;
-
- if (count != 1) {
- fprintf (stderr, "Error: search term did not match precisely one message (matched %d messages).\n", count);
- return 1;
- }
- status = notmuch_query_search_messages_st (query, &messages);
- if (print_status_query ("notmuch reply", query, status))
- return 1;
-
- message = notmuch_messages_get (messages);
- if (mime_node_open (ctx, message, &(params->crypto), &node) != NOTMUCH_STATUS_SUCCESS)
+ if (mime_node_open (ctx, message, ¶ms->crypto, &node) != NOTMUCH_STATUS_SUCCESS)
return 1;
reply = create_reply_message (ctx, config, message, reply_all);
@@ -686,7 +646,6 @@ notmuch_reply_format_sprinter(void *ctx,
sp->map_key (sp, "reply-headers");
format_headers_sprinter (sp, reply, TRUE);
g_object_unref (G_OBJECT (reply));
- reply = NULL;
/* Start the original */
sp->map_key (sp, "original");
@@ -694,7 +653,6 @@ notmuch_reply_format_sprinter(void *ctx,
/* End */
sp->end (sp);
- notmuch_message_destroy (message);
return 0;
}
@@ -703,65 +661,48 @@ notmuch_reply_format_sprinter(void *ctx,
static int
notmuch_reply_format_headers_only(void *ctx,
notmuch_config_t *config,
- notmuch_query_t *query,
+ notmuch_message_t *message,
unused (notmuch_show_params_t *params),
notmuch_bool_t reply_all,
unused (sprinter_t *sp))
{
GMimeMessage *reply;
- notmuch_messages_t *messages;
- notmuch_message_t *message;
const char *in_reply_to, *orig_references, *references;
char *reply_headers;
- notmuch_status_t status;
- status = notmuch_query_search_messages_st (query, &messages);
- if (print_status_query ("notmuch reply", query, status))
+ /* The 0 means we do not want headers in a "pretty" order. */
+ reply = g_mime_message_new (0);
+ if (reply == NULL) {
+ fprintf (stderr, "Out of memory\n");
return 1;
+ }
- for (;
- notmuch_messages_valid (messages);
- notmuch_messages_move_to_next (messages))
- {
- message = notmuch_messages_get (messages);
-
- /* The 0 means we do not want headers in a "pretty" order. */
- reply = g_mime_message_new (0);
- if (reply == NULL) {
- fprintf (stderr, "Out of memory\n");
- return 1;
- }
-
- in_reply_to = talloc_asprintf (ctx, "<%s>",
- notmuch_message_get_message_id (message));
-
- g_mime_object_set_header (GMIME_OBJECT (reply),
- "In-Reply-To", in_reply_to);
+ in_reply_to = talloc_asprintf (ctx, "<%s>",
+ notmuch_message_get_message_id (message));
+ g_mime_object_set_header (GMIME_OBJECT (reply), "In-Reply-To", in_reply_to);
- orig_references = notmuch_message_get_header (message, "references");
+ orig_references = notmuch_message_get_header (message, "references");
- /* We print In-Reply-To followed by References because git format-patch treats them
- * specially. Git does not interpret the other headers specially
- */
- references = talloc_asprintf (ctx, "%s%s%s",
- orig_references ? orig_references : "",
- orig_references ? " " : "",
- in_reply_to);
- g_mime_object_set_header (GMIME_OBJECT (reply),
- "References", references);
+ /*
+ * We print In-Reply-To followed by References because git
+ * format-patch treats them specially. Git does not interpret the
+ * other headers specially.
+ */
+ references = talloc_asprintf (ctx, "%s%s%s",
+ orig_references ? orig_references : "",
+ orig_references ? " " : "",
+ in_reply_to);
+ g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
- (void)add_recipients_from_message (reply, config, message, reply_all);
+ (void)add_recipients_from_message (reply, config, message, reply_all);
- reply_headers = g_mime_object_to_string (GMIME_OBJECT (reply));
- printf ("%s", reply_headers);
- free (reply_headers);
+ reply_headers = g_mime_object_to_string (GMIME_OBJECT (reply));
+ printf ("%s", reply_headers);
+ free (reply_headers);
- g_object_unref (G_OBJECT (reply));
- reply = NULL;
+ g_object_unref (G_OBJECT (reply));
- notmuch_message_destroy (message);
- }
return 0;
}
@@ -772,6 +713,70 @@ enum {
FORMAT_HEADERS_ONLY,
};
+static int do_reply(notmuch_config_t *config,
+ notmuch_query_t *query,
+ notmuch_show_params_t *params,
+ int format,
+ notmuch_bool_t reply_all)
+{
+ notmuch_messages_t *messages;
+ notmuch_message_t *message;
+ notmuch_status_t status;
+ struct sprinter *sp = NULL;
+ int ret = 0;
+ int (*reply_format_func) (void *ctx,
+ notmuch_config_t *config,
+ notmuch_message_t *message,
+ notmuch_show_params_t *params,
+ notmuch_bool_t reply_all,
+ struct sprinter *sp);
+
+ if (format == FORMAT_JSON || format == FORMAT_SEXP) {
+ unsigned count;
+
+ status = notmuch_query_count_messages_st (query, &count);
+ if (print_status_query ("notmuch reply", query, status))
+ return 1;
+
+ if (count != 1) {
+ fprintf (stderr, "Error: search term did not match precisely one message (matched %d messages).\n", count);
+ return 1;
+ }
+ }
+
+ if (format == FORMAT_HEADERS_ONLY) {
+ reply_format_func = notmuch_reply_format_headers_only;
+ } else if (format == FORMAT_JSON) {
+ reply_format_func = notmuch_reply_format_sprinter;
+ sp = sprinter_json_create (config, stdout);
+ } else if (format == FORMAT_SEXP) {
+ reply_format_func = notmuch_reply_format_sprinter;
+ sp = sprinter_sexp_create (config, stdout);
+ } else {
+ reply_format_func = notmuch_reply_format_default;
+ }
+
+ status = notmuch_query_search_messages_st (query, &messages);
+ if (print_status_query ("notmuch reply", query, status))
+ return 1;
+
+ for (;
+ notmuch_messages_valid (messages);
+ notmuch_messages_move_to_next (messages))
+ {
+ message = notmuch_messages_get (messages);
+
+ ret = reply_format_func(config, config, message, params, reply_all, sp);
+
+ notmuch_message_destroy (message);
+
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
int
notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
{
@@ -779,12 +784,6 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
notmuch_query_t *query;
char *query_string;
int opt_index;
- int (*reply_format_func) (void *ctx,
- notmuch_config_t *config,
- notmuch_query_t *query,
- notmuch_show_params_t *params,
- notmuch_bool_t reply_all,
- struct sprinter *sp);
notmuch_show_params_t params = {
.part = -1,
.crypto = {
@@ -795,7 +794,6 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
};
int format = FORMAT_DEFAULT;
int reply_all = TRUE;
- struct sprinter *sp = NULL;
notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_KEYWORD, &format, "format", 'f',
@@ -820,18 +818,6 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
notmuch_process_shared_options (argv[0]);
- if (format == FORMAT_HEADERS_ONLY) {
- reply_format_func = notmuch_reply_format_headers_only;
- } else if (format == FORMAT_JSON) {
- reply_format_func = notmuch_reply_format_sprinter;
- sp = sprinter_json_create (config, stdout);
- } else if (format == FORMAT_SEXP) {
- reply_format_func = notmuch_reply_format_sprinter;
- sp = sprinter_sexp_create (config, stdout);
- } else {
- reply_format_func = notmuch_reply_format_default;
- }
-
notmuch_exit_if_unsupported_format ();
query_string = query_string_from_args (config, argc-opt_index, argv+opt_index);
@@ -859,7 +845,7 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[])
return EXIT_FAILURE;
}
- if (reply_format_func (config, config, query, ¶ms, reply_all, sp) != 0)
+ if (do_reply (config, query, ¶ms, format, reply_all) != 0)
return EXIT_FAILURE;
notmuch_crypto_cleanup (¶ms.crypto);
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 04/15] cli/reply: reuse show_reply_headers() in headers-only format
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (2 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 03/15] cli/reply: push notmuch reply format abstraction lower in the stack Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 05/15] cli/reply: unify reply format functions Jani Nikula
` (12 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Align the code with default format reply. There should be no changes
in output.
---
notmuch-reply.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 847e306f94d2..5adbab624ad7 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -668,7 +668,6 @@ notmuch_reply_format_headers_only(void *ctx,
{
GMimeMessage *reply;
const char *in_reply_to, *orig_references, *references;
- char *reply_headers;
/* The 0 means we do not want headers in a "pretty" order. */
reply = g_mime_message_new (0);
@@ -697,9 +696,7 @@ notmuch_reply_format_headers_only(void *ctx,
(void)add_recipients_from_message (reply, config, message, reply_all);
- reply_headers = g_mime_object_to_string (GMIME_OBJECT (reply));
- printf ("%s", reply_headers);
- free (reply_headers);
+ show_reply_headers (reply);
g_object_unref (G_OBJECT (reply));
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 05/15] cli/reply: unify reply format functions
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (3 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 04/15] cli/reply: reuse show_reply_headers() in headers-only format Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 06/15] cli/reply: reorganize create_reply_message() Jani Nikula
` (11 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Prepare for further future unification by making the code similar. The
only functional change is that errors in mime_node_open() also break
execution in default reply format.
---
notmuch-reply.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 5adbab624ad7..4b97ffa4f096 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -604,20 +604,20 @@ notmuch_reply_format_default(void *ctx,
unused (sprinter_t *sp))
{
GMimeMessage *reply;
- mime_node_t *root;
+ mime_node_t *node;
+
+ if (mime_node_open (ctx, message, ¶ms->crypto, &node))
+ return 1;
reply = create_reply_message (ctx, config, message, reply_all);
if (!reply)
return 1;
show_reply_headers (reply);
+ format_part_reply (node);
g_object_unref (G_OBJECT (reply));
-
- if (mime_node_open (ctx, message, ¶ms->crypto, &root) == NOTMUCH_STATUS_SUCCESS) {
- format_part_reply (root);
- talloc_free (root);
- }
+ talloc_free (node);
return 0;
}
@@ -633,7 +633,7 @@ notmuch_reply_format_sprinter(void *ctx,
GMimeMessage *reply;
mime_node_t *node;
- if (mime_node_open (ctx, message, ¶ms->crypto, &node) != NOTMUCH_STATUS_SUCCESS)
+ if (mime_node_open (ctx, message, ¶ms->crypto, &node))
return 1;
reply = create_reply_message (ctx, config, message, reply_all);
@@ -645,7 +645,6 @@ notmuch_reply_format_sprinter(void *ctx,
/* The headers of the reply message we've created */
sp->map_key (sp, "reply-headers");
format_headers_sprinter (sp, reply, TRUE);
- g_object_unref (G_OBJECT (reply));
/* Start the original */
sp->map_key (sp, "original");
@@ -654,6 +653,9 @@ notmuch_reply_format_sprinter(void *ctx,
/* End */
sp->end (sp);
+ g_object_unref (G_OBJECT (reply));
+ talloc_free (node);
+
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 06/15] cli/reply: reorganize create_reply_message()
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (4 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 05/15] cli/reply: unify reply format functions Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 07/15] cli/reply: make references header creation easier to follow Jani Nikula
` (10 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Again, in preparation for later unification, reorganize
create_reply_message() to be more similar to the headers-only format
reply code in notmuch_reply_format_headers_only(). Due to "pretty"
header ordering, there should be no change in output. There should be
no functional changes.
---
notmuch-reply.c | 45 +++++++++++++++++++++------------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 4b97ffa4f096..eb07405591fd 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -532,12 +532,20 @@ create_reply_message(void *ctx,
return NULL;
}
- subject = notmuch_message_get_header (message, "subject");
- if (subject) {
- if (strncasecmp (subject, "Re:", 3))
- subject = talloc_asprintf (ctx, "Re: %s", subject);
- g_mime_message_set_subject (reply, subject);
- }
+ in_reply_to = talloc_asprintf (ctx, "<%s>",
+ notmuch_message_get_message_id (message));
+
+ g_mime_object_set_header (GMIME_OBJECT (reply), "In-Reply-To", in_reply_to);
+
+ orig_references = notmuch_message_get_header (message, "references");
+ if (!orig_references)
+ /* Treat errors like missing References headers. */
+ orig_references = "";
+ references = talloc_asprintf (ctx, "%s%s%s",
+ *orig_references ? orig_references : "",
+ *orig_references ? " " : "",
+ in_reply_to);
+ g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
from_addr = add_recipients_from_message (reply, config,
message, reply_all);
@@ -572,25 +580,14 @@ create_reply_message(void *ctx,
from_addr = talloc_asprintf (ctx, "%s <%s>",
notmuch_config_get_user_name (config),
from_addr);
- g_mime_object_set_header (GMIME_OBJECT (reply),
- "From", from_addr);
-
- in_reply_to = talloc_asprintf (ctx, "<%s>",
- notmuch_message_get_message_id (message));
-
- g_mime_object_set_header (GMIME_OBJECT (reply),
- "In-Reply-To", in_reply_to);
+ g_mime_object_set_header (GMIME_OBJECT (reply), "From", from_addr);
- orig_references = notmuch_message_get_header (message, "references");
- if (!orig_references)
- /* Treat errors like missing References headers. */
- orig_references = "";
- references = talloc_asprintf (ctx, "%s%s%s",
- *orig_references ? orig_references : "",
- *orig_references ? " " : "",
- in_reply_to);
- g_mime_object_set_header (GMIME_OBJECT (reply),
- "References", references);
+ subject = notmuch_message_get_header (message, "subject");
+ if (subject) {
+ if (strncasecmp (subject, "Re:", 3))
+ subject = talloc_asprintf (ctx, "Re: %s", subject);
+ g_mime_message_set_subject (reply, subject);
+ }
return reply;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 07/15] cli/reply: make references header creation easier to follow
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (5 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 06/15] cli/reply: reorganize create_reply_message() Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 08/15] cli/reply: reuse create_reply_message() also for headers-only format Jani Nikula
` (9 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Just use strdup when original references is not available, instead of
trying to cram everything into a monster asprintf. There should be no
functional changes.
---
notmuch-reply.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index eb07405591fd..c2d7402d40ae 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -538,13 +538,12 @@ create_reply_message(void *ctx,
g_mime_object_set_header (GMIME_OBJECT (reply), "In-Reply-To", in_reply_to);
orig_references = notmuch_message_get_header (message, "references");
- if (!orig_references)
- /* Treat errors like missing References headers. */
- orig_references = "";
- references = talloc_asprintf (ctx, "%s%s%s",
- *orig_references ? orig_references : "",
- *orig_references ? " " : "",
- in_reply_to);
+ if (orig_references && *orig_references)
+ references = talloc_asprintf (ctx, "%s %s", orig_references,
+ in_reply_to);
+ else
+ references = talloc_strdup (ctx, in_reply_to);
+
g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
from_addr = add_recipients_from_message (reply, config,
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 08/15] cli/reply: reuse create_reply_message() also for headers-only format
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (6 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 07/15] cli/reply: make references header creation easier to follow Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 09/15] cli/reply: reduce the reply format abstractions Jani Nikula
` (8 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Add an option for "limited" headers for the (slightly misleadingly
named) headers-only format. There should be no functional changes.
---
notmuch-reply.c | 46 +++++++++++++++-------------------------------
1 file changed, 15 insertions(+), 31 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index c2d7402d40ae..daad453efb09 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -520,13 +520,17 @@ static GMimeMessage *
create_reply_message(void *ctx,
notmuch_config_t *config,
notmuch_message_t *message,
- notmuch_bool_t reply_all)
+ notmuch_bool_t reply_all,
+ notmuch_bool_t limited)
{
const char *subject, *from_addr = NULL;
const char *in_reply_to, *orig_references, *references;
- /* The 1 means we want headers in a "pretty" order. */
- GMimeMessage *reply = g_mime_message_new (1);
+ /*
+ * Use the below header order for limited headers, "pretty" order
+ * otherwise.
+ */
+ GMimeMessage *reply = g_mime_message_new (limited ? 0 : 1);
if (reply == NULL) {
fprintf (stderr, "Out of memory\n");
return NULL;
@@ -549,6 +553,10 @@ create_reply_message(void *ctx,
from_addr = add_recipients_from_message (reply, config,
message, reply_all);
+ /* The above is all that is needed for limited headers. */
+ if (limited)
+ return reply;
+
/*
* Sadly, there is no standard way to find out to which email
* address a mail was delivered - what is in the headers depends
@@ -605,7 +613,7 @@ notmuch_reply_format_default(void *ctx,
if (mime_node_open (ctx, message, ¶ms->crypto, &node))
return 1;
- reply = create_reply_message (ctx, config, message, reply_all);
+ reply = create_reply_message (ctx, config, message, reply_all, FALSE);
if (!reply)
return 1;
@@ -632,7 +640,7 @@ notmuch_reply_format_sprinter(void *ctx,
if (mime_node_open (ctx, message, ¶ms->crypto, &node))
return 1;
- reply = create_reply_message (ctx, config, message, reply_all);
+ reply = create_reply_message (ctx, config, message, reply_all, FALSE);
if (!reply)
return 1;
@@ -665,34 +673,10 @@ notmuch_reply_format_headers_only(void *ctx,
unused (sprinter_t *sp))
{
GMimeMessage *reply;
- const char *in_reply_to, *orig_references, *references;
- /* The 0 means we do not want headers in a "pretty" order. */
- reply = g_mime_message_new (0);
- if (reply == NULL) {
- fprintf (stderr, "Out of memory\n");
+ reply = create_reply_message (ctx, config, message, reply_all, TRUE);
+ if (!reply)
return 1;
- }
-
- in_reply_to = talloc_asprintf (ctx, "<%s>",
- notmuch_message_get_message_id (message));
-
- g_mime_object_set_header (GMIME_OBJECT (reply), "In-Reply-To", in_reply_to);
-
- orig_references = notmuch_message_get_header (message, "references");
-
- /*
- * We print In-Reply-To followed by References because git
- * format-patch treats them specially. Git does not interpret the
- * other headers specially.
- */
- references = talloc_asprintf (ctx, "%s%s%s",
- orig_references ? orig_references : "",
- orig_references ? " " : "",
- in_reply_to);
- g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
-
- (void)add_recipients_from_message (reply, config, message, reply_all);
show_reply_headers (reply);
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 09/15] cli/reply: reduce the reply format abstractions
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (7 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 08/15] cli/reply: reuse create_reply_message() also for headers-only format Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 10/15] cli/reply: use dedicated functions for reply to mapping Jani Nikula
` (7 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Now that we've made the various reply formats quite similar to each
other, there's no point in keeping the abstractions. They are now
close enough to be put in one function.
For now, a mime node will be uselessly created for the headers-only
case, but this is insignificant, and may change in the future.
---
notmuch-reply.c | 145 ++++++++++++++------------------------------------------
1 file changed, 36 insertions(+), 109 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index daad453efb09..b380678e7204 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -599,92 +599,6 @@ create_reply_message(void *ctx,
return reply;
}
-static int
-notmuch_reply_format_default(void *ctx,
- notmuch_config_t *config,
- notmuch_message_t *message,
- notmuch_show_params_t *params,
- notmuch_bool_t reply_all,
- unused (sprinter_t *sp))
-{
- GMimeMessage *reply;
- mime_node_t *node;
-
- if (mime_node_open (ctx, message, ¶ms->crypto, &node))
- return 1;
-
- reply = create_reply_message (ctx, config, message, reply_all, FALSE);
- if (!reply)
- return 1;
-
- show_reply_headers (reply);
- format_part_reply (node);
-
- g_object_unref (G_OBJECT (reply));
- talloc_free (node);
-
- return 0;
-}
-
-static int
-notmuch_reply_format_sprinter(void *ctx,
- notmuch_config_t *config,
- notmuch_message_t *message,
- notmuch_show_params_t *params,
- notmuch_bool_t reply_all,
- sprinter_t *sp)
-{
- GMimeMessage *reply;
- mime_node_t *node;
-
- if (mime_node_open (ctx, message, ¶ms->crypto, &node))
- return 1;
-
- reply = create_reply_message (ctx, config, message, reply_all, FALSE);
- if (!reply)
- return 1;
-
- sp->begin_map (sp);
-
- /* The headers of the reply message we've created */
- sp->map_key (sp, "reply-headers");
- format_headers_sprinter (sp, reply, TRUE);
-
- /* Start the original */
- sp->map_key (sp, "original");
- format_part_sprinter (ctx, sp, node, TRUE, TRUE, FALSE);
-
- /* End */
- sp->end (sp);
-
- g_object_unref (G_OBJECT (reply));
- talloc_free (node);
-
- return 0;
-}
-
-/* This format is currently tuned for a git send-email --notmuch hook */
-static int
-notmuch_reply_format_headers_only(void *ctx,
- notmuch_config_t *config,
- notmuch_message_t *message,
- unused (notmuch_show_params_t *params),
- notmuch_bool_t reply_all,
- unused (sprinter_t *sp))
-{
- GMimeMessage *reply;
-
- reply = create_reply_message (ctx, config, message, reply_all, TRUE);
- if (!reply)
- return 1;
-
- show_reply_headers (reply);
-
- g_object_unref (G_OBJECT (reply));
-
- return 0;
-}
-
enum {
FORMAT_DEFAULT,
FORMAT_JSON,
@@ -698,17 +612,12 @@ static int do_reply(notmuch_config_t *config,
int format,
notmuch_bool_t reply_all)
{
+ GMimeMessage *reply;
+ mime_node_t *node;
notmuch_messages_t *messages;
notmuch_message_t *message;
notmuch_status_t status;
struct sprinter *sp = NULL;
- int ret = 0;
- int (*reply_format_func) (void *ctx,
- notmuch_config_t *config,
- notmuch_message_t *message,
- notmuch_show_params_t *params,
- notmuch_bool_t reply_all,
- struct sprinter *sp);
if (format == FORMAT_JSON || format == FORMAT_SEXP) {
unsigned count;
@@ -721,18 +630,11 @@ static int do_reply(notmuch_config_t *config,
fprintf (stderr, "Error: search term did not match precisely one message (matched %d messages).\n", count);
return 1;
}
- }
- if (format == FORMAT_HEADERS_ONLY) {
- reply_format_func = notmuch_reply_format_headers_only;
- } else if (format == FORMAT_JSON) {
- reply_format_func = notmuch_reply_format_sprinter;
- sp = sprinter_json_create (config, stdout);
- } else if (format == FORMAT_SEXP) {
- reply_format_func = notmuch_reply_format_sprinter;
- sp = sprinter_sexp_create (config, stdout);
- } else {
- reply_format_func = notmuch_reply_format_default;
+ if (format == FORMAT_JSON)
+ sp = sprinter_json_create (config, stdout);
+ else
+ sp = sprinter_sexp_create (config, stdout);
}
status = notmuch_query_search_messages_st (query, &messages);
@@ -745,15 +647,40 @@ static int do_reply(notmuch_config_t *config,
{
message = notmuch_messages_get (messages);
- ret = reply_format_func(config, config, message, params, reply_all, sp);
+ if (mime_node_open (config, message, ¶ms->crypto, &node))
+ return 1;
- notmuch_message_destroy (message);
+ reply = create_reply_message (config, config, message, reply_all,
+ format == FORMAT_HEADERS_ONLY);
+ if (!reply)
+ return 1;
- if (ret)
- break;
+ if (format == FORMAT_JSON || format == FORMAT_SEXP) {
+ sp->begin_map (sp);
+
+ /* The headers of the reply message we've created */
+ sp->map_key (sp, "reply-headers");
+ format_headers_sprinter (sp, reply, TRUE);
+
+ /* Start the original */
+ sp->map_key (sp, "original");
+ format_part_sprinter (config, sp, node, TRUE, TRUE, FALSE);
+
+ /* End */
+ sp->end (sp);
+ } else {
+ show_reply_headers (reply);
+ if (format == FORMAT_DEFAULT)
+ format_part_reply (node);
+ }
+
+ g_object_unref (G_OBJECT (reply));
+ talloc_free (node);
+
+ notmuch_message_destroy (message);
}
- return ret;
+ return 0;
}
int
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 10/15] cli/reply: use dedicated functions for reply to mapping
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (8 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 09/15] cli/reply: reduce the reply format abstractions Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 11/15] cli/reply: check for NULL list first in scan_address_list() Jani Nikula
` (6 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
The main motivation here is to move the special casing around
reply-to/from handling into a function of its own, clarifying the main
logic.
---
notmuch-reply.c | 82 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 49 insertions(+), 33 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index b380678e7204..9b78ea2c2b20 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -256,17 +256,13 @@ scan_address_string (const char *recipients,
* in either the 'To' or 'Cc' header of the message?
*/
static int
-reply_to_header_is_redundant (notmuch_message_t *message)
+reply_to_header_is_redundant (notmuch_message_t *message, const char *reply_to)
{
- const char *reply_to, *to, *cc, *addr;
+ const char *to, *cc, *addr;
InternetAddressList *list;
InternetAddress *address;
InternetAddressMailbox *mailbox;
- reply_to = notmuch_message_get_header (message, "reply-to");
- if (reply_to == NULL || *reply_to == '\0')
- return 0;
-
list = internet_address_list_parse_string (reply_to);
if (internet_address_list_length (list) != 1)
@@ -291,6 +287,47 @@ reply_to_header_is_redundant (notmuch_message_t *message)
return 0;
}
+static const char *get_sender(notmuch_message_t *message)
+{
+ const char *reply_to;
+
+ reply_to = notmuch_message_get_header (message, "reply-to");
+ if (reply_to && *reply_to) {
+ /*
+ * Some mailing lists munge the Reply-To header despite it
+ * being A Bad Thing, see
+ * http://marc.merlins.org/netrants/reply-to-harmful.html
+ *
+ * The munging is easy to detect, because it results in a
+ * redundant reply-to header, (with an address that already
+ * exists in either To or Cc). So in this case, we ignore the
+ * Reply-To field and use the From header. This ensures the
+ * original sender will get the reply even if not subscribed
+ * to the list. Note that the address in the Reply-To header
+ * will always appear in the reply if reply_all is true.
+ */
+ if (! reply_to_header_is_redundant (message, reply_to))
+ return reply_to;
+ }
+
+ return notmuch_message_get_header (message, "from");
+}
+
+static const char *get_to(notmuch_message_t *message)
+{
+ return notmuch_message_get_header (message, "to");
+}
+
+static const char *get_cc(notmuch_message_t *message)
+{
+ return notmuch_message_get_header (message, "cc");
+}
+
+static const char *get_bcc(notmuch_message_t *message)
+{
+ return notmuch_message_get_header (message, "bcc");
+}
+
/* Augment the recipients of 'reply' from the "Reply-to:", "From:",
* "To:", "Cc:", and "Bcc:" headers of 'message'.
*
@@ -310,43 +347,22 @@ add_recipients_from_message (GMimeMessage *reply,
notmuch_bool_t reply_all)
{
struct {
- const char *header;
- const char *fallback;
+ const char * (*get_header)(notmuch_message_t *message);
GMimeRecipientType recipient_type;
} reply_to_map[] = {
- { "reply-to", "from", GMIME_RECIPIENT_TYPE_TO },
- { "to", NULL, GMIME_RECIPIENT_TYPE_TO },
- { "cc", NULL, GMIME_RECIPIENT_TYPE_CC },
- { "bcc", NULL, GMIME_RECIPIENT_TYPE_BCC }
+ { get_sender, GMIME_RECIPIENT_TYPE_TO },
+ { get_to, GMIME_RECIPIENT_TYPE_TO },
+ { get_cc, GMIME_RECIPIENT_TYPE_CC },
+ { get_bcc, GMIME_RECIPIENT_TYPE_BCC },
};
const char *from_addr = NULL;
unsigned int i;
unsigned int n = 0;
- /* Some mailing lists munge the Reply-To header despite it being A Bad
- * Thing, see http://marc.merlins.org/netrants/reply-to-harmful.html
- *
- * The munging is easy to detect, because it results in a
- * redundant reply-to header, (with an address that already exists
- * in either To or Cc). So in this case, we ignore the Reply-To
- * field and use the From header. This ensures the original sender
- * will get the reply even if not subscribed to the list. Note
- * that the address in the Reply-To header will always appear in
- * the reply if reply_all is true.
- */
- if (reply_to_header_is_redundant (message)) {
- reply_to_map[0].header = "from";
- reply_to_map[0].fallback = NULL;
- }
-
for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
const char *recipients;
- recipients = notmuch_message_get_header (message,
- reply_to_map[i].header);
- if ((recipients == NULL || recipients[0] == '\0') && reply_to_map[i].fallback)
- recipients = notmuch_message_get_header (message,
- reply_to_map[i].fallback);
+ recipients = reply_to_map[i].get_header (message);
n += scan_address_string (recipients, config, reply,
reply_to_map[i].recipient_type, &from_addr);
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 11/15] cli/reply: check for NULL list first in scan_address_list()
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (9 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 10/15] cli/reply: use dedicated functions for reply to mapping Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 12/15] cli/reply: return internet address list from get header funcs Jani Nikula
` (5 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Support passing NULL list later on. Also use it to simplify the
recursion.
---
notmuch-reply.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9b78ea2c2b20..d90f46f9bed3 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -192,6 +192,9 @@ scan_address_list (InternetAddressList *list,
int i;
unsigned int n = 0;
+ if (list == NULL)
+ return 0;
+
for (i = 0; i < internet_address_list_length (list); i++) {
address = internet_address_list_get_address (list, i);
if (INTERNET_ADDRESS_IS_GROUP (address)) {
@@ -200,9 +203,6 @@ scan_address_list (InternetAddressList *list,
group = INTERNET_ADDRESS_GROUP (address);
group_list = internet_address_group_get_members (group);
- if (group_list == NULL)
- continue;
-
n += scan_address_list (group_list, config, message, type, user_from);
} else {
InternetAddressMailbox *mailbox;
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 12/15] cli/reply: return internet address list from get header funcs
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (10 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 11/15] cli/reply: check for NULL list first in scan_address_list() Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 13/15] cli/reply: do not parse Reply-To: header into internet address list twice Jani Nikula
` (4 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Pass in GMimeMessage to simplify To/Cc/Bcc headers. We'll eventually
remove the notmuch message passing altogether, but keep both for now
to not make too big changes at once.
Getting the headers from GMimeMessage using GMime functions fixes the
error on duplicate Cc headers reported by Daniel Kahn Gillmor
<dkg@fifthhorseman.net> in id:87d1ngv95p.fsf@alice.fifthhorseman.net.
Get rid of an intermediate function.
The small annoyance is the ownership differences in the address lists.
---
notmuch-reply.c | 73 ++++++++++++++++++++++--------------------------------
test/T220-reply.sh | 1 -
2 files changed, 30 insertions(+), 44 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index d90f46f9bed3..98034485c546 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -227,31 +227,6 @@ scan_address_list (InternetAddressList *list,
return n;
}
-/* Scan addresses in 'recipients'.
- *
- * See the documentation of scan_address_list() above. This function
- * does exactly the same, but converts 'recipients' to an
- * InternetAddressList first.
- */
-static unsigned int
-scan_address_string (const char *recipients,
- notmuch_config_t *config,
- GMimeMessage *message,
- GMimeRecipientType type,
- const char **user_from)
-{
- InternetAddressList *list;
-
- if (recipients == NULL)
- return 0;
-
- list = internet_address_list_parse_string (recipients);
- if (list == NULL)
- return 0;
-
- return scan_address_list (list, config, message, type, user_from);
-}
-
/* Does the address in the Reply-To header of 'message' already appear
* in either the 'To' or 'Cc' header of the message?
*/
@@ -287,11 +262,12 @@ reply_to_header_is_redundant (notmuch_message_t *message, const char *reply_to)
return 0;
}
-static const char *get_sender(notmuch_message_t *message)
+static InternetAddressList *get_sender(notmuch_message_t *message,
+ GMimeMessage *mime_message)
{
const char *reply_to;
- reply_to = notmuch_message_get_header (message, "reply-to");
+ reply_to = g_mime_message_get_reply_to (mime_message);
if (reply_to && *reply_to) {
/*
* Some mailing lists munge the Reply-To header despite it
@@ -307,25 +283,32 @@ static const char *get_sender(notmuch_message_t *message)
* will always appear in the reply if reply_all is true.
*/
if (! reply_to_header_is_redundant (message, reply_to))
- return reply_to;
+ return internet_address_list_parse_string (reply_to);
}
- return notmuch_message_get_header (message, "from");
+ return internet_address_list_parse_string (
+ g_mime_message_get_sender (mime_message));
}
-static const char *get_to(notmuch_message_t *message)
+static InternetAddressList *get_to(unused(notmuch_message_t *message),
+ GMimeMessage *mime_message)
{
- return notmuch_message_get_header (message, "to");
+ return g_mime_message_get_recipients (mime_message,
+ GMIME_RECIPIENT_TYPE_TO);
}
-static const char *get_cc(notmuch_message_t *message)
+static InternetAddressList *get_cc(unused(notmuch_message_t *message),
+ GMimeMessage *mime_message)
{
- return notmuch_message_get_header (message, "cc");
+ return g_mime_message_get_recipients (mime_message,
+ GMIME_RECIPIENT_TYPE_CC);
}
-static const char *get_bcc(notmuch_message_t *message)
+static InternetAddressList *get_bcc(unused(notmuch_message_t *message),
+ GMimeMessage *mime_message)
{
- return notmuch_message_get_header (message, "bcc");
+ return g_mime_message_get_recipients (mime_message,
+ GMIME_RECIPIENT_TYPE_BCC);
}
/* Augment the recipients of 'reply' from the "Reply-to:", "From:",
@@ -344,10 +327,12 @@ static const char *
add_recipients_from_message (GMimeMessage *reply,
notmuch_config_t *config,
notmuch_message_t *message,
+ GMimeMessage *mime_message,
notmuch_bool_t reply_all)
{
struct {
- const char * (*get_header)(notmuch_message_t *message);
+ InternetAddressList * (*get_header)(notmuch_message_t *message,
+ GMimeMessage *mime_message);
GMimeRecipientType recipient_type;
} reply_to_map[] = {
{ get_sender, GMIME_RECIPIENT_TYPE_TO },
@@ -360,12 +345,12 @@ add_recipients_from_message (GMimeMessage *reply,
unsigned int n = 0;
for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
- const char *recipients;
+ InternetAddressList *recipients;
- recipients = reply_to_map[i].get_header (message);
+ recipients = reply_to_map[i].get_header (message, mime_message);
- n += scan_address_string (recipients, config, reply,
- reply_to_map[i].recipient_type, &from_addr);
+ n += scan_address_list (recipients, config, reply,
+ reply_to_map[i].recipient_type, &from_addr);
if (!reply_all && n) {
/* Stop adding new recipients in reply-to-sender mode if
@@ -536,6 +521,7 @@ static GMimeMessage *
create_reply_message(void *ctx,
notmuch_config_t *config,
notmuch_message_t *message,
+ GMimeMessage *mime_message,
notmuch_bool_t reply_all,
notmuch_bool_t limited)
{
@@ -566,8 +552,8 @@ create_reply_message(void *ctx,
g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
- from_addr = add_recipients_from_message (reply, config,
- message, reply_all);
+ from_addr = add_recipients_from_message (reply, config, message,
+ mime_message, reply_all);
/* The above is all that is needed for limited headers. */
if (limited)
@@ -666,7 +652,8 @@ static int do_reply(notmuch_config_t *config,
if (mime_node_open (config, message, ¶ms->crypto, &node))
return 1;
- reply = create_reply_message (config, config, message, reply_all,
+ reply = create_reply_message (config, config, message,
+ GMIME_MESSAGE (node->part), reply_all,
format == FORMAT_HEADERS_ONLY);
if (!reply)
return 1;
diff --git a/test/T220-reply.sh b/test/T220-reply.sh
index a72068ddde95..818a86541496 100755
--- a/test/T220-reply.sh
+++ b/test/T220-reply.sh
@@ -254,7 +254,6 @@ test_expect_equal_json "$output" '
}'
test_begin_subtest "Reply to a message with multiple Cc headers"
-test_subtest_known_broken
add_email_corpus broken
output=$(notmuch reply id:multiple-cc@example.org)
test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 13/15] cli/reply: do not parse Reply-To: header into internet address list twice
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (11 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 12/15] cli/reply: return internet address list from get header funcs Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 14/15] cli/reply: pass gmime message to Reply-To: redundancy detection Jani Nikula
` (3 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Avoid parsing Reply-To: header into internet address list twice. Move
the parsing outside of reply_to_header_is_redundant(), and pass the
parsed internet address list in as parameter. This also avoids leaking
the memory of one copy of the internet address list.
---
notmuch-reply.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 98034485c546..cf4248bd6794 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -231,19 +231,18 @@ scan_address_list (InternetAddressList *list,
* in either the 'To' or 'Cc' header of the message?
*/
static int
-reply_to_header_is_redundant (notmuch_message_t *message, const char *reply_to)
+reply_to_header_is_redundant (notmuch_message_t *message,
+ InternetAddressList *reply_to_list)
{
const char *to, *cc, *addr;
- InternetAddressList *list;
InternetAddress *address;
InternetAddressMailbox *mailbox;
- list = internet_address_list_parse_string (reply_to);
-
- if (internet_address_list_length (list) != 1)
+ if (reply_to_list == NULL ||
+ internet_address_list_length (reply_to_list) != 1)
return 0;
- address = internet_address_list_get_address (list, 0);
+ address = internet_address_list_get_address (reply_to_list, 0);
if (INTERNET_ADDRESS_IS_GROUP (address))
return 0;
@@ -269,6 +268,8 @@ static InternetAddressList *get_sender(notmuch_message_t *message,
reply_to = g_mime_message_get_reply_to (mime_message);
if (reply_to && *reply_to) {
+ InternetAddressList *reply_to_list;
+
/*
* Some mailing lists munge the Reply-To header despite it
* being A Bad Thing, see
@@ -282,8 +283,11 @@ static InternetAddressList *get_sender(notmuch_message_t *message,
* to the list. Note that the address in the Reply-To header
* will always appear in the reply if reply_all is true.
*/
- if (! reply_to_header_is_redundant (message, reply_to))
- return internet_address_list_parse_string (reply_to);
+ reply_to_list = internet_address_list_parse_string (reply_to);
+ if (! reply_to_header_is_redundant (message, reply_to_list))
+ return reply_to_list;
+
+ g_object_unref (G_OBJECT (reply_to_list));
}
return internet_address_list_parse_string (
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 14/15] cli/reply: pass gmime message to Reply-To: redundancy detection
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (12 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 13/15] cli/reply: do not parse Reply-To: header into internet address list twice Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 17:14 ` [PATCH v3 15/15] cli/reply: only pass gmime message to add recipients to reply message Jani Nikula
` (2 subsequent siblings)
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
Use gmime message instead of notmuch message in Reply-To: redundancy
detection. This allows us to easily iterate over all recipient email
addresses accurately, instead of just scanning for strings in the
relevant message headers. This improves the accuracy of the detection
in many ways.
This also makes the notmuch message parameter to get_sender()
unused. This will be cleaned up in a follow-up patch to not make too
many changes here at once.
---
notmuch-reply.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index cf4248bd6794..5421ca80116e 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -230,13 +230,16 @@ scan_address_list (InternetAddressList *list,
/* Does the address in the Reply-To header of 'message' already appear
* in either the 'To' or 'Cc' header of the message?
*/
-static int
-reply_to_header_is_redundant (notmuch_message_t *message,
+static notmuch_bool_t
+reply_to_header_is_redundant (GMimeMessage *message,
InternetAddressList *reply_to_list)
{
- const char *to, *cc, *addr;
+ const char *addr, *reply_to;
InternetAddress *address;
InternetAddressMailbox *mailbox;
+ InternetAddressList *recipients;
+ notmuch_bool_t ret = FALSE;
+ int i;
if (reply_to_list == NULL ||
internet_address_list_length (reply_to_list) != 1)
@@ -247,21 +250,29 @@ reply_to_header_is_redundant (notmuch_message_t *message,
return 0;
mailbox = INTERNET_ADDRESS_MAILBOX (address);
- addr = internet_address_mailbox_get_addr (mailbox);
+ reply_to = internet_address_mailbox_get_addr (mailbox);
- to = notmuch_message_get_header (message, "to");
- cc = notmuch_message_get_header (message, "cc");
+ recipients = g_mime_message_get_all_recipients (message);
- if ((to && strstr (to, addr) != 0) ||
- (cc && strstr (cc, addr) != 0))
- {
- return 1;
+ for (i = 0; i < internet_address_list_length (recipients); i++) {
+ address = internet_address_list_get_address (recipients, i);
+ if (INTERNET_ADDRESS_IS_GROUP (address))
+ continue;
+
+ mailbox = INTERNET_ADDRESS_MAILBOX (address);
+ addr = internet_address_mailbox_get_addr (mailbox);
+ if (strcmp (addr, reply_to) == 0) {
+ ret = TRUE;
+ break;
+ }
}
- return 0;
+ g_object_unref (G_OBJECT (recipients));
+
+ return ret;
}
-static InternetAddressList *get_sender(notmuch_message_t *message,
+static InternetAddressList *get_sender(unused(notmuch_message_t *message),
GMimeMessage *mime_message)
{
const char *reply_to;
@@ -284,7 +295,7 @@ static InternetAddressList *get_sender(notmuch_message_t *message,
* will always appear in the reply if reply_all is true.
*/
reply_to_list = internet_address_list_parse_string (reply_to);
- if (! reply_to_header_is_redundant (message, reply_to_list))
+ if (! reply_to_header_is_redundant (mime_message, reply_to_list))
return reply_to_list;
g_object_unref (G_OBJECT (reply_to_list));
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 15/15] cli/reply: only pass gmime message to add recipients to reply message
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (13 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 14/15] cli/reply: pass gmime message to Reply-To: redundancy detection Jani Nikula
@ 2016-09-13 17:14 ` Jani Nikula
2016-09-13 19:03 ` [PATCH v3 00/15] reply refactor, fixes Tomi Ollila
2016-09-17 12:25 ` David Bremner
16 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2016-09-13 17:14 UTC (permalink / raw)
To: notmuch
The notmuch message is no longer needed. Simplify.
---
notmuch-reply.c | 37 ++++++++++++++-----------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 5421ca80116e..8c894974485d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -272,12 +272,11 @@ reply_to_header_is_redundant (GMimeMessage *message,
return ret;
}
-static InternetAddressList *get_sender(unused(notmuch_message_t *message),
- GMimeMessage *mime_message)
+static InternetAddressList *get_sender(GMimeMessage *message)
{
const char *reply_to;
- reply_to = g_mime_message_get_reply_to (mime_message);
+ reply_to = g_mime_message_get_reply_to (message);
if (reply_to && *reply_to) {
InternetAddressList *reply_to_list;
@@ -295,35 +294,29 @@ static InternetAddressList *get_sender(unused(notmuch_message_t *message),
* will always appear in the reply if reply_all is true.
*/
reply_to_list = internet_address_list_parse_string (reply_to);
- if (! reply_to_header_is_redundant (mime_message, reply_to_list))
+ if (! reply_to_header_is_redundant (message, reply_to_list))
return reply_to_list;
g_object_unref (G_OBJECT (reply_to_list));
}
return internet_address_list_parse_string (
- g_mime_message_get_sender (mime_message));
+ g_mime_message_get_sender (message));
}
-static InternetAddressList *get_to(unused(notmuch_message_t *message),
- GMimeMessage *mime_message)
+static InternetAddressList *get_to(GMimeMessage *message)
{
- return g_mime_message_get_recipients (mime_message,
- GMIME_RECIPIENT_TYPE_TO);
+ return g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
}
-static InternetAddressList *get_cc(unused(notmuch_message_t *message),
- GMimeMessage *mime_message)
+static InternetAddressList *get_cc(GMimeMessage *message)
{
- return g_mime_message_get_recipients (mime_message,
- GMIME_RECIPIENT_TYPE_CC);
+ return g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
}
-static InternetAddressList *get_bcc(unused(notmuch_message_t *message),
- GMimeMessage *mime_message)
+static InternetAddressList *get_bcc(GMimeMessage *message)
{
- return g_mime_message_get_recipients (mime_message,
- GMIME_RECIPIENT_TYPE_BCC);
+ return g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_BCC);
}
/* Augment the recipients of 'reply' from the "Reply-to:", "From:",
@@ -341,13 +334,11 @@ static InternetAddressList *get_bcc(unused(notmuch_message_t *message),
static const char *
add_recipients_from_message (GMimeMessage *reply,
notmuch_config_t *config,
- notmuch_message_t *message,
- GMimeMessage *mime_message,
+ GMimeMessage *message,
notmuch_bool_t reply_all)
{
struct {
- InternetAddressList * (*get_header)(notmuch_message_t *message,
- GMimeMessage *mime_message);
+ InternetAddressList * (*get_header)(GMimeMessage *message);
GMimeRecipientType recipient_type;
} reply_to_map[] = {
{ get_sender, GMIME_RECIPIENT_TYPE_TO },
@@ -362,7 +353,7 @@ add_recipients_from_message (GMimeMessage *reply,
for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
InternetAddressList *recipients;
- recipients = reply_to_map[i].get_header (message, mime_message);
+ recipients = reply_to_map[i].get_header (message);
n += scan_address_list (recipients, config, reply,
reply_to_map[i].recipient_type, &from_addr);
@@ -567,7 +558,7 @@ create_reply_message(void *ctx,
g_mime_object_set_header (GMIME_OBJECT (reply), "References", references);
- from_addr = add_recipients_from_message (reply, config, message,
+ from_addr = add_recipients_from_message (reply, config,
mime_message, reply_all);
/* The above is all that is needed for limited headers. */
--
2.1.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 00/15] reply refactor, fixes
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (14 preceding siblings ...)
2016-09-13 17:14 ` [PATCH v3 15/15] cli/reply: only pass gmime message to add recipients to reply message Jani Nikula
@ 2016-09-13 19:03 ` Tomi Ollila
2016-09-17 12:25 ` David Bremner
16 siblings, 0 replies; 18+ messages in thread
From: Tomi Ollila @ 2016-09-13 19:03 UTC (permalink / raw)
To: Jani Nikula, notmuch
On Tue, Sep 13 2016, Jani Nikula <jani@nikula.org> wrote:
> Updated version of [1] to address David's review comments, mostly just
> commit message updates. Also incorporates the multiple corpora support
> from [2], with the commit message extended a bit, and rebasing patch 2
> on top of it.
I like how things evolve in this patch series.
I did not notice anything to to be changed (imo).
I did not test this patch series (yet).
+1
Tomi
>
> BR,
> Jani.
>
>
> [1] id:cover.1471088022.git.jani@nikula.org
> [2] id:1473609824-6258-1-git-send-email-jani@nikula.org
>
>
> Jani Nikula (15):
> test: make it possible to have multiple corpora
> test: add known broken test for reply to message with multiple Cc
> headers
> cli/reply: push notmuch reply format abstraction lower in the stack
> cli/reply: reuse show_reply_headers() in headers-only format
> cli/reply: unify reply format functions
> cli/reply: reorganize create_reply_message()
> cli/reply: make references header creation easier to follow
> cli/reply: reuse create_reply_message() also for headers-only format
> cli/reply: reduce the reply format abstractions
> cli/reply: use dedicated functions for reply to mapping
> cli/reply: check for NULL list first in scan_address_list()
> cli/reply: return internet address list from get header funcs
> cli/reply: do not parse Reply-To: header into internet address list
> twice
> cli/reply: pass gmime message to Reply-To: redundancy detection
> cli/reply: only pass gmime message to add recipients to reply message
>
> notmuch-reply.c | 435 ++++++++-------------
> test/T220-reply.sh | 12 +
> test/corpora/README | 11 +
> test/corpora/broken/broken-cc | 9 +
> test/{corpus => corpora/default}/01:2, | 0
> test/{corpus => corpora/default}/02:2, | 0
> test/{corpus => corpora/default}/bar/17:2, | 0
> test/{corpus => corpora/default}/bar/18:2, | 0
> test/{corpus => corpora/default}/bar/baz/05:2, | 0
> test/{corpus => corpora/default}/bar/baz/23:2, | 0
> test/{corpus => corpora/default}/bar/baz/24:2, | 0
> test/{corpus => corpora/default}/bar/baz/cur/25:2, | 0
> test/{corpus => corpora/default}/bar/baz/cur/26:2, | 0
> test/{corpus => corpora/default}/bar/baz/new/27:2, | 0
> test/{corpus => corpora/default}/bar/baz/new/28:2, | 0
> test/{corpus => corpora/default}/bar/cur/19:2, | 0
> test/{corpus => corpora/default}/bar/cur/20:2, | 0
> test/{corpus => corpora/default}/bar/new/21:2, | 0
> test/{corpus => corpora/default}/bar/new/22:2, | 0
> test/{corpus => corpora/default}/cur/29:2, | 0
> test/{corpus => corpora/default}/cur/30:2, | 0
> test/{corpus => corpora/default}/cur/31:2, | 0
> test/{corpus => corpora/default}/cur/32:2, | 0
> test/{corpus => corpora/default}/cur/33:2, | 0
> test/{corpus => corpora/default}/cur/34:2, | 0
> test/{corpus => corpora/default}/cur/35:2, | 0
> test/{corpus => corpora/default}/cur/36:2, | 0
> test/{corpus => corpora/default}/cur/37:2, | 0
> test/{corpus => corpora/default}/cur/38:2, | 0
> test/{corpus => corpora/default}/cur/39:2, | 0
> test/{corpus => corpora/default}/cur/40:2, | 0
> test/{corpus => corpora/default}/cur/41:2, | 0
> test/{corpus => corpora/default}/cur/42:2, | 0
> test/{corpus => corpora/default}/cur/43:2, | 0
> test/{corpus => corpora/default}/cur/44:2, | 0
> test/{corpus => corpora/default}/cur/45:2, | 0
> test/{corpus => corpora/default}/cur/46:2, | 0
> test/{corpus => corpora/default}/cur/47:2, | 0
> test/{corpus => corpora/default}/cur/48:2, | 0
> test/{corpus => corpora/default}/cur/49:2, | 0
> test/{corpus => corpora/default}/cur/50:2, | 0
> test/{corpus => corpora/default}/cur/51:2, | 0
> test/{corpus => corpora/default}/cur/52:2, | 0
> test/{corpus => corpora/default}/cur/53:2, | 0
> test/{corpus => corpora/default}/foo/06:2, | 0
> test/{corpus => corpora/default}/foo/baz/11:2, | 0
> test/{corpus => corpora/default}/foo/baz/12:2, | 0
> test/{corpus => corpora/default}/foo/baz/cur/13:2, | 0
> test/{corpus => corpora/default}/foo/baz/cur/14:2, | 0
> test/{corpus => corpora/default}/foo/baz/new/15:2, | 0
> test/{corpus => corpora/default}/foo/baz/new/16:2, | 0
> test/{corpus => corpora/default}/foo/cur/07:2, | 0
> test/{corpus => corpora/default}/foo/cur/08:2, | 0
> test/{corpus => corpora/default}/foo/new/03:2, | 0
> test/{corpus => corpora/default}/foo/new/09:2, | 0
> test/{corpus => corpora/default}/foo/new/10:2, | 0
> test/{corpus => corpora/default}/new/04:2, | 0
> test/test-lib.sh | 19 +-
> 58 files changed, 212 insertions(+), 274 deletions(-)
> create mode 100644 test/corpora/README
> create mode 100644 test/corpora/broken/broken-cc
> rename test/{corpus => corpora/default}/01:2, (100%)
> rename test/{corpus => corpora/default}/02:2, (100%)
> rename test/{corpus => corpora/default}/bar/17:2, (100%)
> rename test/{corpus => corpora/default}/bar/18:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/05:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/23:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/24:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/cur/25:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/cur/26:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/new/27:2, (100%)
> rename test/{corpus => corpora/default}/bar/baz/new/28:2, (100%)
> rename test/{corpus => corpora/default}/bar/cur/19:2, (100%)
> rename test/{corpus => corpora/default}/bar/cur/20:2, (100%)
> rename test/{corpus => corpora/default}/bar/new/21:2, (100%)
> rename test/{corpus => corpora/default}/bar/new/22:2, (100%)
> rename test/{corpus => corpora/default}/cur/29:2, (100%)
> rename test/{corpus => corpora/default}/cur/30:2, (100%)
> rename test/{corpus => corpora/default}/cur/31:2, (100%)
> rename test/{corpus => corpora/default}/cur/32:2, (100%)
> rename test/{corpus => corpora/default}/cur/33:2, (100%)
> rename test/{corpus => corpora/default}/cur/34:2, (100%)
> rename test/{corpus => corpora/default}/cur/35:2, (100%)
> rename test/{corpus => corpora/default}/cur/36:2, (100%)
> rename test/{corpus => corpora/default}/cur/37:2, (100%)
> rename test/{corpus => corpora/default}/cur/38:2, (100%)
> rename test/{corpus => corpora/default}/cur/39:2, (100%)
> rename test/{corpus => corpora/default}/cur/40:2, (100%)
> rename test/{corpus => corpora/default}/cur/41:2, (100%)
> rename test/{corpus => corpora/default}/cur/42:2, (100%)
> rename test/{corpus => corpora/default}/cur/43:2, (100%)
> rename test/{corpus => corpora/default}/cur/44:2, (100%)
> rename test/{corpus => corpora/default}/cur/45:2, (100%)
> rename test/{corpus => corpora/default}/cur/46:2, (100%)
> rename test/{corpus => corpora/default}/cur/47:2, (100%)
> rename test/{corpus => corpora/default}/cur/48:2, (100%)
> rename test/{corpus => corpora/default}/cur/49:2, (100%)
> rename test/{corpus => corpora/default}/cur/50:2, (100%)
> rename test/{corpus => corpora/default}/cur/51:2, (100%)
> rename test/{corpus => corpora/default}/cur/52:2, (100%)
> rename test/{corpus => corpora/default}/cur/53:2, (100%)
> rename test/{corpus => corpora/default}/foo/06:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/11:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/12:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/cur/13:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/cur/14:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/new/15:2, (100%)
> rename test/{corpus => corpora/default}/foo/baz/new/16:2, (100%)
> rename test/{corpus => corpora/default}/foo/cur/07:2, (100%)
> rename test/{corpus => corpora/default}/foo/cur/08:2, (100%)
> rename test/{corpus => corpora/default}/foo/new/03:2, (100%)
> rename test/{corpus => corpora/default}/foo/new/09:2, (100%)
> rename test/{corpus => corpora/default}/foo/new/10:2, (100%)
> rename test/{corpus => corpora/default}/new/04:2, (100%)
>
> --
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 00/15] reply refactor, fixes
2016-09-13 17:14 [PATCH v3 00/15] reply refactor, fixes Jani Nikula
` (15 preceding siblings ...)
2016-09-13 19:03 ` [PATCH v3 00/15] reply refactor, fixes Tomi Ollila
@ 2016-09-17 12:25 ` David Bremner
16 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-09-17 12:25 UTC (permalink / raw)
To: Jani Nikula, notmuch
Jani Nikula <jani@nikula.org> writes:
> Updated version of [1] to address David's review comments, mostly just
> commit message updates. Also incorporates the multiple corpora support
> from [2], with the commit message extended a bit, and rebasing patch 2
> on top of it.
>
series pushed.
d
^ permalink raw reply [flat|nested] 18+ messages in thread