Package: emacs;
Reported by: Juri Linkov <juri <at> linkov.net>
Date: Sun, 13 Mar 2022 18:14:02 UTC
Severity: normal
Fixed in version 29.0.50
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Juri Linkov <juri <at> linkov.net> To: Philip Kaludercic <philipk <at> posteo.net> Cc: 54374 <at> debbugs.gnu.org Subject: bug#54374: 29.0.50; previous-completion fails at beginning of completions buffer Date: Tue, 24 May 2022 22:12:27 +0300
[Message part 1 (text/plain, inline)]
>>> 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.
[first-completion.patch (text/x-diff, inline)]
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."
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.