To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: add stash support for git send-email command line
In-Reply-To: <1414525138-838-1-git-send-email-jani@nikula.org>
References: <1414525138-838-1-git-send-email-jani@nikula.org>
User-Agent: Notmuch/0.18.1 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-apple-darwin14.0.0)
From: David Edmondson <dme@dme.org>
Date: Wed, 29 Oct 2014 08:41:22 +0000
Message-ID: <m2h9yn6zf1.fsf@dme.org>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue, Oct 28 2014, Jani Nikula wrote:
> Stash From/To/Cc as --to/--to/--cc, respectively, and Message-Id as
> --in-reply-to, suitable for pasting to git send-email command line.
> ---
>  emacs/notmuch-show.el | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index a9974826e824..328c93ba0584 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1274,6 +1274,7 @@ reset based on the original query."
>      (define-key map "t" 'notmuch-show-stash-to)
>      (define-key map "l" 'notmuch-show-stash-mlarchive-link)
>      (define-key map "L" 'notmuch-show-stash-mlarchive-link-and-go)
> +    (define-key map "G" 'notmuch-show-stash-git-send-email)
>      (define-key map "?" 'notmuch-subkeymap-help)
>      map)
>    "Submap for stash commands")
> @@ -2125,6 +2126,30 @@ the user (see `notmuch-show-stash-mlarchive-link-alist')."
>    (notmuch-show-stash-mlarchive-link mla)
>    (browse-url (current-kill 0 t)))
>  
> +(defun notmuch-show-stash-git-helper (addresses prefix)
> +  "Escape, trim, and add PREFIX to each address in list of ADDRESSES."
> +  (when addresses
> +    (mapconcat (lambda (x)
> +		 (concat prefix "\""
> +			 ;; escape double-quotes
> +			 (replace-regexp-in-string
> +			  "\"" "\\\\\""
> +			  ;; trim leading and trailing spaces
> +			  (replace-regexp-in-string
> +			   "\\(^ *\\| *$\\)" ""
> +			   x)) "\" "))
> +	       addresses "")))

This doesn't seem quite right. You leave a spurious trailing
space. Maybe that's because you need it in the following function to
separate the from/to/cc elements? That kind of interaction between the
two functions is icky.

There's no need to test `addresses' at the head of the list - mapconcat
is fine with nil.

How about:

(defun notmuch-show-stash-git-helper (addresses prefix)
  "Escape, trim, and add PREFIX to each address in list of ADDRESSES."
  (mapconcat (lambda (x)
	       (concat prefix "\""
		       ;; escape double-quotes
		       (replace-regexp-in-string
			"\"" "\\\\\""
			;; trim leading and trailing spaces
			(replace-regexp-in-string
			 "\\(^ *\\| *$\\)" ""
			 x)) "\""))
	     addresses " "))

This would remove the trailing space, so...

> +
> +(defun notmuch-show-stash-git-send-email ()
> +  "Copy From/To/Cc/Message-Id of current message to kill-ring in a form suitable for pasting to git send-email command line."
> +  (interactive)
> +  (notmuch-common-do-stash
> +   (concat
> +    (notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-from)) "--to=")
> +    (notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-to)) "--to=")
> +    (notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-cc)) "--cc=")
> +    (concat "--in-reply-to=\"" (notmuch-show-get-message-id t) "\""))))

...this would have to use something like:

  (mapconcat 'identity (list
			(notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-from)) "--to=")
			(notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-to)) "--to=")
			(notmuch-show-stash-git-helper (message-tokenize-header (notmuch-show-get-cc)) "--cc=")
			(concat "--in-reply-to=\"" (notmuch-show-get-message-id t) "\""))
	     "")

to separate the chunks (untested).

Could you avoid the double-quote quoting by using single quotes inside
the strings? Do the leading and trailing spaces really matter?

(Domo: Look, I managed to write 'separate', twice!)

> +
>  ;; Interactive part functions and their helpers
>  
>  (defun notmuch-show-generate-part-buffer (message-id nth)
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch