Subject: Re: bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview

From: Eshel Yaron <[email-masked]>
Date: Fri, 02 Feb 2024 13:54:13 +0100

> Tags: patch
>
> The attached patch fixes an issue where the completion preview overlay
> (of Completion Preview mode) could at certain cases appear in the wrong
> place when point is in the middle of a symbol.
>
> To observe the effect of this patch:
>
> 1. emacs -Q
> 2. In the *scratch* buffer, say M-x completion-preview-mode RET
> 3. Type "defaul-di"
> 4. C-3 C-b to place point before the hyphen
> 5. Type "t"
>    The completion preview overlay appears after "-di", showing "rectory".
>    So far so good.
> 6. M-x completion-preview-next-candidate RET
>    Before this patch, the preview now shows "-directory", thus repeating
>    the existing suffix "-di".  With this patch, the preview shows just
>    "rectory", as expected.

Sorry, that patch contained a thinko (which was caught by one of the
existing tests, fortunately).  I'm attaching an updated patch below.

Thanks,

Eshel

>From dd7e8339ec705ebc00ad9a839373d3b668fd39ed Mon Sep 17 00:00:00 2001
From: Eshel Yaron <[email-masked]>
Date: Thu, 1 Feb 2024 12:30:24 +0100
Subject: [PATCH v2] ; Fix mid-symbol updating/cycling completion preview

This fixes an issue where 'completion-preview-next-candidate' would
fail to take into account the part of the symbol that follows
point (the suffix) when point is at the middle of a symbol, as well as
a similar issue in 'completion-preview--show' that would manifest with
slow 'completion-at-point-functions'.

* lisp/completion-preview.el (completion-preview-next-candidate)
(completion-preview--show): Use recorded 'completion-preview-end'
position instead of current point.

* test/lisp/completion-preview-tests.el (completion-preview-mid-symbol-cycle):
New test.  (Bug#68875)
---
 lisp/completion-preview.el            | 24 ++++++++++++------------
 test/lisp/completion-preview-tests.el | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index 6fd60f3c416..e827da43a08 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -302,21 +302,21 @@ completion-preview--show
     ;; never display a stale preview and that the preview doesn't
     ;; flicker, even with slow completion backends.
     (let* ((beg (completion-preview--get 'completion-preview-beg))
+           (end (max (point) (overlay-start completion-preview--overlay)))
            (cands (completion-preview--get 'completion-preview-cands))
            (index (completion-preview--get 'completion-preview-index))
            (cand (nth index cands))
-           (len (length cand))
-           (end (+ beg len))
-           (cur (point))
-           (face (get-text-property 0 'face (completion-preview--get 'after-string))))
-      (if (and (< beg cur end) (string-prefix-p (buffer-substring beg cur) cand))
+           (after (completion-preview--get 'after-string))
+           (face (get-text-property 0 'face after)))
+      (if (and (<= beg (point) end (1- (+ beg (length cand))))
+               (string-prefix-p (buffer-substring beg end) cand))
           ;; The previous preview is still applicable, update it.
           (overlay-put (completion-preview--make-overlay
-                        cur (propertize (substring cand (- cur beg))
+                        end (propertize (substring cand (- end beg))
                                         'face face
                                         'mouse-face 'completion-preview-highlight
                                         'keymap completion-preview--mouse-map))
-                       'completion-preview-end cur)
+                       'completion-preview-end end)
         ;; The previous preview is no longer applicable, hide it.
         (completion-preview-active-mode -1))))
   ;; Run `completion-at-point-functions' to get a new candidate.
@@ -366,16 +366,16 @@ completion-preview-next-candidate
   (interactive "p")
   (when completion-preview-active-mode
     (let* ((beg (completion-preview--get 'completion-preview-beg))
+           (end (completion-preview--get 'completion-preview-end))
            (all (completion-preview--get 'completion-preview-cands))
            (cur (completion-preview--get 'completion-preview-index))
            (len (length all))
            (new (mod (+ cur direction) len))
-           (str (nth new all))
-           (pos (point)))
-      (while (or (<= (+ beg (length str)) pos)
-                 (not (string-prefix-p (buffer-substring beg pos) str)))
+           (str (nth new all)))
+      (while (or (<= (+ beg (length str)) end)
+                 (not (string-prefix-p (buffer-substring beg end) str)))
         (setq new (mod (+ new direction) len) str (nth new all)))
-      (let ((aft (propertize (substring str (- pos beg))
+      (let ((aft (propertize (substring str (- end beg))
                              'face (if (< 1 len)
                                        'completion-preview
                                      'completion-preview-exact)
diff --git a/test/lisp/completion-preview-tests.el b/test/lisp/completion-preview-tests.el
index 190764e9125..5b2c28bd3dd 100644
--- a/test/lisp/completion-preview-tests.el
+++ b/test/lisp/completion-preview-tests.el
@@ -181,4 +181,19 @@ completion-preview-capf-errors
       (completion-preview--post-command))
     (completion-preview-tests--check-preview "barbaz" 'exact)))
 
+(ert-deftest completion-preview-mid-symbol-cycle ()
+  "Test cycling the completion preview with point at the middle of a symbol."
+  (with-temp-buffer
+    (setq-local completion-at-point-functions
+                (list
+                 (completion-preview-tests--capf
+                  '("foobar" "foobaz"))))
+    (insert "fooba")
+    (forward-char -2)
+    (let ((this-command 'self-insert-command))
+      (completion-preview--post-command))
+    (completion-preview-tests--check-preview "r")
+    (completion-preview-next-candidate 1)
+    (completion-preview-tests--check-preview "z")))
+
 ;;; completion-preview-tests.el ends here
-- 
2.42.0