From: Mark Walters <markwalters1009@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Cc: tomi.ollila@iki.fi
Subject: Re: [PATCH 3/4] emacs: add function to resend message to new
 recipients
In-Reply-To: <1441204482-26509-3-git-send-email-tomi.ollila@iki.fi>
References: <1441204482-26509-1-git-send-email-tomi.ollila@iki.fi>
 <1441204482-26509-3-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.18+469~gb743c48 (http://notmuchmail.org) Emacs/23.4.1
 (x86_64-pc-linux-gnu)
Date: Wed, 28 Oct 2015 08:00:11 +0000
Message-ID: <87pozza0dw.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


On Wed, 02 Sep 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> The new function notmuch-show-message-resend re-sends
> message to new recipients using #'message-resend.
>
> Recipients are read from minibuffer as a comma-separated
> string (with some keyboard support including tab completion).
>
> Final confirmation before sending is asked.
> ---

This series looks good to me modulo three minor comments  (I even
like the choice of binding to b). 

The first comment is that, in light of the recent address series, I
think Patch 2 can be dropped completely, with a small change to this
patch. See below for that, and the other two minor suggestions.

Also, I wonder if when it gets a NEWS item it might be worth mentioning
that a user might have already added this function to their emacs init
file as it has been on the wiki. (It took me way too long to realise
that was why this patch was not working for me!)

Best wishes

Mark


>
> Since id:1440619626-18768-2-git-send-email-tomi.ollila@iki.fi
>   - changed (bury-buffer) to (notmuch-bury-or-kill-this-buffer)
>     - it is hard to have the buffer been kept around but it is posiible
>
>  emacs/notmuch-address.el | 19 +++++++++++++++++++
>  emacs/notmuch-show.el    |  8 ++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index 8982a415ce11..83788efd3c1b 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -119,4 +119,23 @@ (defun notmuch-address-locate-command (command)
>  
>  ;;
>  
> +(defun notmuch-address-from-minibuffer (prompt)
> +  (if (not (notmuch-address--message-insinuated))
> +      (read-string prompt)

This can become 
     (if (not notmuch-address-command)
 
> +    (let ((rmap (copy-keymap minibuffer-local-map))
> +	  (omap minibuffer-local-map))
> +      ;; Configure TAB to start completion when executing read-string.
> +      ;; "Original" minibuffer keymap is restored just before calling
> +      ;; notmuch-address-expand-name as it may also use minibuffer-local-map
> +      ;; (completing-read probably does not but if something else is used there).
> +      (define-key rmap "\C-i" (lambda () ;; TAB

I think this could become (define-key rmap (kbd "TAB") (lambda () ....

which is easy to read and consistent with the definitions of other
keymaps (eg the main notmuch-show keymap).

> +			       (interactive)
> +			       (let ((enable-recursive-minibuffers t)
> +				     (minibuffer-local-map omap))
> +				 (notmuch-address-expand-name))))
> +      (let ((minibuffer-local-map rmap))
> +	(read-string prompt)))))
> +
> +;;
> +
>  (provide 'notmuch-address)
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 0565ab0725b2..046cb0e41f0b 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1806,6 +1806,14 @@ (defun notmuch-show-forward-message (&optional prompt-for-sender)
>    (with-current-notmuch-show-message
>     (notmuch-mua-new-forward-message prompt-for-sender)))
>  
> +(defun notmuch-show-resend-message (addresses)
> +  "Resend the current message."
> +  (interactive (list (notmuch-address-from-minibuffer "Resend to: ")))
> +  (when (yes-or-no-p (concat "Confirm resend to " addresses " "))

Perhaps y-or-n-p rather than yes-or-no-p?
 
> +    (notmuch-show-view-raw-message)
> +    (message-resend addresses)
> +    (notmuch-bury-or-kill-this-buffer)))
> +
>  (defun notmuch-show-next-message (&optional pop-at-end)
>    "Show the next message.
>  
> -- 
> 2.0.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch