Subject: bug#12619: completion-at-point and changing buffer

From: Jorgen Schaefer <[email-masked]>
Date: Thu, 11 Oct 2012 20:19:25 +0200

On Wed, 10 Oct 2012 20:37:42 -0400
Stefan Monnier <[email-masked]> wrote:

> > First, in `completion-at-point',
> > `completion-in-region-mode-predicate' is set to a function that
> > compares the old start value with the new using `eq'. This prevents
> > markers to work in this case.  Simply replacing `eq' with `=' means
> > markers work, so the positions used by completion just move
> > together with text changes.
> 
> Sounds OK, tho we should make sure that those values can't be nil
> or some other non-numeric thingy.

True, I think some of those values can be nil in some cases (didn't
happen during my testing, though). Pity that neither `eql' nor `equal'
work on markers.

> > Second, `completion--cache-all-sorted-completions' adds a function
> > to `after-change-functions' that cleans up the cache of sorted
> > completions. I'm not entirely sure why it does so, but my patch adds
> > that function only if not both of the returned positions are
> > markers.
> 
> Hmm...but if the buffer modification happens right in the middle of
> the completion text what should we do?  Should we really ignore
> this modification?

Actually, I have no idea what would happen then and couldn't make a
test case to reproduce it easily. :-)

The patch below flushes the cache if the changed text intersects with
the completion text.

--- lisp/minibuffer.el	2012-10-06 17:29:15 +0000
+++ lisp/minibuffer.el	2012-10-11 17:56:42 +0000
@@ -1048,12 +1048,29 @@
 
 (defun completion--cache-all-sorted-completions (comps)
   (add-hook 'after-change-functions
-               'completion--flush-all-sorted-completions nil t)
+            'completion--flush-all-sorted-completions-after-change nil t)
   (setq completion-all-sorted-completions comps))
 
-(defun completion--flush-all-sorted-completions (&rest _ignore)
+(defun completion--flush-all-sorted-completions-after-change (change-start change-end pre-change-length)
+  (let ((start (nth 1 completion-in-region--data))
+        (end (nth 2 completion-in-region--data)))
+    (when (or
+           ;; We don't even have completion data
+           (not start)
+           (not end)
+           ;; Change completely before our completion, but our
+           ;; completion didn't use markers.
+           (and (<= change-end start)
+                (not (and (markerp start)
+                          (markerp end))))
+           ;; Change overlaps our completion, regardless of markers.
+           (not (or (< change-end start)
+                    (< end change-start))))
+      (completion--flush-all-sorted-completions))))
+
+(defun completion--flush-all-sorted-completions ()
   (remove-hook 'after-change-functions
-               'completion--flush-all-sorted-completions t)
+               'completion--flush-all-sorted-completions-after-change t)
   (setq completion-cycling nil)
   (setq completion-all-sorted-completions nil))
 
@@ -1882,8 +1899,12 @@
       (let* ((completion-extra-properties plist)
              (completion-in-region-mode-predicate
               (lambda ()
-                ;; We're still in the same completion field.
-                (eq (car-safe (funcall hookfun)) start))))
+                (let ((old-start (car-safe (funcall hookfun))))
+                  ;; We're still in the same completion field.
+                  (condition-case error
+                      (= old-start start)
+                    (error
+                     (eq old-start start)))))))
         (completion-in-region start end collection
                               (plist-get plist :predicate))))
      ;; Maybe completion already happened and the function returned t.

(I have no idea at what point patches are considered non-trivial for
copyright reasons, but there should be an assignment "for all future
contributions to Emacs" with the FSF.)

Regards,
	-- Jorgen