From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: address completion,
 allow sender/recipient and filters
In-Reply-To: <1450602453-2623-1-git-send-email-markwalters1009@gmail.com>
References: <1450602453-2623-1-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.21+30~g55c056a (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Mon, 28 Dec 2015 12:50:08 +0100
Message-ID: <874mf2yeq7.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Hi Mark,

I tried this patch. When I set the variable to 'received, first
completion candidates are generated incorrectly. The query looked as
"(to:sojkam1@fel.cvut.cz) and (to:prefix*)". It should be "...
(from:prefix*)". This "to:" comes from notmuch-address-options or from
notmuch-company so these should be changes as well.

Other minor comments are below.

Thanks.
-Michal

On Sun, Dec 20 2015, Mark Walters wrote:
> This commit lets the user customize the address completion.
>
> The first change controls whether to build the address completion list
> based on messages you have sent or you have received (the latter is
> much faster).
>
> The second change add a possible filter query to limit the messages
> used -- for example, setting this to date:1y..  would limit the
> address completions to addresses used in the last year. This speeds up
> the address harvest and may also make the search less cluttered as old
> addresses may well no longer be valid.
> ---
>
> There was some discussion on irc about the address completion harvest
> taking a long time. This patch makes is possible to tune this
> behaviour. It may be that this complicates the customize variable too
> much -- but if nothing else its a patch that anyone who experiences
> problems can try.
>
> Best wishes
>
> Mark
>
> emacs/notmuch-address.el | 64 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index 49e2402..8942fe4 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -26,15 +26,40 @@
>  ;;
>  (declare-function company-manual-begin "company")
>
> +(defvar notmuch-address-last-harvest 0
> +  "Time of last address harvest")
> +
> +(defvar notmuch-address-completions (make-hash-table :test 'equal)
> +  "Hash of email addresses for completion during email composition.
> +  This variable is set by calling `notmuch-address-harvest'.")
> +
> +(defvar notmuch-address-full-harvest-finished nil
> +  "t indicates that full completion address harvesting has been
> +finished")
> +
>  (defcustom notmuch-address-command 'internal

Default value should be changed to something matching the :type. Perhaps
'(sent nil).

>    "The command which generates possible addresses. It must take a
>  single argument and output a list of possible matches, one per
>  line. The default value of `internal' uses built-in address
>  completion."

Doc text can be updated as well.

>    :type '(radio
> -	  (const :tag "Use internal address completion" internal)
> +	  (list :tag "Use internal address completion"
> +		(radio
> +		 :tag "Build list based on messages you have"
> +		 :value sent
> +		 (const :tag "sent" sent)
> +		 (const :tag "received" received))

I would mention that received is faster here.

> +		(radio :tag "Filter messages used for completion"
> +		       (const :tag "Use all messages" nil)
> +		       (string :tag "Filter query")))
>  	  (const :tag "Disable address completion" nil)
> -	  (string :tag "Use external completion command" "notmuch-addresses"))
> +	  (string :tag "Use external completion command"))
> +  ;; We override set so that we can clear the cache when this changes
> +  :set (lambda (symbol value)
> +	 (set-default symbol value)
> +	 (setq notmuch-address-last-harvest 0)
> +	 (setq notmuch-address-completions (clrhash notmuch-address-completions))
> +	 (setq notmuch-address-full-harvest-finished nil))
>    :group 'notmuch-send
>    :group 'notmuch-external)
>
> @@ -49,17 +74,6 @@ to know how address selection is made by default."
>    :group 'notmuch-send
>    :group 'notmuch-external)
>
> -(defvar notmuch-address-last-harvest 0
> -  "Time of last address harvest")
> -
> -(defvar notmuch-address-completions (make-hash-table :test 'equal)
> -  "Hash of email addresses for completion during email composition.
> -  This variable is set by calling `notmuch-address-harvest'.")
> -
> -(defvar notmuch-address-full-harvest-finished nil
> -  "t indicates that full completion address harvesting has been
> -finished")
> -
>  (defun notmuch-address-selection-function (prompt collection initial-input)
>    "Call (`completing-read'
>        PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
> @@ -81,7 +95,8 @@ finished")
>
>  (defun notmuch-address-setup ()
>    (let* ((use-company (and notmuch-address-use-company
> -			   (eq notmuch-address-command 'internal)
> +			   notmuch-address-command
> +			   (listp notmuch-address-command)
>  			   (require 'company nil t)))
>  	 (pair (cons notmuch-address-completion-headers-regexp
>  		     (if use-company
> @@ -109,7 +124,7 @@ The candidates are taken from `notmuch-address-completions'."
>  elisp-based implementation or older implementation requiring
>  external commands."
>    (cond
> -   ((eq notmuch-address-command 'internal)
> +   ((and notmuch-address-command (listp notmuch-address-command))
>      (when (not notmuch-address-full-harvest-finished)
>        ;; First, run quick synchronous harvest based on what the user
>        ;; entered so far
> @@ -198,12 +213,21 @@ addresses from those messages and stores them in
>  time so the address collection runs asynchronously unless
>  SYNCHRONOUS is t. In case of asynchronous execution, CALLBACK is
>  called when harvesting finishes."
> -  (let* ((from-me-query (mapconcat (lambda (x) (concat "from:" x)) (notmuch-user-emails) " or "))
> -	 (query (if filter-query
> -		    (format "(%s) and (%s)" from-me-query filter-query)
> -		  from-me-query))
> +  (let* ((sent (eq (car notmuch-address-command) 'sent))
> +	 (user-query (cadr notmuch-address-command))
> +	 (from-or-to-me-query
> +	  (mapconcat (lambda (x)
> +		       (concat (if sent "from:" "to:") x))
> +		     (notmuch-user-emails) " or "))
> +	 (query (if (or filter-query user-query)
> +		    (concat (format "(%s)" from-or-to-me-query)
> +			    (when filter-query
> +			      (format " and (%s)" filter-query))
> +			    (when user-query
> +			      (format " and (%s)" user-query)))
> +		  from-or-to-me-query))
>  	 (args `("address" "--format=sexp" "--format-version=2"
> -		 "--output=recipients"
> +		 ,(if sent "--output=recipients" "--output=sender")
>  		 "--deduplicate=address"
>  		 ,query)))
>      (if synchronous
> --
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch