* bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview
@ 2024-02-01 17:06 Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-02 12:54 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 3+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-01 17:06 UTC (permalink / raw)
To: 68875
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-mid-symbol-updating-cycling-completion-preview.patch --]
[-- Type: text/x-patch, Size: 5112 bytes --]
From e64361b0fbd78bb48b859658f9d0c1bb36916f50 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Thu, 1 Feb 2024 12:30:24 +0100
Subject: [PATCH] ; 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.
---
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..d7d3e9bf9f7 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) (completion-preview--get 'completion-preview-end)))
(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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview
2024-02-01 17:06 bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-02 12:54 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-21 16:59 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 3+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-02 12:54 UTC (permalink / raw)
To: 68875
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
> 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Fix-mid-symbol-updating-cycling-completion-previe.patch --]
[-- Type: text/x-patch, Size: 5122 bytes --]
From dd7e8339ec705ebc00ad9a839373d3b668fd39ed Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview
2024-02-02 12:54 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-21 16:59 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 3+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-21 16:59 UTC (permalink / raw)
To: 68875
close 68875 30.1
quit
Eshel Yaron <me@eshelyaron.com> writes:
>> 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.
Pushed to master, and closing the bug.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-21 16:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 17:06 bug#68875: [PATCH] ; Fix mid-symbol updating/cycling completion preview Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-02 12:54 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-21 16:59 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).