[Patch v5 2/4] emacs: postpone a message

[David Bremner]

David Bremner <david at tethera.net> writes:

> From: Mark Walters <markwalters1009 at gmail.com>

This really Mark's work, that I have split out into a separate file.

> +(defcustom notmuch-draft-tags '("+draft")
> +  "List of tags changes to apply to a draft message when it is saved in the database.

Here and a few other places the documentation uses "the database" to
mean the directory hierachy containing mail messages + the xapian
database.  At some point I would like to be able to distinguish between
the database and the maildir root (which doesn't need to be maildirs)
when talking about configuration. I don't really know better terminology
here, but I thought I would mention it in case someone else is inspired.

> +(defun notmuch-draft--mark-deleted ()

This -- naming convention is my contribution. Perhaps eventually we
could mark all private functions not intended to be called by users this
way. Since it is essentially cosmetic, I didn't want to do that
now. Indeed, one could quibble about the correctness of calling a
function private and then putting it in a public hook.

> +(defun notmuch-draft-quote-some-mml ()
> +  "Quote the mml tags in `notmuch-draft-quoted-tags`."
> +  (save-excursion
> +    ;; First we deal with any secure tag separately.
> +    (message-goto-body)
> +    (when (looking-at "<#secure[^\n]*>\n")
> +      (let ((secure-tag (match-string 0)))
> +	(delete-region (match-beginning 0) (match-end 0))
> +	(message-add-header (concat "X-Notmuch-Emacs-Secure: " secure-tag))))
> +    ;; This is copied from mml-quote-region but only quotes the
> +    ;; specified tags.
> +    (when notmuch-draft-quoted-tags
> +      (let ((re (concat "<#!*/?\\("
> +			(mapconcat 'identity notmuch-draft-quoted-tags "\\|")
> +			"\\)")))
One "hidden feature" is that regex characters in the quoted tags will be
interpreted. Possibly calling regexp-quote instead of identity would be
extra cautious here?

> +(defun notmuch-draft-save ()
> +  "Save the current draft message in the notmuch database.
> +
> +This saves the current message in the database with tags
> +`notmuch-draft-tags` (in addition to any default tags
> +applied to newly inserted messages)."
> +  (interactive)
> +  (let (;; We need the message id as we need it for tagging. Note
> +	;; message-make-message-id gives the id inside a "<" ">" pair,
> +	;; but notmuch doesn't want that form, so remove them.
> +	(id (concat "draft-" (substring (message-make-message-id) 1
> -1))))

what do you think of isolating this code and commentary in a private
function?

> +     (if (member 'Message-ID message-deletable-headers)
> +	 (progn
> +	   (message-remove-header "Message-ID")
> +	   (message-add-header (concat "Message-ID: <" id ">")))
> +       (message "You have customized emacs so Message-ID is not a deletable header, so not changing it")
> +       (setq id nil))

I'm not sure if it's just me, but I find the (if (progn ...)
else-clauses) a bit off-putting. An alternative would be to use cond
     
(cond
 ((member 'Message-ID message-deletable-headers)
  (message-remove-header "Message-id")
  (message-add-header (concat "Message-ID: <" id ">")))
 (t
  (message "You have customized emacs so Message-ID is not a deletable header, so not changing it")
  (setq id nil)))


> +(add-hook 'message-send-hook 'notmuch-draft--mark-deleted)

Can we avoid this by adding some code notmuch-mua-send-common?

> --- /dev/null
> +++ b/test/T630-emacs-draft.sh
> @@ -0,0 +1,42 @@
> +#!/usr/bin/env bash
> +test_description="Emacs Draft Handling"
> +. ./test-lib.sh || exit 1

Ok, now I remember I wrote this part, so someone else should review.