From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3] emacs: address completion,
 allow sender/recipient and filters
In-Reply-To: <1463384211-13086-1-git-send-email-markwalters1009@gmail.com>
References: <87h9e11vq1.fsf@steelpick.2x.cz>
 <1463384211-13086-1-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.22+17~ga5b8c02 (http://notmuchmail.org) Emacs/24.5.1
 (x86_64-pc-linux-gnu)
Date: Tue, 17 May 2016 13:53:04 +0200
Message-ID: <8737pg99zz.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Hi Mark,

few minor comments bellow.

On Mon, May 16 2016, Mark Walters wrote:
> This commit lets the user customize the address completion.

Add "It makes two changes."

> 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.
> ---
>
> This version uses the docstrings suggested my Michal (which are much
> better than mine), and renames some variables, as he suggested, to
> make the code clearer.
>
> I wondered about allowing the user to specify completion based on
> message "sent or received", rather than either sent, or received, but
> think that is adding too much mess. We could also allow completion
> based on any messages in the database, which would the include
> completion based on messages received via mailing lists or
> distribution lists.
>
> I also note that if the user enters a bad query into the filter query
> box (eg 6M.. rather than date:6M..) they may get an obscure error as
> notmuch/xapian fails. I don't see a goo way round that so have left
> that as a "don't do that" case.

Agreed.

>  emacs/notmuch-address.el | 119 ++++++++++++++++++++++++++++++++---------------
>  emacs/notmuch-company.el |   2 +-
>  2 files changed, 82 insertions(+), 39 deletions(-)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index aafbe5f..3dc5da9 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -28,15 +28,51 @@
>  ;;
>  (declare-function company-manual-begin "company")
>  
> -(defcustom notmuch-address-command 'internal
> -  "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."
> +(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 '(sent nil)
> +  "Determines how to generate address completion candidates.
> +
> +If it is a string then that string should be an external program
> +which must take a single argument (searched string) and output a
> +list of completion candidates, one per line.
> +
> +Alternatively, it can be a (non-nil) list, in which case internal
> +completion is used; in this case the list should have form
> +'(DIRECTION FILTER), where DIRECTION is either sent or received
> +and specifies whether the candidates are searched in messages
> +sent by the user or received by the user (note received by is
> +much faster), and FILTER is either nil or a filter-string, such
> +as \"date:1y..\" to append to the query.
> +
> +If this variable is nil then address completion is disabled."
>    :type '(radio
> -	  (const :tag "Use internal address completion" internal)
> +	  (list :tag "Use internal address completion"
> +		(radio
> +		 :tag "Base completion on messages you have"
> +		 :value sent
> +		 (const :tag "sent" sent)
> +		 (const :tag "received" received))

I think, users will be more happy if they understand the difference
without reading the full doc string.

		 (const :tag "sent (more accurate)" sent)
		 (const :tag "received (faster)" received))


Thanks.
-Michal