bug#54374: 29.0.50; previous-completion fails at beginning of completions buffer

[Juri Linkov]

>>> Thanks, this fixed the issue.  And I noticed another one: at the
>>> beginning of the buffer previous-completion wraps to the end of the buffer,
>>> not to the beginning of the last completion.
>>>
>>> Also when point at the last completion, next-completion
>>> jumps to the end of the buffer instead of wrapping to the
>>> first completion.
>>
>> I cannot replicate this issue in every case, but only if the last option
>> has completion annotation.  It should be fixable by checking for these
>> kinds of edge-cases, but it might also be better to reconsider if
>> next-completion should be using `mouse-face' for navigation, or if a
>> special text property might be more elegant?
>
> I agree that `mouse-face' is unsuitable for detecting the completion
> boundaries, and it causes other problems: typing RET on the completion prefix
> or on the completion suffix/annotation causes the error:
>
>   Debugger entered--Lisp error: (error "No completion here")
>     error("No completion here")
>     choose-completion(13 nil)
>     funcall-interactively(choose-completion 13 nil)
>     command-execute(choose-completion)
>
> So a special text property could also help to detect the completion for RET,
> if there are no such text properties already.  But I see there is the
> text property 'completion--string'.  So maybe it could be extended to cover
> prefix and suffix/annotation as well.

Here is the patch that fixes 4 bugs: 2 bugs described above,
and also bug#55289 and bug#55430.

diff --git a/lisp/ido.el b/lisp/ido.el
index e5717d6e53..73cd163d46 100644
--- a/lisp/ido.el
+++ b/lisp/ido.el
@@ -3939,7 +3939,7 @@ ido-switch-to-completions
       ;; In the new buffer, go to the first completion.
       ;; FIXME: Perhaps this should be done in `ido-completion-help'.
       (when (bobp)
-       (next-completion 1)))))
+       (first-completion)))))
 
 (defun ido-completion-auto-help ()
   "Call `ido-completion-help' if `completion-auto-help' is non-nil."
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 8287007d32..8948d16949 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2070,11 +2151,11 @@ completion--insert
       (when prefix
         (let ((beg (point))
               (end (progn (insert prefix) (point))))
-          (put-text-property beg end 'mouse-face nil)))
+          (add-text-properties beg end `(mouse-face nil completion--string 
,(car str)))))
       (completion--insert (car str) group-fun)
       (let ((beg (point))
             (end (progn (insert suffix) (point))))
-        (put-text-property beg end 'mouse-face nil)
+        (add-text-properties beg end `(mouse-face nil completion--string ,(car 
str)))
         ;; Put the predefined face only when suffix
         ;; is added via annotation-function without prefix,
         ;; and when the caller doesn't use own face.
diff --git a/lisp/simple.el b/lisp/simple.el
index 1efd900030..75b548aea6 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9523,6 +9523,20 @@ completion-auto-select
   :version "29.1"
   :group 'completion)
 
+(defun first-completion ()
+  (interactive)
+  (goto-char (point-min))
+  (unless (get-text-property (point) 'completion--string)
+    (let ((next (next-single-property-change (point) 'completion--string)))
+      (when next (goto-char next)))))
+
+(defun last-completion ()
+  (interactive)
+  (goto-char (point-max))
+  (unless (get-text-property (point) 'completion--string)
+    (let ((prev (previous-single-property-change (point) 'completion--string)))
+      (when prev (goto-char prev)))))
+
 (defun previous-completion (n)
   "Move to the previous item in the completion list.
 With prefix argument N, move back N items (negative N means move
@@ -9539,14 +9553,6 @@ next-completion
 
 Also see the `completion-wrap-movement' variable."
   (interactive "p")
-  (let ((prev (previous-single-property-change (point) 'mouse-face)))
-    (goto-char (cond
-                ((not prev)
-                 (1- (next-single-property-change (point) 'mouse-face)))
-                ((/= prev (point))
-                 (point))
-                (t prev))))
-
   (let ((beg (point-min))
         (end (point-max))
         (tabcommand (member (this-command-keys) '("\t" [backtab])))
@@ -9554,44 +9560,43 @@ next-completion
     (catch 'bound
       (while (> n 0)
         ;; If in a completion, move to the end of it.
-        (when (get-text-property (point) 'mouse-face)
-          (goto-char (next-single-property-change (point) 'mouse-face nil 
end)))
+        (when (get-text-property (point) 'completion--string)
+          (goto-char (next-single-property-change (point) 'completion--string 
nil end)))
         ;; If at the last completion option, wrap or skip to the
-        ;; minibuffer, if requested. We can't use (eobp) because some
-        ;; extra text may be after the last candidate: ex: when
-        ;; completion-detailed
-        (setq prop (next-single-property-change (point) 'mouse-face nil end))
+        ;; minibuffer, if requested.
+        (setq prop (next-single-property-change (point) 'completion--string 
nil end))
         (when (and completion-wrap-movement (eq end prop))
           (if (and completion-auto-select tabcommand)
               (throw 'bound nil)
             (goto-char (point-min))))
         ;; Move to start of next one.
-        (unless (get-text-property (point) 'mouse-face)
-          (goto-char (next-single-property-change (point) 'mouse-face nil 
end)))
+        (unless (get-text-property (point) 'completion--string)
+          (goto-char (next-single-property-change (point) 'completion--string 
nil end)))
         (setq n (1- n)))
 
-      (while (and (< n 0) (not (bobp)))
-        (setq prop (get-text-property (1- (point)) 'mouse-face))
+      (while (< n 0)
+        (unless (bobp)
+          (setq prop (get-text-property (1- (point)) 'completion--string)))
         ;; If in a completion, move to the start of it.
-        (when (and prop (eq prop (get-text-property (point) 'mouse-face)))
+        (when (and prop (eq prop (get-text-property (point) 
'completion--string)))
           (goto-char (previous-single-property-change
-                      (point) 'mouse-face nil beg)))
+                      (point) 'completion--string nil beg)))
         ;; Move to end of the previous completion.
-        (unless (or (bobp) (get-text-property (1- (point)) 'mouse-face))
+        (unless (or (bobp) (get-text-property (1- (point)) 
'completion--string))
           (goto-char (previous-single-property-change
-                      (point) 'mouse-face nil beg)))
+                      (point) 'completion--string nil beg)))
         ;; If at the first completion option, wrap or skip to the
         ;; minibuffer, if requested.
-        (setq prop (previous-single-property-change (point) 'mouse-face nil 
beg))
+        (setq prop (previous-single-property-change (point) 
'completion--string nil beg))
         (when (and completion-wrap-movement (eq beg prop))
           (if (and completion-auto-select tabcommand)
               (progn
-                (goto-char (next-single-property-change (point) 'mouse-face 
nil end))
+                (goto-char (next-single-property-change (point) 
'completion--string nil end))
                 (throw 'bound nil))
             (goto-char (point-max))))
         ;; Move to the start of that one.
         (goto-char (previous-single-property-change
-                    (point) 'mouse-face nil beg))
+                    (point) 'completion--string nil beg))
         (setq n (1+ n))))
     (when (/= 0 n)
       (switch-to-minibuffer))))
@@ -9620,13 +9625,14 @@ choose-completion
              (goto-char (posn-point (event-start event)))
              (let (beg)
                (cond
-                ((and (not (eobp)) (get-text-property (point) 'mouse-face))
+                ((and (not (eobp)) (get-text-property (point) 
'completion--string))
                  (setq beg (1+ (point))))
                 ((and (not (bobp))
-                      (get-text-property (1- (point)) 'mouse-face))
+                      (get-text-property (1- (point)) 'completion--string))
                  (setq beg (point)))
                 (t (error "No completion here")))
-               (setq beg (previous-single-property-change beg 'mouse-face))
+               (setq beg (or (previous-single-property-change beg 
'completion--string)
+                             beg))
                (substring-no-properties
                 (get-text-property beg 'completion--string))))))
 
@@ -9832,8 +9838,8 @@ switch-to-completions
        ((and (memq this-command '(completion-at-point minibuffer-complete))
              (equal (this-command-keys) [backtab]))
         (goto-char (point-max))
-        (previous-completion 1))
-       (t (next-completion 1))))))
+        (last-completion))
+       (t (first-completion))))))
 
 (defun read-expression-switch-to-completions ()
   "Select the completion list window while reading an expression."