bug#71716: [PATCH] Add new completion-preview-insert-{word, sexp} commands

[Jules Tamagnan <jtamagnan@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]


Hi Eshel,

Eshel Yaron <me@eshelyaron.com> writes:

> Thanks for the feature request and for the patch.

Thanks for the incredibly quick feedback and thoughtful comments. I
really appreciate it. I've attached a patch addressing the first comment
and will post an additional patch for a possible fix to the second
comment.

> That sounds interesting.  The ELPA package capf-autosuggest.el provided
> a similar feature, IIRC.  I'd like to get a better understanding of the
> use case though: when would you use one of these commands instead of
> completion-preview-complete?

This is a functionality that I got used to when I tried using some other
packages. One issue that I have with completion on the "common" part of
candidates is that oftentimes when using thing functionality in my shell
I have so many candidates from my history that the common part is
relatively useless.

Generally I'd say that I'm so comfortable navigating with forward-word
and forward-sexp that this using M-f and C-M-f for this is second nature
to me. Given that this is also implemented in `capf-autosuggest` and
`github-copilot` I imagine that others might feel the same way.

> 1. AFAICT, unlike completion-preview-insert, these new commands should
>    preserve (the rest of) the completion preview.  So instead of
>    dismissing the preview by disabling completion-preview-active-mode
>    and then relying on the subsequent post-command-hook to recreate the
>    preview, I think these commands should modify (e.g. remove a word
>    from the start of) the after-string property of the preview overlay,
>    and inhibit a subsequent update of the preview, like we do in
>    completion-preview-complete.  That way we avoid recomputing the
>    completion candidates, which may lead to a flicker in this case.

Ahh that is a really good point, thank you.

> 2. The temporary buffer where the motion command is executed has a
>    different major mode than the original buffer, so they might have
>    different notions of words/sexps.

I was thinking about that when implementing this, even further one could
have locally changed the value of `find-word-boundary-function-table`
outside of `subword-mode`. One idea I had thought of was inserting the
complete after-string and performing character deletions until the
suffix was removed but this felt like an even worse solution. Maybe, in
the temporary buffer, we can bind `find-word-boundary-function-table` to
the parent buffer's value. I need to hop on a flight but can implement
this in a third patch.

 - Jules
 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Follow on patch to address comment 1 --]
[-- Type: text/x-patch, Size: 3962 bytes --]

From 1bbcc10c5b23d63dc8454113403c2d834a69d803 Mon Sep 17 00:00:00 2001
From: Jules Tamagnan <jtamagnan@gmail.com>
Date: Sat, 22 Jun 2024 11:40:09 -0700
Subject: [PATCH 2/2] [Cont] Add new completion-preview-insert-{word,sexp}
 commands

---
 lisp/completion-preview.el            | 37 ++++++++++++++++++++-------
 test/lisp/completion-preview-tests.el |  4 +--
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index 3a7fa37afe0..637778caadb 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -456,18 +456,37 @@ completion-preview--insert
              (end (completion-preview--get 'completion-preview-end))
              (efn (plist-get (completion-preview--get 'completion-preview-props)
                              :exit-function))
-             (ful (completion-preview--get 'after-string))
-             (aft (with-temp-buffer
-                      (insert ful)
+             (aft (completion-preview--get 'after-string))
+             (ful (with-temp-buffer
+                      (insert aft)
                       (goto-char (point-min))
                       (funcall action)
-                      (buffer-substring-no-properties (point-min) (point)))))
-        (completion-preview-active-mode -1)
+                      (cons (buffer-substring-no-properties (point-min) (point))
+                            (buffer-substring (point) (point-max)))))
+             (ins (car ful))
+             (suf (cdr ful)))
+        ;; If the completion is a full completion (there is no suffix)
+        ;; deactivate the preview
+        (when (string-empty-p suf)
+          (completion-preview-active-mode -1))
+
+        ;; Insert the new text
         (goto-char end)
-        (insert aft)
-        (when (and (functionp efn) (string= ful aft))
-          ;; If we've inserted a full completion call the exit-function
-          (funcall efn (concat (buffer-substring-no-properties beg end) aft) 'finished)))
+        (insert ins)
+
+        ;; If we are not inserting a full completion update the preview
+        (when (not (string-empty-p suf))
+          (let ((pos (point)))
+            (completion-preview--inhibit-update)
+            (overlay-put (completion-preview--make-overlay
+                          pos (propertize suf
+                                          'mouse-face 'completion-preview-highlight
+                                          'keymap completion-preview--mouse-map))
+                         'completion-preview-end pos)))
+
+        ;; If we've inserted a full completion call the exit-function
+        (when (and (functionp efn) (string-empty-p suf))
+          (funcall efn (concat (buffer-substring-no-properties beg end) ins) 'finished)))
     (user-error "No current completion preview")))
 
 (defun completion-preview-insert ()
diff --git a/test/lisp/completion-preview-tests.el b/test/lisp/completion-preview-tests.el
index dedd135da73..54ba566ad3c 100644
--- a/test/lisp/completion-preview-tests.el
+++ b/test/lisp/completion-preview-tests.el
@@ -325,7 +325,7 @@ completion-preview-insert-word
       (completion-preview-tests--check-preview "bar-1 2" 'completion-preview-common)
       (completion-preview-insert-word)
       (should (string= (buffer-string) "foobar"))
-      (should-not completion-preview--overlay)
+      (completion-preview-tests--check-preview "-1 2" 'completion-preview)
       (should-not exit-fn-called)
       (should-not exit-fn-args))))
 
@@ -347,7 +347,7 @@ completion-preview-insert-sexp
       (completion-preview-tests--check-preview "bar-1 2" 'completion-preview-common)
       (completion-preview-insert-sexp)
       (should (string= (buffer-string) "foobar-1"))
-      (should-not completion-preview--overlay)
+      (completion-preview-tests--check-preview " 2" 'completion-preview)
       (should-not exit-fn-called)
       (should-not exit-fn-args))))
 
-- 
2.45.1