--- /dev/null
+Return-Path: <sojkam1@fel.cvut.cz>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by arlo.cworth.org (Postfix) with ESMTP id 1DB6F6DE02B2\r
+ for <notmuch@notmuchmail.org>; Fri, 13 May 2016 14:42:12 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at cworth.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -1.635\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-1.635 tagged_above=-999 required=5 tests=[AWL=0.675,\r
+ RCVD_IN_DNSWL_MED=-2.3, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled\r
+Received: from arlo.cworth.org ([127.0.0.1])\r
+ by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id osPu82yF95Js for <notmuch@notmuchmail.org>;\r
+ Fri, 13 May 2016 14:42:03 -0700 (PDT)\r
+Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36])\r
+ by arlo.cworth.org (Postfix) with ESMTP id 2C9646DE0317\r
+ for <notmuch@notmuchmail.org>; Fri, 13 May 2016 14:42:03 -0700 (PDT)\r
+Received: from localhost (unknown [192.168.200.7])\r
+ by max.feld.cvut.cz (Postfix) with ESMTP id 977F45CD662;\r
+ Fri, 13 May 2016 23:42:01 +0200 (CEST)\r
+X-Virus-Scanned: IMAP STYX AMAVIS\r
+Received: from max.feld.cvut.cz ([192.168.200.1])\r
+ by localhost (styx.feld.cvut.cz [192.168.200.7]) (amavisd-new, port 10044)\r
+ with ESMTP id oFXwixJwOxl4; Fri, 13 May 2016 23:41:59 +0200 (CEST)\r
+Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34])\r
+ by max.feld.cvut.cz (Postfix) with ESMTP id 974735CD275;\r
+ Fri, 13 May 2016 23:41:59 +0200 (CEST)\r
+Received: from wsh by steelpick.2x.cz with local (Exim 4.87)\r
+ (envelope-from <sojkam1@fel.cvut.cz>)\r
+ id 1b1KqM-00014s-Va; Fri, 13 May 2016 23:41:58 +0200\r
+From: Michal Sojka <sojkam1@fel.cvut.cz>\r
+To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v2] emacs: address completion,\r
+ allow sender/recipient and filters\r
+In-Reply-To: <1463153337-11381-1-git-send-email-markwalters1009@gmail.com>\r
+References: <874mf2yeq7.fsf@steelpick.2x.cz>\r
+ <1463153337-11381-1-git-send-email-markwalters1009@gmail.com>\r
+User-Agent: Notmuch/0.22 (http://notmuchmail.org) Emacs/24.5.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Fri, 13 May 2016 23:41:58 +0200\r
+Message-ID: <87h9e11vq1.fsf@steelpick.2x.cz>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.20\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <https://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <https://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Fri, 13 May 2016 21:42:12 -0000\r
+\r
+Hi Mark,\r
+\r
+On Fri, May 13 2016, Mark Walters wrote:\r
+> This commit lets the user customize the address completion.\r
+>\r
+> The first change controls whether to build the address completion list\r
+> based on messages you have sent or you have received (the latter is\r
+> much faster).\r
+>\r
+> The second change add a possible filter query to limit the messages\r
+> used -- for example, setting this to date:1y.. would limit the\r
+> address completions to addresses used in the last year. This speeds up\r
+> the address harvest and may also make the search less cluttered as old\r
+> addresses may well no longer be valid.\r
+> ---\r
+>\r
+> I have fixed the bug pointed out by Michal in his review. It seems to\r
+> work, but there are a lot of possible configurations,\r
+>\r
+> I also don't like my docstring for the notmuch-address-command\r
+> defcustom, so any suggestions gratefully received.\r
+>\r
+> Also, I am not sure whether this is all to much complexity for this\r
+> feature.\r
+\r
+A big part of the patch is moving variable definitions and I'd say the\r
+rest is OK (complexity-wise). Other comments below.\r
+\r
+> emacs/notmuch-address.el | 104 +++++++++++++++++++++++++++++++----------------\r
+> emacs/notmuch-company.el | 2 +-\r
+> 2 files changed, 71 insertions(+), 35 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el\r
+> index aafbe5f..8b84a4c 100644\r
+> --- a/emacs/notmuch-address.el\r
+> +++ b/emacs/notmuch-address.el\r
+> @@ -28,15 +28,50 @@\r
+> ;;\r
+> (declare-function company-manual-begin "company")\r
+>\r
+> -(defcustom notmuch-address-command 'internal\r
+> - "The command which generates possible addresses. It must take a\r
+> -single argument and output a list of possible matches, one per\r
+> -line. The default value of `internal' uses built-in address\r
+> -completion."\r
+> +(defvar notmuch-address-last-harvest 0\r
+> + "Time of last address harvest")\r
+> +\r
+> +(defvar notmuch-address-completions (make-hash-table :test 'equal)\r
+> + "Hash of email addresses for completion during email composition.\r
+> + This variable is set by calling `notmuch-address-harvest'.")\r
+> +\r
+> +(defvar notmuch-address-full-harvest-finished nil\r
+> + "t indicates that full completion address harvesting has been\r
+> +finished")\r
+> +\r
+> +(defcustom notmuch-address-command '(sent nil)\r
+> + "The command which generates possible addresses.\r
+> +\r
+> +It can be a (non-nil) list, in which case internal completion is\r
+> +used; in this case the first list item 'sent/'received specifies\r
+> +whether you match message sent by the user or received by the\r
+> +user (note received by is much faster), and the second list item\r
+> +should be nil or a filter-string, such as \"date:1y..\" to append\r
+> +to the query.\r
+> +\r
+> +If this variable is nil then address completion is disabled.\r
+> +\r
+> +If it is a string then that string should be an external program\r
+> +which must take a single argument and output a list of possible\r
+> +matches, one per line."\r
+\r
+What about the following? (I think that the string value should be\r
+described first because it corresponds to the name of the variable)\r
+\r
+ "Determines how to generate address completion candidates.\r
+\r
+ If it is a string then that string should be an external program\r
+ which must take a single argument (searched string) and output a list\r
+ of completion candidates, one per line.\r
+\r
+ Alternatively, it can be a (non-nil) list, in which case internal\r
+ completion is used; in this case the list should have form\r
+ '(DIRECTION FILTER), where DIRECTION is either sent or received and\r
+ specifies whether the candidates are searched in messages sent by the\r
+ user or received by the user (note received by is much faster), and\r
+ FILTER is either nil or a filter-string, such as \"date:1y..\" to\r
+ append to the query.\r
+\r
+ If this variable is nil then address completion is disabled."\r
+\r
+> :type '(radio\r
+> - (const :tag "Use internal address completion" internal)\r
+> + (list :tag "Use internal address completion"\r
+> + (radio\r
+> + :tag "Build list based on messages you have"\r
+\r
+What about "Base completion on messages you have"?\r
+\r
+> + :value sent\r
+> + (const :tag "sent" sent)\r
+> + (const :tag "received" received))\r
+> + (radio :tag "Filter messages used for completion"\r
+> + (const :tag "Use all messages" nil)\r
+> + (string :tag "Filter query")))\r
+> (const :tag "Disable address completion" nil)\r
+> - (string :tag "Use external completion command" "notmuch-addresses"))\r
+> + (string :tag "Use external completion command"))\r
+> + ;; We override set so that we can clear the cache when this changes\r
+> + :set (lambda (symbol value)\r
+> + (set-default symbol value)\r
+> + (setq notmuch-address-last-harvest 0)\r
+> + (setq notmuch-address-completions (clrhash notmuch-address-completions))\r
+> + (setq notmuch-address-full-harvest-finished nil))\r
+> :group 'notmuch-send\r
+> :group 'notmuch-external)\r
+>\r
+> @@ -51,17 +86,6 @@ to know how address selection is made by default."\r
+> :group 'notmuch-send\r
+> :group 'notmuch-external)\r
+>\r
+> -(defvar notmuch-address-last-harvest 0\r
+> - "Time of last address harvest")\r
+> -\r
+> -(defvar notmuch-address-completions (make-hash-table :test 'equal)\r
+> - "Hash of email addresses for completion during email composition.\r
+> - This variable is set by calling `notmuch-address-harvest'.")\r
+> -\r
+> -(defvar notmuch-address-full-harvest-finished nil\r
+> - "t indicates that full completion address harvesting has been\r
+> -finished")\r
+> -\r
+> (defun notmuch-address-selection-function (prompt collection initial-input)\r
+> "Call (`completing-read'\r
+> PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"\r
+> @@ -83,7 +107,8 @@ finished")\r
+>\r
+> (defun notmuch-address-setup ()\r
+> (let* ((use-company (and notmuch-address-use-company\r
+> - (eq notmuch-address-command 'internal)\r
+> + notmuch-address-command\r
+> + (listp notmuch-address-command)\r
+> (require 'company nil t)))\r
+> (pair (cons notmuch-address-completion-headers-regexp\r
+> (if use-company\r
+> @@ -111,11 +136,11 @@ The candidates are taken from `notmuch-address-completions'."\r
+> elisp-based implementation or older implementation requiring\r
+> external commands."\r
+> (cond\r
+> - ((eq notmuch-address-command 'internal)\r
+> + ((and notmuch-address-command (listp notmuch-address-command))\r
+> (when (not notmuch-address-full-harvest-finished)\r
+> ;; First, run quick synchronous harvest based on what the user\r
+> ;; entered so far\r
+> - (notmuch-address-harvest (format "to:%s*" original) t))\r
+> + (notmuch-address-harvest original t))\r
+> (prog1 (notmuch-address-matching original)\r
+> ;; Then start the (potentially long-running) full asynchronous harvest if necessary\r
+> (notmuch-address-harvest-trigger)))\r
+> @@ -191,21 +216,32 @@ external commands."\r
+>\r
+> The car is a partial harvest, and the cdr is a full harvest")\r
+>\r
+> -(defun notmuch-address-harvest (&optional filter-query synchronous callback)\r
+> +(defun notmuch-address-harvest (&optional filter-string synchronous callback)\r
+> "Collect addresses completion candidates. It queries the\r
+> -notmuch database for all messages sent by the user optionally\r
+> -matching FILTER-QUERY (if not nil). It collects the destination\r
+> -addresses from those messages and stores them in\r
+> -`notmuch-address-completions'. Address harvesting may take some\r
+> -time so the address collection runs asynchronously unless\r
+> -SYNCHRONOUS is t. In case of asynchronous execution, CALLBACK is\r
+> -called when harvesting finishes."\r
+> - (let* ((from-me-query (mapconcat (lambda (x) (concat "from:" x)) (notmuch-user-emails) " or "))\r
+> - (query (if filter-query\r
+> - (format "(%s) and (%s)" from-me-query filter-query)\r
+> - from-me-query))\r
+> +notmuch database for messages sent/received by the user\r
+> +optionally with to/from matching FILTER-STRING (if not nil). It\r
+> +collects the destination addresses from those messages and stores\r
+> +them in `notmuch-address-completions'. Address harvesting may\r
+> +take some time so the address collection runs asynchronously\r
+> +unless SYNCHRONOUS is t. In case of asynchronous execution,\r
+> +CALLBACK is called when harvesting finishes."\r
+\r
+I'm a bit lost in the names of variables here so I suggest more\r
+descriptive names below. Also the doc string could be IMHO better.\r
+\r
+(defun notmuch-address-harvest (&optional addr-prefix synchronous callback)\r
+ "Collect addresses completion candidates.\r
+\r
+ It queries the notmuch database for messages sent/received (as\r
+ configured with `notmuch-address-command`) by the user, collects\r
+ destination/source addresses from those messages and stores them in\r
+ `notmuch-address-completions'.\r
+\r
+ If ADDR-PREFIX is not nil, only messages with to/from addresses\r
+ matching ADDR-PREFIX*' are queried.\r
+\r
+ Address harvesting may take some time so the address collection runs\r
+ asynchronously unless SYNCHRONOUS is t. In case of asynchronous\r
+ execution, CALLBACK is called when harvesting finishes."\r
+\r
+> + (let* ((sent (eq (car notmuch-address-command) 'sent))\r
+> + (user-query (cadr notmuch-address-command))\r
+\r
+config-query\r
+\r
+> + (filter-query (when filter-string\r
+> + (format "%s:%s*" (if sent "to" "from") filter-string)))\r
+\r
+prefix-query\r
+\r
+> + (from-or-to-me-query\r
+> + (mapconcat (lambda (x)\r
+> + (concat (if sent "from:" "to:") x))\r
+> + (notmuch-user-emails) " or "))\r
+> + (query (if (or filter-query user-query)\r
+> + (concat (format "(%s)" from-or-to-me-query)\r
+> + (when filter-query\r
+> + (format " and (%s)" filter-query))\r
+> + (when user-query\r
+> + (format " and (%s)" user-query)))\r
+> + from-or-to-me-query))\r
+> (args `("address" "--format=sexp" "--format-version=2"\r
+> - "--output=recipients"\r
+> + ,(if sent "--output=recipients" "--output=sender")\r
+> "--deduplicate=address"\r
+> ,query)))\r
+> (if synchronous\r
+> diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el\r
+> index b881d6d..dcb59cd 100644\r
+> --- a/emacs/notmuch-company.el\r
+> +++ b/emacs/notmuch-company.el\r
+> @@ -72,7 +72,7 @@\r
+> (lambda (callback)\r
+> ;; First run quick asynchronous harvest based on what the user entered so far\r
+> (notmuch-address-harvest\r
+> - (format "to:%s*" arg) nil\r
+> + arg nil\r
+> (lambda (_proc _event)\r
+> (funcall callback (notmuch-address-matching arg))\r
+> ;; Then start the (potentially long-running) full asynchronous harvest if necessary\r
+> --\r
+> 2.1.4\r
+\r
+Cheers\r
+-Michal\r
projects / notmuch-archives.git / commitdiff