Closing CIDER connection removes all local CAPFs, not only CIDER's CAPF

[andreyorst]

Expected behavior

When the CIDER connection is closed, other CAPFs, that were added before it or after it remain.

Actual behavior

CIDER reverts completion-at-point-functions to its global value.

Steps to reproduce the problem

With eglot active in the buffer, observe the value of completion-at-point-functions:

Its value is (eglot-completion-at-point t)
Local in buffer foo.clj; global value is 
(cape-file cape-dabbrev)

Start CIDER, see that completion-at-point-functions now has both cider-complete-at-point and eglot-completion-at-point:

Its value is (cider-complete-at-point eglot-completion-at-point t)
Local in buffer foo.clj; global value is 
(cape-file cape-dabbrev)

Disable CIDER with ,sayonara. The value of completion-at-point-functions becomes:

Its value is (cape-file cape-dabbrev)

Why having multiple CPFS is good for me:

When I have both clojure-lsp and CIDER in a clojure buffer, CIDER's CAPF is preferred, which is good, as I like it more. But if I need to disable CIDER for any reason I'd still like to have completion from Eglot. The same goes for lsp-mode, as it sets CAPFs in a similar way.

On contrary, if I disable eglot it only removes the eglot-completion-at-point from the list, without resetting it to a global value, which is what I'd expect from CIDER. Here's the relevant code:

https://github.com/joaotavora/eglot/blob/83a61f673a0395e9ea3f17f8bf7afd7da37bce03/eglot.el#L1624
https://github.com/joaotavora/eglot/blob/83a61f673a0395e9ea3f17f8bf7afd7da37bce03/eglot.el#L1653

For comparison, here's how CIDER does this:

cider/cider-mode.el

Line 1057 in 398b370

(add-hook 'completion-at-point-functions #'cider-complete-at-point nil t)

cider/cider-mode.el

Line 1076 in 398b370

(mapc #'kill-local-variable '(completion-at-point-functions

This kill-local-variable may make sense for other variables, but for completion-at-point-functions seems like an oversight.

Environment & Version information

CIDER version information

Include here the version string displayed when
CIDER's REPL is launched. Here's an example:

;; CIDER 1.3.0 (Ukraine), nREPL 0.9.0
;; Clojure 1.10.3, Java 11.0.14.1

Lein/Boot version

Leiningen 2.9.6 on Java 11.0.14.1 OpenJDK 64-Bit Server VM

Emacs version

GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4) of 2022-03-23

Operating system

Fedora 35

[vemv]

Sounding reasonable 🙌

Would you be up for a PR?

[bbatsov]

This kill-local-variable may make sense for other variables, but for completion-at-point-functions seems like an oversight.

Yeah, that's true. We should simply use remove-hook here. No idea how no one noticed this for so many years. Better late then never, though. :-)