Subject: Re: bug#16604: 24.3.50; False negatives in lisp-completion-at-point

From: Dmitry Gutov <[email-masked]>
Date: Wed, 05 Feb 2014 06:41:14 +0200

On 04.02.2014 19:54, Stefan Monnier wrote:
> I was thinking of a new property which would define a function which
> reduces the set of candidates considered for
> completion-try-completions.  File completion could use it to implement
> completion-ignored-extensions and we could use it here to prefer
> a local varname.

An approach that would work for completion-at-point, but wouldn't touch 
compan-capf? Sounds promising, though out of scope for this bug.

> We don't want to do those checks all the time, so it should probably be
> done only once when we combine the two tables (if possible)

`completion-metadata' requires both input string and a predicate (I 
guess, for maximum flexibility of functional completion tables). Pass it 
an empty string and a nil?

And completion boundaries requires suffix as an input. I guess we could 
see if the table supports boundaries at all by passing an empty string, 
and if so, raise an error (ignoring the possibility that the table 
supports only a certain set of suffixes). In that case, we'd be breaking 
support for merging tables that define boundaries, but are known by the 
caller to return identical ones. Which could be useful, I guess.

> or not at all.

Sounds good to me.

> Sounds like a bug in lisp-completion-at-point.

See http://debbugs.gnu.org/16646.

>> +      (let ((retvals (mapcar (lambda (table)
>> +                               (try-completion string table pred))
>> +                             tables))
>> +            (prelim (try-completion string retvals pred)))
>
> try-completion's behavior when passed a list mixing strings and symbols
> is not really defined.  So this second call to `try-completion' relies
> on largely undocumented behavior.  Two solutions: either you add
> a comment about what behavior you assume, or you change the code to
> avoid this problem.
>
> E.g. change retvals with (delq nil ...) and replace t with `string'.

Sure, thanks for catching this.

=== modified file 'lisp/emacs-lisp/lisp.el'
--- lisp/emacs-lisp/lisp.el	2014-01-01 07:43:34 +0000
+++ lisp/emacs-lisp/lisp.el	2014-02-02 01:42:32 +0000
@@ -830,7 +830,7 @@
                  ;; use it to provide a more specific completion table 
in some
                  ;; cases.  E.g. filter out keywords that are not 
understood by
                  ;; the macro/function being called.
-                (list nil (completion-table-in-turn
+                (list nil (completion-table-merge
                             lisp--local-variables-completion-table
                             obarray)       ;Could be anything.
                        :annotation-function

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2014-01-07 23:36:29 +0000
+++ lisp/minibuffer.el	2014-02-05 04:38:11 +0000
@@ -388,11 +388,38 @@
    "Create a completion table that tries each table in TABLES in turn."
    ;; FIXME: the boundaries may come from TABLE1 even when the 
completion list
    ;; is returned by TABLE2 (because TABLE1 returned an empty list).
+  ;; Same potential problem if any of the tables use quoting.
    (lambda (string pred action)
      (completion--some (lambda (table)
                          (complete-with-action action table string pred))
                        tables)))

+(defun completion-table-merge (&rest tables)
+  "Create a completion table that collects completions from all TABLES."
+  ;; FIXME: same caveats as in `completion-table-in-turn', only harder
+  ;; to fix.
+  (lambda (string pred action)
+    (cond
+     ((null action)
+      (let ((retvals (mapcar (lambda (table)
+                               (try-completion string table pred))
+                             tables)))
+        (if (member string retvals)
+            string
+          (try-completion string
+                          (mapcar (lambda (value)
+                                    (if (eq value t) string value))
+                                  (delq nil retvals))
+                          pred))))
+     ((eq action t)
+      (apply #'append (mapcar (lambda (table)
+                                (all-completions string table pred))
+                              tables)))
+     (t
+      (completion--some (lambda (table)
+                          (complete-with-action action table string pred))
+                        tables)))))
+
  (defun completion-table-with-quoting (table unquote requote)
    ;; A difficult part of completion-with-quoting is to map positions 
in the
    ;; quoted string to equivalent positions in the unquoted string and