bug in emacs reply code?

bug in emacs reply code?

[Jani Nikula]

Hi all, just upgraded from 0.11 to master on one machine, and emacs
reply started failing as follows. The four tests fail, all others pass.

$ emacs --version
GNU Emacs 23.2.1

$ make test
[...]
emacs: Testing emacs interface
[...]
 FAIL   Reply within emacs
	--- emacs.24.expected  2012-03-27 15:02:05.894335772 +0000
	+++ emacs.24.output    2012-03-27 15:02:05.884335772 +0000
	@@ -1,6 +1,9 @@
	From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+To: user@example.com
	To: user@example.com
	Subject: Re: Testing message sent via SMTP
	+Subject: Re: Testing message sent via SMTP
	In-Reply-To: <XXX>
	Fcc: /home/jani/tools/notmuch/test/tmp.emacs/mail/sent
	--text follows this line--
 FAIL   Reply within emacs to a multipart/mixed message
	--- emacs.25.expected     2012-03-27 15:02:06.294335772 +0000
	+++ emacs.25.output       2012-03-27 15:02:06.294335772 +0000
	@@ -1,6 +1,9 @@
	From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+To: Adrian Perez de Castro <aperez@igalia.com>,
	notmuch@notmuchmail.org
	To: Adrian Perez de Castro <aperez@igalia.com>,
	notmuch@notmuchmail.org
	Subject: Re: [notmuch] Introducing myself
	+Subject: Re: [notmuch] Introducing myself
	In-Reply-To: <20091118002059.067214ed@hikari>
	Fcc: /home/jani/tools/notmuch/test/tmp.emacs/mail/sent
	--text follows this line--
 FAIL   Reply within emacs to a multipart/alternative message
	--- emacs.26.expected     2012-03-27 15:02:06.914335772 +0000
	+++ emacs.26.output       2012-03-27 15:02:06.914335772 +0000
	@@ -1,6 +1,9 @@
	From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+To: Alex Botero-Lowry <alex.boterolowry@gmail.com>,
	notmuch@notmuchmail.org
	To: Alex Botero-Lowry <alex.boterolowry@gmail.com>,
	notmuch@notmuchmail.org
	Subject: Re: [notmuch] preliminary FreeBSD support
	+Subject: Re: [notmuch] preliminary FreeBSD support
	In-Reply-To:
	<cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com>
	Fcc: /home/jani/tools/notmuch/test/tmp.emacs/mail/sent
	--text follows this line--
 FAIL   Quote MML tags in reply
	--- emacs.27.expected 2012-03-27 15:02:07.524335772 +0000
	+++ emacs.27.output   2012-03-27 15:02:07.524335772 +0000
	@@ -1,6 +1,8 @@
	From: Notmuch Test Suite <test_suite@notmuchmail.org>
	+From: Notmuch Test Suite <test_suite@notmuchmail.org>
	To: 
	Subject: Re: Quote MML tags in reply
	+Subject: Re: Quote MML tags in reply
	In-Reply-To: <test-emacs-mml-quoting@message.id>
	Fcc: /home/jani/tools/notmuch/test/tmp.emacs/mail/sent
	--text follows this line--
[...]

Re: bug in emacs reply code?

[Jani Nikula]

On Tue, 27 Mar 2012 15:11:48 +0000, Jani Nikula <jani@nikula.org> wrote:
> 
> Hi all, just upgraded from 0.11 to master on one machine, and emacs
> reply started failing as follows. The four tests fail, all others pass.
> 
> $ emacs --version
> GNU Emacs 23.2.1

I bisected the problem to commit "emacs: Use the new JSON reply format
and message-cite-original" 650123510cfa64caf6b20f5239f43433fa6f2941.

Following up Adam's suggestion, it boils down to a change in emacs
message-mail function in lisp/gnus/message.el. If I eval-defun the
message-mail from 23.3, the UI seems to work. The diff between the
versions is below. That bit of elisp is beyond me, please look into it.

BR,
Jani.

--- emacs-23.2/lisp/gnus/message.el     2010-04-23 17:59:51.000000000+0300
+++ emacs-23.3/lisp/gnus/message.el     2011-11-26 05:20:20.000000000+0200
[...]
@@ -6487,7 +6513,13 @@
     (message-setup
      (nconc
       `((To . ,(or to "")) (Subject . ,(or subject "")))
-      (when other-headers other-headers))
+      ;; C-h f compose-mail says that headers should be specified as
+      ;; (string . value); however all the rest of message expects
+      ;; headers to be symbols, not strings (eg message-header-format-alist).
+      ;; http://lists.gnu.org/archive/html/emacs-devel/2011-01/msg00337.html
+      ;; We need to convert any string input, eg from rmail-start-mail.
+      (dolist (h other-headers other-headers)
+       (if (stringp (car h)) (setcar h (intern (capitalize (car h)))))))
      yank-action send-actions continue switch-function)
     ;; FIXME: Should return nil if failure.
     t))
@@ -8189,5 +8221,4 @@
[...]

[BUG/PATCH] emacs: Fix header problem in reply for emacs 23.2

[Adam Wolfe Gordon]

The new reply code used strings instead of symbols for header names,
which message-mail is OK with on emacs 23.3, but not 23.2. The symptom
is that on 23.2 (and presumably on earlier versions) the reply message
would end up with two of some headers.

This fixes the problem by converting the header names to symbols of
the type message.el usually expects before passing the headers to
message-mail.
---
 emacs/notmuch-lib.el |    5 ++++-
 emacs/notmuch-mua.el |   10 +++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c146748..af46611 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -232,9 +232,12 @@ the given type."
   (or (plist-get part :content)
       (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth process-crypto)))
 
+;; Converts a plist of headers to an alist of headers. The input plist should
+;; have symbols of the form :Header as keys, and the resulting alist will have
+;; symbols of the form 'Header as keys.
 (defun notmuch-plist-to-alist (plist)
   (loop for (key value . rest) on plist by #'cddr
-	collect (cons (substring (symbol-name key) 1) value)))
+	collect (cons (intern (substring (symbol-name key) 1)) value)))
 
 ;; Compatibility functions for versions of emacs before emacs 23.
 ;;
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 9805d79..24918d3 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -185,11 +185,11 @@ OTHER-ARGS are passed through to `message-mail'."
   (when notmuch-mua-user-agent-function
     (let ((user-agent (funcall notmuch-mua-user-agent-function)))
       (when (not (string= "" user-agent))
-	(push (cons "User-Agent" user-agent) other-headers))))
+	(push (cons 'User-Agent user-agent) other-headers))))
 
-  (unless (assoc "From" other-headers)
-    (push (cons "From" (concat
-			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
+  (unless (assq 'From other-headers)
+    (push (cons 'From (concat
+		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
   (apply #'message-mail to subject other-headers other-args)
   (message-sort-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)
-- 
1.7.5.4

Re: [BUG/PATCH] emacs: Fix header problem in reply for emacs 23.2

[Austin Clements]

Quoth Adam Wolfe Gordon on Mar 28 at  8:02 am:
> The new reply code used strings instead of symbols for header names,
> which message-mail is OK with on emacs 23.3, but not 23.2. The symptom
> is that on 23.2 (and presumably on earlier versions) the reply message
> would end up with two of some headers.
> 
> This fixes the problem by converting the header names to symbols of
> the type message.el usually expects before passing the headers to
> message-mail.
> ---
>  emacs/notmuch-lib.el |    5 ++++-
>  emacs/notmuch-mua.el |   10 +++++-----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index c146748..af46611 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -232,9 +232,12 @@ the given type."
>    (or (plist-get part :content)
>        (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth process-crypto)))
>  
> +;; Converts a plist of headers to an alist of headers. The input plist should
> +;; have symbols of the form :Header as keys, and the resulting alist will have
> +;; symbols of the form 'Header as keys.
>  (defun notmuch-plist-to-alist (plist)

This name isn't really appropriate now.
notmuch-plist-to-headers-list?  notmuch-headers-plist-to-alist?

Otherwise LGTM, though I don't run such ancient versions of Emacs, so
I can't verify that it fixes the problem.

>    (loop for (key value . rest) on plist by #'cddr
> -	collect (cons (substring (symbol-name key) 1) value)))
> +	collect (cons (intern (substring (symbol-name key) 1)) value)))
>  
>  ;; Compatibility functions for versions of emacs before emacs 23.
>  ;;
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 9805d79..24918d3 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -185,11 +185,11 @@ OTHER-ARGS are passed through to `message-mail'."
>    (when notmuch-mua-user-agent-function
>      (let ((user-agent (funcall notmuch-mua-user-agent-function)))
>        (when (not (string= "" user-agent))
> -	(push (cons "User-Agent" user-agent) other-headers))))
> +	(push (cons 'User-Agent user-agent) other-headers))))
>  
> -  (unless (assoc "From" other-headers)
> -    (push (cons "From" (concat
> -			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
> +  (unless (assq 'From other-headers)
> +    (push (cons 'From (concat
> +		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
>    (apply #'message-mail to subject other-headers other-args)
>    (message-sort-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)

Re: [BUG/PATCH] emacs: Fix header problem in reply for emacs 23.2

[Jani Nikula]

On Wed, 28 Mar 2012 08:02:18 -0600, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> The new reply code used strings instead of symbols for header names,
> which message-mail is OK with on emacs 23.3, but not 23.2. The symptom
> is that on 23.2 (and presumably on earlier versions) the reply message
> would end up with two of some headers.
> 
> This fixes the problem by converting the header names to symbols of
> the type message.el usually expects before passing the headers to
> message-mail.

The patch depends on some set of the other reply fixes (*), so I applied
this on top of them. All tests pass with emacs 23.2.

Unfortunately, the emacs UI fails with "Symbol's function definition is
void: remove-if" when trying to reply. After a manual (require 'cl) the
UI seems to otherwise work as expected. This is probably unrelated to
this particular patch, but why is it a (eval-when-compile (require 'cl))
and not (require 'cl) in notmuch-mua.el?


BR,
Jani.


(*)
id:"1332941635-21019-2-git-send-email-awg+notmuch@xvx.ca"
id:"1332941635-21019-3-git-send-email-awg+notmuch@xvx.ca"
id:"1332996818-15700-1-git-send-email-awg+notmuch@xvx.ca"

> ---
>  emacs/notmuch-lib.el |    5 ++++-
>  emacs/notmuch-mua.el |   10 +++++-----
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index c146748..af46611 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -232,9 +232,12 @@ the given type."
>    (or (plist-get part :content)
>        (notmuch-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth process-crypto)))
>  
> +;; Converts a plist of headers to an alist of headers. The input plist should
> +;; have symbols of the form :Header as keys, and the resulting alist will have
> +;; symbols of the form 'Header as keys.
>  (defun notmuch-plist-to-alist (plist)
>    (loop for (key value . rest) on plist by #'cddr
> -	collect (cons (substring (symbol-name key) 1) value)))
> +	collect (cons (intern (substring (symbol-name key) 1)) value)))
>  
>  ;; Compatibility functions for versions of emacs before emacs 23.
>  ;;
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 9805d79..24918d3 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -185,11 +185,11 @@ OTHER-ARGS are passed through to `message-mail'."
>    (when notmuch-mua-user-agent-function
>      (let ((user-agent (funcall notmuch-mua-user-agent-function)))
>        (when (not (string= "" user-agent))
> -	(push (cons "User-Agent" user-agent) other-headers))))
> +	(push (cons 'User-Agent user-agent) other-headers))))
>  
> -  (unless (assoc "From" other-headers)
> -    (push (cons "From" (concat
> -			(notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
> +  (unless (assq 'From other-headers)
> +    (push (cons 'From (concat
> +		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
>  
>    (apply #'message-mail to subject other-headers other-args)
>    (message-sort-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)
> -- 
> 1.7.5.4
>