notmuch-message-mode initialization follow-up

notmuch-message-mode initialization follow-up

[Michal Sojka]

Hi,

the first patch fixes the problem reported in
id:87wprt2r4f.fsf@zancas.localnet. Patch 2/3 is just refactoring.
Patch 3/3 is reaction to id:87y4cc3qse.fsf@zancas.localnet; I'm not
sure whether it addresses exactly what David had in mind, but IMHO it
is an improvement.

Cheers.
-Michal

[PATCH 1/3] emacs: Fix mail composition under Emacs 23

[Michal Sojka]

Commit 570c0aeb40bd0c3af8174624a55e968f62c44f09 reworked
notmuch-mua-mail function in a way that worked only under Emacs 24.
The reason was that message-setup-1 took one argument less in Emacs
23.

We fix this by only supplying the return-action argument when it is
actually set by the caller.
---
 emacs/notmuch-mua.el | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index a66a306..d4950cb 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -320,18 +320,24 @@ modified. This function is notmuch addaptation of
 		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
   (notmuch-mua-pop-to-buffer (message-buffer-name "mail" to))
-  (message-setup-1
-   ;; The following sexp is copied from `message-mail'
-   (nconc
-    `((To . ,(or to "")) (Subject . ,(or subject "")))
-    ;; 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 return-action)
+  (let ((args (list yank-action send-actions)))
+    ;; message-setup-1 in Emacs 23 does not accept return-action
+    ;; argument. Pass it only if it is supplied by the caller. This
+    ;; will never be the case when we're called by `compose-mail' in
+    ;; Emacs 23.
+    (when return-action (nconc args '(return-action)))
+    (apply 'message-setup-1
+	   ;; The following sexp is copied from `message-mail'
+	   (nconc
+	    `((To . ,(or to "")) (Subject . ,(or subject "")))
+	    ;; 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)))))))
+	   args))
   (notmuch-fcc-header-setup)
   (message-sort-headers)
   (message-hide-headers)
-- 
2.6.4

Re: [PATCH 1/3] emacs: Fix mail composition under Emacs 23

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> +    (when return-action (nconc args '(return-action)))
> +    (apply 'message-setup-1
> +	   ;; The following sexp is copied from `message-mail'

> +	   (nconc
> +	    `((To . ,(or to "")) (Subject . ,(or subject "")))

I missed this the first time, but I a bit worried about this used of
nconc. It seems to fall under "A common pitfall is to use a quoted
constant list as a non-last argument to ‘nconc’"  (from the elisp
manual).  In any case it's not really performance critical code (I
guess?) so maybe we could just use append?

The other use of nconc is more understandable to me.

Re: [PATCH 1/3] emacs: Fix mail composition under Emacs 23

[Michal Sojka]

On Tue, Jan 05 2016, David Bremner wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> +    (when return-action (nconc args '(return-action)))
>> +    (apply 'message-setup-1
>> +	   ;; The following sexp is copied from `message-mail'
>
>> +	   (nconc
>> +	    `((To . ,(or to "")) (Subject . ,(or subject "")))
>
> I missed this the first time, but I a bit worried about this used of
> nconc. It seems to fall under "A common pitfall is to use a quoted
> constant list as a non-last argument to ‘nconc’"  (from the elisp
> manual).  In any case it's not really performance critical code (I
> guess?) so maybe we could just use append?

Yes, this make sense. As the nconc is already in master, I'll not
combine the fix for this with 1/3, but I'm sending a separate patch that
applies after 3/3.

-Michal

[PATCH] emacs: Don't use nconc on quoted list

[Michal Sojka]

As pointed out by David Bremner, Elisp manual says "A common pitfall
is to use a quoted constant list as a non-last argument to ‘nconc’."
Since this was the case in recently added code, we fix it here.
---
 emacs/notmuch-mua.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 45a6daa..5462f54 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -322,8 +322,8 @@ modified. This function is notmuch addaptation of
   (notmuch-mua-pop-to-buffer (message-buffer-name "mail" to)
 			     (or switch-function (notmuch-mua-get-switch-function)))
   (let ((headers
-	 ;; The following sexp is copied from `message-mail'
-	 (nconc
+	 (append
+	  ;; The following is copied from `message-mail'
 	  `((To . ,(or to "")) (Subject . ,(or subject "")))
 	  ;; C-h f compose-mail says that headers should be specified as
 	  ;; (string . value); however all the rest of message expects
-- 
2.6.4

[PATCH 2/3] emacs: Refactor notmuch-mua-mail

[Michal Sojka]

This should be more readable.
---
 emacs/notmuch-mua.el | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index d4950cb..2d6825d 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -320,24 +320,24 @@ modified. This function is notmuch addaptation of
 		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
   (notmuch-mua-pop-to-buffer (message-buffer-name "mail" to))
-  (let ((args (list yank-action send-actions)))
+  (let ((headers
+	 ;; The following sexp is copied from `message-mail'
+	 (nconc
+	  `((To . ,(or to "")) (Subject . ,(or subject "")))
+	  ;; 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))))))))
+	(args (list yank-action send-actions)))
     ;; message-setup-1 in Emacs 23 does not accept return-action
     ;; argument. Pass it only if it is supplied by the caller. This
     ;; will never be the case when we're called by `compose-mail' in
     ;; Emacs 23.
     (when return-action (nconc args '(return-action)))
-    (apply 'message-setup-1
-	   ;; The following sexp is copied from `message-mail'
-	   (nconc
-	    `((To . ,(or to "")) (Subject . ,(or subject "")))
-	    ;; 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)))))))
-	   args))
+    (apply 'message-setup-1 headers args))
   (notmuch-fcc-header-setup)
   (message-sort-headers)
   (message-hide-headers)
-- 
2.6.4

[PATCH 3/3] emacs: Handle switch-function argument of notmuch-mua-mail

[Michal Sojka]

notmuch-mua-mail ignored the switch-function argument and always used
the function returned by notmuch-mua-get-switch-function instead. In
order to support standard emacs interfaces (compose-mail in this
case), this commit changes notmuch-mua-mail to use the switch-function
argument if it is non-nil and notmuch-mua-get-switch-function
otherwise.
---
 emacs/notmuch-mua.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 2d6825d..45a6daa 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -278,7 +278,7 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
 (define-key notmuch-message-mode-map (kbd "C-c C-c") #'notmuch-mua-send-and-exit)
 (define-key notmuch-message-mode-map (kbd "C-c C-s") #'notmuch-mua-send)
 
-(defun notmuch-mua-pop-to-buffer (name)
+(defun notmuch-mua-pop-to-buffer (name switch-function)
   "Pop to buffer NAME, and warn if it already exists and is
 modified. This function is notmuch addaptation of
 `message-pop-to-buffer'."
@@ -291,7 +291,7 @@ modified. This function is notmuch addaptation of
 	      (progn
 		(gnus-select-frame-set-input-focus (window-frame window))
 		(select-window window))
-	    (funcall (notmuch-mua-get-switch-function) buffer)
+	    (funcall switch-function buffer)
 	    (set-buffer buffer))
 	  (when (and (buffer-modified-p)
 		     (not (prog1
@@ -299,7 +299,7 @@ modified. This function is notmuch addaptation of
 			       "Message already being composed; erase? ")
 			    (message nil))))
 	    (error "Message being composed")))
-      (funcall (notmuch-mua-get-switch-function) name)
+      (funcall switch-function name)
       (set-buffer name))
     (erase-buffer)
     (notmuch-message-mode)))
@@ -319,7 +319,8 @@ modified. This function is notmuch addaptation of
     (push (cons 'From (concat
 		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
-  (notmuch-mua-pop-to-buffer (message-buffer-name "mail" to))
+  (notmuch-mua-pop-to-buffer (message-buffer-name "mail" to)
+			     (or switch-function (notmuch-mua-get-switch-function)))
   (let ((headers
 	 ;; The following sexp is copied from `message-mail'
 	 (nconc
-- 
2.6.4

Re: [PATCH 3/3] emacs: Handle switch-function argument of notmuch-mua-mail

[David Bremner]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> notmuch-mua-mail ignored the switch-function argument and always used
> the function returned by notmuch-mua-get-switch-function instead. In
> order to support standard emacs interfaces (compose-mail in this
> case), this commit changes notmuch-mua-mail to use the switch-function
> argument if it is non-nil and notmuch-mua-get-switch-function
> otherwise.

pushed 1-3, plus the fixup

d

Re: notmuch-message-mode initialization follow-up

[Tomi Ollila]

On Sat, Jan 02 2016, Michal Sojka <sojkam1@fel.cvut.cz> wrote:

> Hi,
>
> the first patch fixes the problem reported in
> id:87wprt2r4f.fsf@zancas.localnet. Patch 2/3 is just refactoring.
> Patch 3/3 is reaction to id:87y4cc3qse.fsf@zancas.localnet; I'm not
> sure whether it addresses exactly what David had in mind, but IMHO it
> is an improvement.

Applied the patches and tested with (*)

EMACS=/usr/bin/emacs ./devel/try-emacs-mua -q

on Scientific linux 6.2 where /usr/bin/emacs is version 23.1

and it worked fine. 1/3 and 2/3 are ok, I don't know why 3/3 is there
but it looks ok to me.

>
> Cheers.
> -Michal

Tomi

(*) devel/try-emacs-mua is posted in 
id:1450610032-23776-1-git-send-email-tomi.ollila@iki.fi