From 72f1ffcf2fd81c2a847b312f477cfc29c1b57bd3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Mon, 20 Jan 2025 17:23:08 +0000 Subject: [PATCH] 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) --- lisp/progmodes/eglot.el | 22 +++----- test/lisp/progmodes/eglot-tests.el | 86 ++++++++++++++---------------- 2 files changed, 48 insertions(+), 60 deletions(-) diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el index b8ce0f9811c..d2b175d3b9c 100644 --- 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 dd2f4428366..10e8fb1d41b 100644 --- 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 () -- 2.39.5