From: Juri Linkov <juri@linkov.net>
To: Philip Kaludercic <philipk@posteo.net>
Cc: 54374@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 [thread overview]
Message-ID: <861qwiu4rd.fsf@mail.linkov.net> (raw)
In-Reply-To: <864k3tu4pp.fsf@mail.linkov.net> (Juri Linkov's message of "Sat, 19 Mar 2022 21:13:14 +0200")
[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]
>>> 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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: first-completion.patch --]
[-- Type: text/x-diff, Size: 7490 bytes --]
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."
next prev parent reply other threads:[~2022-05-24 19:12 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-13 17:39 bug#54374: 29.0.50; previous-completion fails at beginning of completions buffer Juri Linkov
2022-03-13 20:39 ` Philip Kaludercic
2022-03-14 3:22 ` Eli Zaretskii
2022-03-14 8:20 ` Philip Kaludercic
2022-03-14 12:59 ` Eli Zaretskii
2022-03-14 13:34 ` Philip Kaludercic
2022-03-14 8:35 ` Andreas Schwab
2022-03-14 8:30 ` Juri Linkov
2022-03-14 14:12 ` Philip Kaludercic
2022-03-14 18:24 ` Juri Linkov
2022-03-18 21:49 ` Philip Kaludercic
2022-03-19 19:13 ` Juri Linkov
2022-05-24 19:12 ` Juri Linkov [this message]
2022-05-26 16:26 ` Juri Linkov
2022-05-27 16:13 ` Juri Linkov
2022-03-24 18:11 ` Juri Linkov
2022-03-14 15:21 ` bug#54374: [External] : " Drew Adams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=861qwiu4rd.fsf@mail.linkov.net \
--to=juri@linkov.net \
--cc=54374@debbugs.gnu.org \
--cc=philipk@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.