unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [BUG/PATCH 0/2] Replying from other addresses in emacs
@ 2012-03-25  0:27 Adam Wolfe Gordon
  2012-03-25  0:27 ` [BUG/PATCH 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-25  0:27 UTC (permalink / raw)
  To: notmuch

Hi everyone,

This patch series adds a test for and fixes a bug that Dmitry pointed
out on IRC yesterday:

When replying to mail sent to an other_address, the CLI correctly sets
the From header to the address that received the message. This works
correctly in the default format and the JSON format.  However, emacs was
ignoring the provided From header and inserting the primary address, due
to an incompatibility between how we construct the header list and how
the `mail-header` function from the message library looks up header
values in the list.

People with other_addresses who are using the latest reply changes
should probably apply this patch right away. Without it, you might end
up replying to messages from the wrong address.

Adam Wolfe Gordon (2):
  test: Tests for reply from alternate addresses in emacs
  emacs: Fix replying from alternate addresses

 emacs/notmuch-mua.el |    4 +-
 test/emacs           |   51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs
  2012-03-25  0:27 [BUG/PATCH 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
@ 2012-03-25  0:27 ` Adam Wolfe Gordon
  2012-03-26 17:01   ` Dmitry Kurochkin
  2012-03-25  0:27 ` [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
  2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2 siblings, 1 reply; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-25  0:27 UTC (permalink / raw)
  To: notmuch

Since the recent reply changes were pushed, there has been a bug that
causes emacs to always reply from the primary address, even if the
JSON or default CLI reply output uses an alternate address.

This adds two tests to the emacs test library based on the two "Reply
form..." tests in the reply test library. One is currently marked
broken.
---
 test/emacs |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8a28705..fa5d706 100755
--- a/test/emacs
+++ b/test/emacs
@@ -274,6 +274,58 @@ Notmuch Test Suite <test_suite@notmuchmail.org> writes:
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Reply from alternate address within emacs"
+test_subtest_known_broken
+add_message '[from]="Sender <sender@example.com>"' \
+	     [to]=test_suite_other@notmuchmail.org \
+	     [subject]=notmuch-reply-test \
+	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+	    '[body]="reply from alternate address"'
+
+test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
+	    (notmuch-test-wait)
+	    (notmuch-search-reply-to-thread)
+	    (test-output)"
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite_other@notmuchmail.org>
+To: Sender <sender@example.com>
+Subject: Re: notmuch-reply-test
+In-Reply-To: <XXX>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Sender <sender@example.com> writes:
+
+> reply from alternate address
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Reply from address in named group list within emacs"
+add_message '[from]="Sender <sender@example.com>"' \
+            '[to]=group:test_suite@notmuchmail.org,someone@example.com\;' \
+             [cc]=test_suite_other@notmuchmail.org \
+             [subject]=notmuch-reply-test \
+            '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+            '[body]="Reply from address in named group list"'
+
+test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
+	    (notmuch-test-wait)
+	    (notmuch-search-reply-to-thread)
+	    (test-output)"
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Sender <sender@example.com>, someone@example.com
+Subject: Re: notmuch-reply-test
+In-Reply-To: <XXX>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Sender <sender@example.com> writes:
+
+> Reply from address in named group list
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Reply within emacs to a multipart/mixed message"
 test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
 		(notmuch-show-reply)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
  2012-03-25  0:27 [BUG/PATCH 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2012-03-25  0:27 ` [BUG/PATCH 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
@ 2012-03-25  0:27 ` Adam Wolfe Gordon
  2012-03-25 12:22   ` Mark Walters
  2012-03-26 17:02   ` Dmitry Kurochkin
  2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2 siblings, 2 replies; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-25  0:27 UTC (permalink / raw)
  To: notmuch

The bug was that notmuch-mua-mail used `mail-header` to check whether
it was passed a "From" header. The implementation of `mail-header`
must try to compare symbols instead of strings when looking for
headers, as it was returning nil when a From header was present. This
is probably because the mail functions construct headers as alists
with symbols for the header names, while our code uses strings for the
header names.

Since we don't use `mail-header` anywhere else, and `message-mail` is
perfectly happy to accept string header names, the fix is just to use
`assoc` to look for the From header, so that the strings get compared
properly.
---
 emacs/notmuch-mua.el |    4 ++--
 test/emacs           |    1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 6aae3a0..9805d79 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'."
       (when (not (string= "" user-agent))
 	(push (cons "User-Agent" user-agent) other-headers))))
 
-  (unless (mail-header 'From other-headers)
+  (unless (assoc "From" other-headers)
     (push (cons "From" (concat
 			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
@@ -250,7 +250,7 @@ the From: address first."
   (interactive "P")
   (let ((other-headers
 	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
-	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
+	   (list (cons "From" (notmuch-mua-prompt-for-sender))))))
     (notmuch-mua-mail nil nil other-headers)))
 
 (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
diff --git a/test/emacs b/test/emacs
index fa5d706..08db1ee 100755
--- a/test/emacs
+++ b/test/emacs
@@ -275,7 +275,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Reply from alternate address within emacs"
-test_subtest_known_broken
 add_message '[from]="Sender <sender@example.com>"' \
 	     [to]=test_suite_other@notmuchmail.org \
 	     [subject]=notmuch-reply-test \
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
  2012-03-25  0:27 ` [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
@ 2012-03-25 12:22   ` Mark Walters
  2012-03-26 17:02   ` Dmitry Kurochkin
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Walters @ 2012-03-25 12:22 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

Adam Wolfe Gordon <awg+notmuch@xvx.ca> writes:
> The bug was that notmuch-mua-mail used `mail-header` to check whether
> it was passed a "From" header. The implementation of `mail-header`
> must try to compare symbols instead of strings when looking for
> headers, as it was returning nil when a From header was present. This
> is probably because the mail functions construct headers as alists
> with symbols for the header names, while our code uses strings for the
> header names.
>
> Since we don't use `mail-header` anywhere else, and `message-mail` is
> perfectly happy to accept string header names, the fix is just to use
> `assoc` to look for the From header, so that the strings get compared
> properly.

I can confirm that everything works as expected with this fix. 

Best wishes

Mark
> ---
>  emacs/notmuch-mua.el |    4 ++--
>  test/emacs           |    1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 6aae3a0..9805d79 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'."
>        (when (not (string= "" user-agent))
>  	(push (cons "User-Agent" user-agent) other-headers))))
>  
> -  (unless (mail-header 'From other-headers)
> +  (unless (assoc "From" other-headers)
>      (push (cons "From" (concat
>  			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
> @@ -250,7 +250,7 @@ the From: address first."
>    (interactive "P")
>    (let ((other-headers
>  	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
> -	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
> +	   (list (cons "From" (notmuch-mua-prompt-for-sender))))))
>      (notmuch-mua-mail nil nil other-headers)))
>  
>  (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
> diff --git a/test/emacs b/test/emacs
> index fa5d706..08db1ee 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -275,7 +275,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply from alternate address within emacs"
> -test_subtest_known_broken
>  add_message '[from]="Sender <sender@example.com>"' \
>  	     [to]=test_suite_other@notmuchmail.org \
>  	     [subject]=notmuch-reply-test \
> -- 
> 1.7.5.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [BUG/PATCH 1/2] test: Tests for reply from alternate addresses in emacs
  2012-03-25  0:27 ` [BUG/PATCH 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
@ 2012-03-26 17:01   ` Dmitry Kurochkin
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-03-26 17:01 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

Adam Wolfe Gordon <awg+notmuch@xvx.ca> writes:
> Since the recent reply changes were pushed, there has been a bug that
> causes emacs to always reply from the primary address, even if the
> JSON or default CLI reply output uses an alternate address.
>
> This adds two tests to the emacs test library based on the two "Reply
> form..." tests in the reply test library. One is currently marked
> broken.
> ---
>  test/emacs |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 8a28705..fa5d706 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -274,6 +274,58 @@ Notmuch Test Suite <test_suite@notmuchmail.org> writes:
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "Reply from alternate address within emacs"
> +test_subtest_known_broken
> +add_message '[from]="Sender <sender@example.com>"' \
> +	     [to]=test_suite_other@notmuchmail.org \
> +	     [subject]=notmuch-reply-test \
> +	    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
> +	    '[body]="reply from alternate address"'

Please remove subject, data and body parameters.  add_message would
provide sane default values.

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> +	    (notmuch-test-wait)
> +	    (notmuch-search-reply-to-thread)
> +	    (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
> +cat <<EOF >EXPECTED
> +From: Notmuch Test Suite <test_suite_other@notmuchmail.org>
> +To: Sender <sender@example.com>
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: <XXX>
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender <sender@example.com> writes:
> +
> +> reply from alternate address
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
> +test_begin_subtest "Reply from address in named group list within emacs"
> +add_message '[from]="Sender <sender@example.com>"' \
> +            '[to]=group:test_suite@notmuchmail.org,someone@example.com\;' \
> +             [cc]=test_suite_other@notmuchmail.org \
> +             [subject]=notmuch-reply-test \
> +            '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
> +            '[body]="Reply from address in named group list"'

Same here.

Regards,
  Dmitry

> +
> +test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
> +	    (notmuch-test-wait)
> +	    (notmuch-search-reply-to-thread)
> +	    (test-output)"
> +sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
> +cat <<EOF >EXPECTED
> +From: Notmuch Test Suite <test_suite@notmuchmail.org>
> +To: Sender <sender@example.com>, someone@example.com
> +Subject: Re: notmuch-reply-test
> +In-Reply-To: <XXX>
> +Fcc: ${MAIL_DIR}/sent
> +--text follows this line--
> +Sender <sender@example.com> writes:
> +
> +> Reply from address in named group list
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "Reply within emacs to a multipart/mixed message"
>  test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
>  		(notmuch-show-reply)
> -- 
> 1.7.5.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses
  2012-03-25  0:27 ` [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
  2012-03-25 12:22   ` Mark Walters
@ 2012-03-26 17:02   ` Dmitry Kurochkin
  1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-03-26 17:02 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

Adam Wolfe Gordon <awg+notmuch@xvx.ca> writes:
> The bug was that notmuch-mua-mail used `mail-header` to check whether
> it was passed a "From" header. The implementation of `mail-header`
> must try to compare symbols instead of strings when looking for
> headers, as it was returning nil when a From header was present. This
> is probably because the mail functions construct headers as alists
> with symbols for the header names, while our code uses strings for the
> header names.
>
> Since we don't use `mail-header` anywhere else, and `message-mail` is
> perfectly happy to accept string header names, the fix is just to use
> `assoc` to look for the From header, so that the strings get compared
> properly.
> ---

LGTM.  Thanks for fixing this.

Regards,
  Dmitry

>  emacs/notmuch-mua.el |    4 ++--
>  test/emacs           |    1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 6aae3a0..9805d79 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'."
>        (when (not (string= "" user-agent))
>  	(push (cons "User-Agent" user-agent) other-headers))))
>  
> -  (unless (mail-header 'From other-headers)
> +  (unless (assoc "From" other-headers)
>      (push (cons "From" (concat
>  			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
> @@ -250,7 +250,7 @@ the From: address first."
>    (interactive "P")
>    (let ((other-headers
>  	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
> -	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
> +	   (list (cons "From" (notmuch-mua-prompt-for-sender))))))
>      (notmuch-mua-mail nil nil other-headers)))
>  
>  (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
> diff --git a/test/emacs b/test/emacs
> index fa5d706..08db1ee 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -275,7 +275,6 @@ EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
>  test_begin_subtest "Reply from alternate address within emacs"
> -test_subtest_known_broken
>  add_message '[from]="Sender <sender@example.com>"' \
>  	     [to]=test_suite_other@notmuchmail.org \
>  	     [subject]=notmuch-reply-test \
> -- 
> 1.7.5.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [BUG/PATCH v2 0/2] Replying from other addresses in emacs
  2012-03-25  0:27 [BUG/PATCH 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2012-03-25  0:27 ` [BUG/PATCH 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
  2012-03-25  0:27 ` [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
@ 2012-03-28 13:33 ` Adam Wolfe Gordon
  2012-03-28 13:33   ` [BUG/PATCH v2 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
                     ` (2 more replies)
  2 siblings, 3 replies; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-28 13:33 UTC (permalink / raw)
  To: notmuch

This is a new version of my previous series [1], fixing the bug Dmitry noticed
when replying from alternate addresses. This version fixes a couple of review
issues in the test patch [2], but is otherwise identical.

[1] id:"1332635232-15269-1-git-send-email-awg+notmuch@xvx.ca"
[2] id:"87bonjjny9.fsf@gmail.com"

Adam Wolfe Gordon (2):
  test: Tests for reply from alternate addresses in emacs
  emacs: Fix replying from alternate addresses

 emacs/notmuch-mua.el |    4 ++--
 test/emacs           |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

-- 
1.7.5.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [BUG/PATCH v2 1/2] test: Tests for reply from alternate addresses in emacs
  2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
@ 2012-03-28 13:33   ` Adam Wolfe Gordon
  2012-03-28 13:33   ` [BUG/PATCH v2 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
  2012-03-28 22:10   ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Jameson Graef Rollins
  2 siblings, 0 replies; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-28 13:33 UTC (permalink / raw)
  To: notmuch

Since the recent reply changes were pushed, there has been a bug that
causes emacs to always reply from the primary address, even if the
JSON or default CLI reply output uses an alternate address.

This adds two tests to the emacs test library based on the two "Reply
form..." tests in the reply test library. One is currently marked
broken.
---
 test/emacs |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8a28705..3402efb 100755
--- a/test/emacs
+++ b/test/emacs
@@ -274,6 +274,50 @@ Notmuch Test Suite <test_suite@notmuchmail.org> writes:
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Reply from alternate address within emacs"
+test_subtest_known_broken
+add_message '[from]="Sender <sender@example.com>"' \
+	     [to]=test_suite_other@notmuchmail.org
+
+test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
+	    (notmuch-test-wait)
+	    (notmuch-search-reply-to-thread)
+	    (test-output)"
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite_other@notmuchmail.org>
+To: Sender <sender@example.com>
+Subject: Re: ${test_subtest_name}
+In-Reply-To: <${gen_msg_id}>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Sender <sender@example.com> writes:
+
+> This is just a test message (#${gen_msg_cnt})
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Reply from address in named group list within emacs"
+add_message '[from]="Sender <sender@example.com>"' \
+            '[to]=group:test_suite@notmuchmail.org,someone@example.com\;' \
+             [cc]=test_suite_other@notmuchmail.org
+
+test_emacs "(notmuch-search \"id:\\\"${gen_msg_id}\\\"\")
+	    (notmuch-test-wait)
+	    (notmuch-search-reply-to-thread)
+	    (test-output)"
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: Sender <sender@example.com>, someone@example.com
+Subject: Re: ${test_subtest_name}
+In-Reply-To: <${gen_msg_id}>
+Fcc: ${MAIL_DIR}/sent
+--text follows this line--
+Sender <sender@example.com> writes:
+
+> This is just a test message (#${gen_msg_cnt})
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Reply within emacs to a multipart/mixed message"
 test_emacs '(notmuch-show "id:20091118002059.067214ed@hikari")
 		(notmuch-show-reply)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BUG/PATCH v2 2/2] emacs: Fix replying from alternate addresses
  2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2012-03-28 13:33   ` [BUG/PATCH v2 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
@ 2012-03-28 13:33   ` Adam Wolfe Gordon
  2012-03-28 22:10   ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Jameson Graef Rollins
  2 siblings, 0 replies; 10+ messages in thread
From: Adam Wolfe Gordon @ 2012-03-28 13:33 UTC (permalink / raw)
  To: notmuch

The bug was that notmuch-mua-mail used `mail-header` to check whether
it was passed a "From" header. The implementation of `mail-header`
must try to compare symbols instead of strings when looking for
headers, as it was returning nil when a From header was present. This
is probably because the mail functions construct headers as alists
with symbols for the header names, while our code uses strings for the
header names.

Since we don't use `mail-header` anywhere else, and `message-mail` is
perfectly happy to accept string header names, the fix is just to use
`assoc` to look for the From header, so that the strings get compared
properly.
---
 emacs/notmuch-mua.el |    4 ++--
 test/emacs           |    1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 6aae3a0..9805d79 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -187,7 +187,7 @@ OTHER-ARGS are passed through to `message-mail'."
       (when (not (string= "" user-agent))
 	(push (cons "User-Agent" user-agent) other-headers))))
 
-  (unless (mail-header 'From other-headers)
+  (unless (assoc "From" other-headers)
     (push (cons "From" (concat
 			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
@@ -250,7 +250,7 @@ the From: address first."
   (interactive "P")
   (let ((other-headers
 	 (when (or prompt-for-sender notmuch-always-prompt-for-sender)
-	   (list (cons 'From (notmuch-mua-prompt-for-sender))))))
+	   (list (cons "From" (notmuch-mua-prompt-for-sender))))))
     (notmuch-mua-mail nil nil other-headers)))
 
 (defun notmuch-mua-new-forward-message (&optional prompt-for-sender)
diff --git a/test/emacs b/test/emacs
index 3402efb..06291d3 100755
--- a/test/emacs
+++ b/test/emacs
@@ -275,7 +275,6 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Reply from alternate address within emacs"
-test_subtest_known_broken
 add_message '[from]="Sender <sender@example.com>"' \
 	     [to]=test_suite_other@notmuchmail.org
 
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [BUG/PATCH v2 0/2] Replying from other addresses in emacs
  2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
  2012-03-28 13:33   ` [BUG/PATCH v2 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
  2012-03-28 13:33   ` [BUG/PATCH v2 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
@ 2012-03-28 22:10   ` Jameson Graef Rollins
  2 siblings, 0 replies; 10+ messages in thread
From: Jameson Graef Rollins @ 2012-03-28 22:10 UTC (permalink / raw)
  To: Adam Wolfe Gordon, notmuch

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On Wed, Mar 28 2012, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> This is a new version of my previous series [1], fixing the bug Dmitry noticed
> when replying from alternate addresses. This version fixes a couple of review
> issues in the test patch [2], but is otherwise identical.

I have tested this patch against current master and can confirm that it
fixes the bug for me.  The patches also look nice clean, and include a
good test, so this definitely LGTM.  Since it fixes a bug I recommend
pushing as soon as possible.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-03-28 22:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-25  0:27 [BUG/PATCH 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
2012-03-25  0:27 ` [BUG/PATCH 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
2012-03-26 17:01   ` Dmitry Kurochkin
2012-03-25  0:27 ` [BUG/PATCH 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
2012-03-25 12:22   ` Mark Walters
2012-03-26 17:02   ` Dmitry Kurochkin
2012-03-28 13:33 ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Adam Wolfe Gordon
2012-03-28 13:33   ` [BUG/PATCH v2 1/2] test: Tests for reply from alternate " Adam Wolfe Gordon
2012-03-28 13:33   ` [BUG/PATCH v2 2/2] emacs: Fix replying from alternate addresses Adam Wolfe Gordon
2012-03-28 22:10   ` [BUG/PATCH v2 0/2] Replying from other addresses in emacs Jameson Graef Rollins

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).