Date: Sat, 4 Jun 2011 17:55:24 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Daniel Schoepe <daniel.schoepe@googlemail.com>
Subject: Re: [PATCH] emacs: Tab completion for notmuch-search and
	notmuch-search-filter
Message-ID: <20110604215523.GF29861@mit.edu>
References: <1307189970-728-1-git-send-email-daniel.schoepe@googlemail.com>
	<BANLkTimudbqYy8JhGHu67WsOS04mCm0hfA@mail.gmail.com>
	<87mxhxjrry.fsf@gilead.invalid>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87mxhxjrry.fsf@gilead.invalid>
User-Agent: Mutt/1.5.20 (2009-06-14)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Daniel Schoepe on Jun 04 at  9:55 pm:
> On Sat, 4 Jun 2011 11:32:15 -0400, Austin Clements <amdragon@mit.edu> wrote:
> > Dynamic scoping is obnoxious, but I think programmed completion is
> > steeped in the assumption that you'll use it.  This code would be much
> > simpler if notmuch-query-completions took only `string' and used the
> > dynamically-bound all-compls (which should probably be renamed
> > notmuch-completions or something if you do this).  Then this could be
> > just
> >   (minibuffer-completion-table (completion-table-dynamic
> > #'notmuch-query-completions)))
> > and there'd be no need for quasiquoting, comments, and fake lexical scoping.
> 
> Sounds reasonable, I guess I really should stop fighting all those ugly
> parts of elisp with unreadable constructs like that. I made it a global
> variable though to avoid compilation warnings about notmuch-completion
> being a free variable. Since it's contents are not dependent on
> how/where notmuch-read-query is called, this shouldn't cause any
> problems, except my personal discomfort arising from the use of side
> effects for something as simple as this. :)

Oh, sorry, I wasn't suggesting setq'ing a global.  I agree that that's
really ugly.  Rather, something like

(defun notmuch-query-completions (string)
  ... as you have it now ...)

(defun notmuch-read-query (prompt)
  (let ((notmuch-completions (append (list "folder:" ...)))
        (keymap ...)
        (minibuffer-completion-table ...))
    (read-from-minibuffer ...)))

Unfortunately, you still need the global defvar to avoid compilation
warnings, but this at least avoids the side-effects, and is probably
how programmed completion was intended to be used.

Alternatively, here's a completely different way to structure this
that avoids globals and dynamic scoping entirely.  This is how some of
the standard completing read functions appear to work:

(defun notmuch-read-query (prompt)
  "Read a notmuch-query from the minibuffer with completion.

PROMPT is the string to prompt with."
  (lexical-let ((completions
		 (append (list "folder:" "thread:" "id:" "date:" "from:" "to:"
			       "subject:" "attachment:")
			 (mapcar (lambda (tag)
				   (concat "tag:" tag))
				 (process-lines "notmuch" "search-tags")))))
    (let ((minibuffer-completion-table
	   (completion-table-dynamic
	    (lambda (string)
	      (cond
	       ;; this ugly regexp is used to get the last word of the
	       ;; input possibly preceded by a '('
	       ((string-match "\\(^\\|.* (?\\)\\([^ ]*\\)$" string)
		(mapcar (lambda (compl)
			  (concat (match-string-no-properties 1 string) compl))
			(all-completions (match-string-no-properties 2 string)
					 completions)))
	       (t (list string))))))
	  (keymap (copy-keymap minibuffer-local-map)))
      ;; this was simpler than convincing completing-read to accept spaces:
      (define-key keymap (kbd "<tab>") 'minibuffer-complete)
    (read-from-minibuffer prompt nil keymap nil minibuffer-history nil nil))))

> > > +    (define-key keymap (kbd "<tab>") 'minibuffer-complete)
> > 
> > This probably deserves a comment about why you're doing so much work
> > to avoid completing-read (which I assume is because it also binds SPC,
> > even if require-match is nil, which is unfortunate).
> 
> Yes, that was the reason.
> 
> Another thing that bugs me, is that I did not find a better way of doing
> the completion: Ideally I'd like to just specify a list of completions
> for individual words and have emacs handle separating the input string
> into individual words, but I couldn't find any options to accomplish
> that.

Yeah, I futzed with it for a bit, swearing that there had to be a
better way, but didn't find one either.