From: Tomi Ollila <tomi.ollila@iki.fi>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] contrib: pick: use notmuch-clean-address
In-Reply-To: <1354369006-1793-1-git-send-email-markwalters1009@gmail.com>
References: <1354369006-1793-1-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.14+116~g29fcdb5 (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Sat, 01 Dec 2012 16:31:55 +0200
Message-ID: <m28v9hhmhw.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Sat, Dec 01 2012, Mark Walters <markwalters1009@gmail.com> wrote:

> Now notmuch-clean-address is split out in show pick can use that (with a small
> wrapper).
>
> ---
> This removes another 50 lines or so of duplicated code from notmuch-pick.
>
> Best wishes
>
> Mark
>
>  contrib/notmuch-pick/notmuch-pick.el |   65 +++++----------------------------
>  1 files changed, 10 insertions(+), 55 deletions(-)
>
> diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
> index 5f9e456..6871d7b 100644
> --- a/contrib/notmuch-pick/notmuch-pick.el
> +++ b/contrib/notmuch-pick/notmuch-pick.el
> @@ -35,7 +35,6 @@
>  (declare-function notmuch-show "notmuch-show" (&rest args))
>  (declare-function notmuch-tag "notmuch" (query &rest tags))
>  (declare-function notmuch-show-strip-re "notmuch-show" (subject))
> -(declare-function notmuch-show-clean-address "notmuch-show" (parsed-address))
>  (declare-function notmuch-show-spaces-n "notmuch-show" (n))
>  (declare-function notmuch-read-query "notmuch" (prompt))
>  (declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
> @@ -521,62 +520,18 @@ than only the current message."
>  	  (message (format "Command '%s' exited abnormally with code %d"
>  			   shell-command exit-code)))))))
>  
> -;; Shamelessly stolen from notmuch-show.el: should be unified.
>  (defun notmuch-pick-clean-address (address)
> -  "Try to clean a single email ADDRESS for display.  Return
> +  "Try to clean a single email ADDRESS for display. Return
> +AUTHOR_NAME if present, otherwise return AUTHOR_EMAIL. Return
>  unchanged ADDRESS if parsing fails."
> -  (condition-case nil
> -    (let (p-name p-address)
> -      ;; It would be convenient to use `mail-header-parse-address',
> -      ;; but that expects un-decoded mailbox parts, whereas our
> -      ;; mailbox parts are already decoded (and hence may contain
> -      ;; UTF-8). Given that notmuch should handle most of the awkward
> -      ;; cases, some simple string deconstruction should be sufficient
> -      ;; here.
> -      (cond
> -       ;; "User <user@dom.ain>" style.
> -       ((string-match "\\(.*\\) <\\(.*\\)>" address)
> -	(setq p-name (match-string 1 address)
> -	      p-address (match-string 2 address)))
> -
> -       ;; "<user@dom.ain>" style.
> -       ((string-match "<\\(.*\\)>" address)
> -	(setq p-address (match-string 1 address)))
> -
> -       ;; Everything else.
> -       (t
> -	(setq p-address address)))
> -
> -      (when p-name
> -	;; Remove elements of the mailbox part that are not relevant for
> -	;; display, even if they are required during transport:
> -	;;
> -	;; Backslashes.
> -	(setq p-name (replace-regexp-in-string "\\\\" "" p-name))
> -
> -	;; Outer single and double quotes, which might be nested.
> -	(loop
> -	 with start-of-loop
> -	 do (setq start-of-loop p-name)
> -
> -	 when (string-match "^\"\\(.*\\)\"$" p-name)
> -	 do (setq p-name (match-string 1 p-name))
> -
> -	 when (string-match "^'\\(.*\\)'$" p-name)
> -	 do (setq p-name (match-string 1 p-name))
> -
> -	 until (string= start-of-loop p-name)))
> -
> -      ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
> -      ;; 'foo@bar.com'.
> -      (when (string= p-name p-address)
> -	(setq p-name nil))
> -
> -      ;; If we have a name return that otherwise return the address.
> -      (if (not p-name)
> -	  p-address
> -	p-name))
> -    (error address)))
> +  (let* ((clean-address (notmuch-clean-address address))
> +	 (p-address (car clean-address))
> +	 (p-name (cdr clean-address)))
> +
> +    ;; If we have a name return that otherwise return the address.
> +    (if (not p-name)
> +	p-address
> +      p-name)))

LGTM, but would this look so good... ?

   ;; If we have a name return that otherwise return the address.
   (or p-name p-address))

Tomi


>  
>  (defun notmuch-pick-insert-field (field format-string msg)
>    (let* ((headers (plist-get msg :headers))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch