Eglot: try again to fix try-completion logic

Emacs partial completion simply doesn't make sense in LSP.
Attempts to make it make some sense end up failing for one
reason or another.

This commit restores the original intention of the
eglot--dumb-allc and eglot--dumb-tryc functions, which was to be
dumb and pop up a *Completions* buffer (or a company tooltip),
as soon as there is doubt over what the user wants to do.

It also fixes tests, including an expensive Rust test that had
been broken for a long time.

* lisp/progmodes/eglot.el (eglot--dumb-allc): Consider point.
(eglot--dumb-tryc): Make it suitably dumb again.

* test/lisp/progmodes/eglot-tests.el
(eglot--wait-for-rust-analyzer): Wait longer.
(eglot-test-common-prefix-completion): Delete this test.  It's not
the intended behavior.
(eglot--kill-completions-buffer): New helper.
(eglot-test-try-completion-nomatch): Rework test.
(eglot-test-try-completion-inside-symbol): Pops *Completions*
buffer because mustn't partial complete.
(eglot-test-try-completion-inside-symbol-2): Does complete.
(eglot-test-rust-completion-exit-function): Fix long-broken test.

(cherry picked from commit 73f51f3a99ed454fa57e5ebe177df00cada1f11a)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index b8ce0f9811cd84b7db46fc73e20f5d0ae207eba1..d2b175d3b9ca20629e758c2b70cd90fbe610324b 100644 (file)
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -3201,22 +3201,14 @@ for which LSP on-type-formatting should be requested."
                                       nil comp)
            finally (cl-return comp)))
 
-(defun eglot--dumb-allc (pat table pred _point) (funcall table pat pred t))
+(defun eglot--dumb-allc (pat table pred point)
+  (funcall table (substring pat 0 point) pred t))
+
 (defun eglot--dumb-tryc (pat table pred point)
-  (let ((probe (funcall table pat pred nil)))
-    (cond ((eq probe t) t)
-          (probe
-           (if (and (not (equal probe pat))
-                    (cl-every
-                     (lambda (s) (string-prefix-p probe s completion-ignore-case))
-                     (funcall table pat pred t)))
-               (cons probe (length probe))
-             (cons pat point)))
-          (t
-           ;; Match ignoring suffix: if there are any completions for
-           ;; the current prefix at least, keep the current input.
-           (and (funcall table (substring pat 0 point) pred t)
-                (cons pat point))))))
+  (let* ((probe (funcall table (substring pat 0 point) pred t)))
+    (cond ((and probe (null (cdr probe)))
+           (cons (car probe) (length (car probe))))
+          (t (cons pat point)))))
 
 (add-to-list 'completion-category-defaults '(eglot-capf (styles eglot--dumb-flex)))
 (add-to-list 'completion-styles-alist '(eglot--dumb-flex eglot--dumb-tryc eglot--dumb-allc))

diff --git a/test/lisp/progmodes/eglot-tests.el b/test/lisp/progmodes/eglot-tests.el
index dd2f44283669b17f070e795250d113fee7b7379d..10e8fb1d41b28f2d2306cbac632e5fe7094fa642 100644 (file)
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -595,11 +595,12 @@ directory hierarchy."
     (eglot--wait-for (s-notifs 20) (&key method params &allow-other-keys)
       (and
        (string= method "$/progress")
-       "rustAnalyzer/Indexing"
-       (equal params
-              '(:token "rustAnalyzer/Indexing" :value
-                       ;; Could wait for :kind "end" instead, but it's 2 more seconds.
-                       (:kind "begin" :title "Indexing" :cancellable :json-false :percentage 0)))))))
+       (equal (plist-get params :token) "rustAnalyzer/Roots Scanned")
+       (equal (plist-get (plist-get params :value) :kind) "end")))
+    ;; Annoyingly, waiting for that special progress report is still not
+    ;; enough to make sure the server is ready to provide completions,
+    ;; so here's two extra seconds.
+    (sit-for 2)))
 
 (ert-deftest eglot-test-basic-completions ()
   "Test basic autocompletion in a clangd LSP."
@@ -614,20 +615,6 @@ directory hierarchy."
       (message (buffer-string))
       (should (looking-back "fprintf.?")))))
 
-(ert-deftest eglot-test-common-prefix-completion ()
-  "Test completion appending the common prefix."
-  (skip-unless (executable-find "clangd"))
-  (eglot--with-fixture
-      `(("project" . (("coiso.c" .
-                       ,(concat "int foo_bar; int foo_bar_baz;"
-                                "int main() {foo")))))
-    (with-current-buffer
-        (eglot--find-file-noselect "project/coiso.c")
-      (eglot--wait-for-clangd)
-      (goto-char (point-max))
-      (completion-at-point)
-      (should (looking-back "{foo_bar")))))
-
 (ert-deftest eglot-test-non-unique-completions ()
   "Test completion resulting in 'Complete, but not unique'."
   (skip-unless (executable-find "clangd"))
@@ -661,6 +648,10 @@ directory hierarchy."
       (completion-at-point)
       (should (looking-back "foo")))))
 
+(defun eglot--kill-completions-buffer ()
+  (when (buffer-live-p (get-buffer "*Completions*"))
+        (kill-buffer "*Completions*")))
+
 (ert-deftest eglot-test-try-completion-nomatch ()
   "Test completion table with non-matching input, returning nil."
   (skip-unless (executable-find "clangd"))
@@ -670,12 +661,11 @@ directory hierarchy."
     (with-current-buffer
         (eglot--find-file-noselect "project/coiso.c")
       (eglot--wait-for-clangd)
+      (eglot--kill-completions-buffer)
       (goto-char (point-max))
-      (should
-       (null
-        (completion-try-completion
-         "abc"
-         (nth 2 (eglot-completion-at-point)) nil 3))))))
+      (completion-at-point)
+      (should (looking-back "abc"))
+      (should-not (get-buffer "*Completions*")))))
 
 (ert-deftest eglot-test-try-completion-inside-symbol ()
   "Test completion table inside symbol, with only prefix matching."
@@ -684,21 +674,38 @@ directory hierarchy."
       `(("project" . (("coiso.c" .
                        ,(concat
                          "int foobar;"
+                         "int foobarbaz;"
                          "int main() {foo123")))))
     (with-current-buffer
         (eglot--find-file-noselect "project/coiso.c")
       (eglot--wait-for-clangd)
       (goto-char (- (point-max) 3))
-      (when (buffer-live-p "*Completions*")
-        (kill-buffer "*Completions*"))
+      (eglot--kill-completions-buffer)
       (completion-at-point)
       (should (looking-back "foo"))
       (should (looking-at "123"))
-      (should (get-buffer "*Completions*"))
-      )))
+      (should (get-buffer "*Completions*")))))
+
+(ert-deftest eglot-test-try-completion-inside-symbol-2 ()
+  "Test completion table inside symbol, with only prefix matching."
+  (skip-unless (executable-find "clangd"))
+  (eglot--with-fixture
+      `(("project" . (("coiso.c" .
+                       ,(concat
+                         "int foobar;"
+                         "int main() {foo123")))))
+    (with-current-buffer
+        (eglot--find-file-noselect "project/coiso.c")
+      (eglot--wait-for-clangd)
+      (goto-char (- (point-max) 3))
+      (completion-at-point)
+      (should (looking-back "foobar"))
+      (should (looking-at "123")))))
 
 (ert-deftest eglot-test-rust-completion-exit-function ()
-  "Ensure that the rust-analyzer exit function creates the expected contents."
+  "Ensure rust-analyzer exit function creates the expected contents."
+  :tags '(:expensive-test)
+  ;; This originally appeared in github#1339
   (skip-unless (executable-find "rust-analyzer"))
   (skip-unless (executable-find "cargo"))
   (eglot--with-fixture
@@ -708,25 +715,14 @@ directory hierarchy."
     (with-current-buffer
         (eglot--find-file-noselect "cmpl-project/main.rs")
       (should (zerop (shell-command "cargo init")))
-      (eglot--tests-connect)
-      (goto-char (point-min))
       (search-forward "v.count_on")
-      (let ((minibuffer-message-timeout 0)
-            ;; Fail at (ding) if completion fails.
-            (executing-kbd-macro t))
-        (when (buffer-live-p "*Completions*")
-          (kill-buffer "*Completions*"))
-        ;; The design is pretty brittle, we'll need to monitor the
-        ;; language server for changes in behavior.
-        (eglot--wait-for-rust-analyzer)
-        (completion-at-point)
-        (should (looking-back "\\.count_on"))
-        (should (get-buffer "*Completions*"))
-        (minibuffer-next-completion 1)
-        (minibuffer-choose-completion t))
+      (eglot--wait-for-rust-analyzer)
+      (completion-at-point)
       (should
        (equal
-        "fn test() -> i32 { let v: usize = 1; v.count_ones.1234567890;"
+        (if (bound-and-true-p yas-minor-mode)
+            "fn test() -> i32 { let v: usize = 1; v.count_ones().1234567890;"
+          "fn test() -> i32 { let v: usize = 1; v.count_ones.1234567890;")
         (buffer-string))))))
 
 (ert-deftest eglot-test-basic-xref ()