[PATCH] emacs: remove use of message-send-hook

[PATCH] emacs: remove use of message-send-hook

[Mark Walters]

We use message-send-hook for two things -- tagging drafts deleted, and
tagging replied the parent message of a reply. We should not use
message-send-hook as that will affect gnus etc too. Moreover,
message-send-hook is run before the message is sent, even before the
user confirms they want to send it (if message-confirm-send is
set). This means the user can do C-c C-c to send, then cancel, and the
parent is still tagged replied.

Now we have our own send function it is natural to put these functions
there rather than in the hook. We also fix the bug mentioned
above. This is slightly tricky as message-send-and-exit kills the
compose buffer, so we need to store the tagging commands ready to be
run after the send is successful.
---

As mentioned in the commit message, we need to remove our use of
message-send-hook and fix the bug of erroneously tagging messages
"replied" or "deleted". This approach might be a little overkill (a
dolist for a list of two elements), but it avoids breaking the api
(for notmuch-message-mark-replied) and is easily extendable if we need
to add more post send tagging operations later.

Also, it seemed easiest to tweak the require orders, but maybe we
should just move the two things in notmuch-message into notmuch-mua.

Best wishes

Mark


emacs/notmuch-draft.el   | 12 ++++++------
 emacs/notmuch-message.el | 12 +++++++-----
 emacs/notmuch-mua.el     | 19 +++++++++++++++----
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-draft.el b/emacs/notmuch-draft.el
index fb7f4f5..738628b 100644
--- a/emacs/notmuch-draft.el
+++ b/emacs/notmuch-draft.el
@@ -91,12 +91,15 @@ postponing and resuming a message."
   "Message-id of the most recent saved draft of this message")
 (make-variable-buffer-local 'notmuch-draft-id)
 
-(defun notmuch-draft--mark-deleted ()
+(defun notmuch-draft--mark-deleted (&optional dry-run)
   "Tag the last saved draft deleted.
 
-Used when a new version is saved, or the message is sent."
+Used when a new version is saved, or the message is sent.  If
+DRY-RUN is set, then just return a list of the arguments for
+`notmuch-tag' rather than doing the tagging."
   (when notmuch-draft-id
-    (notmuch-tag notmuch-draft-id '("+deleted"))))
+    (funcall (if dry-run #'list #'notmuch-tag)
+	     notmuch-draft-id '("+deleted"))))
 
 (defun notmuch-draft-quote-some-mml ()
   "Quote the mml tags in `notmuch-draft-quoted-tags`."
@@ -259,9 +262,6 @@ applied to newly inserted messages)."
       (setq notmuch-draft-id (when draft id)))))
 
 
-(add-hook 'message-send-hook 'notmuch-draft--mark-deleted)
-
-
 (provide 'notmuch-draft)
 
 ;;; notmuch-draft.el ends here
diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index 55e4cfe..373d38c 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -23,7 +23,6 @@
 
 (require 'message)
 (require 'notmuch-tag)
-(require 'notmuch-mua)
 
 (defcustom notmuch-message-replied-tags '("+replied")
   "List of tag changes to apply to a message when it has been replied to.
@@ -38,15 +37,18 @@ the \"inbox\" and \"todo\" tags, you would set:
   :type '(repeat string)
   :group 'notmuch-send)
 
-(defun notmuch-message-mark-replied ()
+(defun notmuch-message-mark-replied (&optional dry-run)
+  "Tag the parent message replied.
+
+If DRY-RUN is set, then just return a list of the arguments for
+`notmuch-tag' rather than doing the tagging."
   ;; get the in-reply-to header and parse it for the message id.
   (let ((rep (mail-header-parse-addresses (message-field-value "In-Reply-To"))))
     (when (and notmuch-message-replied-tags rep)
-      (notmuch-tag (notmuch-id-to-query (car (car rep)))
+      (funcall (if dry-run #'list #'notmuch-tag)
+	       (notmuch-id-to-query (car (car rep)))
 	       (notmuch-tag-change-list notmuch-message-replied-tags)))))
 
-(add-hook 'message-send-hook 'notmuch-message-mark-replied)
-
 (provide 'notmuch-message)
 
 ;;; notmuch-message.el ends here
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 93747b1..07be69a 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -28,6 +28,7 @@
 (require 'notmuch-lib)
 (require 'notmuch-address)
 (require 'notmuch-draft)
+(require 'notmuch-message)
 
 (eval-when-compile (require 'cl))
 
@@ -545,10 +546,20 @@ unencrypted.  Really send? "))))
   (interactive "P")
   (when (and (notmuch-mua-check-no-misplaced-secure-tag)
 	     (notmuch-mua-check-secure-tag-has-newline))
-    (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
-	  (if exit
-	      (message-send-and-exit arg)
-	    (message-send arg)))))
+    (let (tag-change
+	  post-send-tag-changes)
+      ;; post-send-tag-changes are tag-changes to apply after sending,
+      ;; but we need to store them now as the compose buffer is
+      ;; typically killed before message-send-and-exit returns.
+      (push (notmuch-message-mark-replied t) post-send-tag-changes)
+      (push (notmuch-draft--mark-deleted t) post-send-tag-changes)
+      (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
+	(if exit
+	    (message-send-and-exit arg)
+	  (message-send arg)))
+      (dolist (tag-change post-send-tag-changes)
+	(when tag-change
+	  (apply #'notmuch-tag tag-change))))))
 
 (defun notmuch-mua-send-and-exit (&optional arg)
   (interactive "P")
-- 
2.1.4

Re: [PATCH] emacs: remove use of message-send-hook

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> +    (let (tag-change
> +	  post-send-tag-changes)
> +      ;; post-send-tag-changes are tag-changes to apply after sending,
> +      ;; but we need to store them now as the compose buffer is
> +      ;; typically killed before message-send-and-exit returns.
> +      (push (notmuch-message-mark-replied t) post-send-tag-changes)
> +      (push (notmuch-draft--mark-deleted t) post-send-tag-changes)
> +      (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
> +	(if exit
> +	    (message-send-and-exit arg)
> +	  (message-send arg)))
> +      (dolist (tag-change post-send-tag-changes)
> +	(when tag-change
> +	  (apply #'notmuch-tag tag-change))))))

Would it be possible to concatenate the two lists and only call
notmuch-tag once? It seems like that should be notably faster.

d

Re: [PATCH] emacs: remove use of message-send-hook

[Mark Walters]

On Fri, 10 Mar 2017, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> +    (let (tag-change
>> +	  post-send-tag-changes)
>> +      ;; post-send-tag-changes are tag-changes to apply after sending,
>> +      ;; but we need to store them now as the compose buffer is
>> +      ;; typically killed before message-send-and-exit returns.
>> +      (push (notmuch-message-mark-replied t) post-send-tag-changes)
>> +      (push (notmuch-draft--mark-deleted t) post-send-tag-changes)
>> +      (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
>> +	(if exit
>> +	    (message-send-and-exit arg)
>> +	  (message-send arg)))
>> +      (dolist (tag-change post-send-tag-changes)
>> +	(when tag-change
>> +	  (apply #'notmuch-tag tag-change))))))

Hi

> Would it be possible to concatenate the two lists and only call
> notmuch-tag once? It seems like that should be notably faster.

I think that it's not possible as they are tagging different
messages. We could probably do something with --batch but that would
mean some change in notmuch-tag itself.

Best wishes

Mark

Re: [PATCH] emacs: remove use of message-send-hook

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Hi
>
>> Would it be possible to concatenate the two lists and only call
>> notmuch-tag once? It seems like that should be notably faster.
>
> I think that it's not possible as they are tagging different
> messages. We could probably do something with --batch but that would
> mean some change in notmuch-tag itself.

Ah right, I missed the fact that an id was returned as well from the two
invoked functions. Nevermind then.

Re: [PATCH] emacs: remove use of message-send-hook

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> We use message-send-hook for two things -- tagging drafts deleted, and
> tagging replied the parent message of a reply. We should not use
> message-send-hook as that will affect gnus etc too. Moreover,
> message-send-hook is run before the message is sent, even before the
> user confirms they want to send it (if message-confirm-send is
> set). This means the user can do C-c C-c to send, then cancel, and the
> parent is still tagged replied.

pushed to master

d