corfu-popupinfo error with newly released eglot-1.14

[mpenet]

Hi,

eglot-1.14 was released yesterday, I gave it a try today and corfu-popupinfo now returns an error instead of showing the docs.
I confirmed it's linked to eglot+corfu-popupinfo, if I revert eglot everything is fine, same if I disable corfu-popupinfo. It also works fine on non-eglot enabled buffers.

The error:

Error running timer ‘corfu-popupinfo--show’: (buffer-read-only #)

recent eglot commits: https://github.com/emacs-mirror/emacs/commits/master/lisp/progmodes/eglot.el

My setup:

GNU Emacs 29.0.60 (build 1, aarch64-apple-darwin22.1.0, NS appkit-2299.00 Version 13.0.1 (Build 22A400)) of 2022-12-07

(use-package corfu
  :straight (:files (:defaults "extensions/*"))
  :custom
  (corfu-cycle t)                ;; Enable cycling for `corfu-next/previous'
  (corfu-auto t)                 ;; Enable auto completion
  (corfu-quit-at-boundary t)     ;; Automatically quit at word boundary
  (corfu-quit-no-match t)        ;; Automatically quit if there is no match
  (corfu-preselect-first nil)    ;; Disable candidate preselection
  (corfu-scroll-margin 5)        ;; Use scroll margin
  :hook ((corfu-mode . corfu-popupinfo-mode))
  :bind
  (:map corfu-map
        ("TAB" . corfu-next)
        ([tab] . corfu-next)
        ("<C-return>" . corfu-insert))
  :init
  (global-corfu-mode))

[minad]

Can you please provide a stack trace?

[mpenet]

oddly enough I don't get one, even with debug-on-error enabled.

[Icy-Thought]

Thought I had done something wrong with my config and was experiencing this issue alone. You could also strip away most configs and retain the popupinfo setting + try rustic mode for the error to show up. No stack trace though...

[mpenet]

I also tried https://github.com/minad/corfu#debugging-corfu but still no stack trace.
It happens on any buffer with eglot enabled for me.

[minad]

I cannot reproduce this issue on Emacs 29.0.60 with the newest Corfu and Eglot 1.14 using Clangd. Please provide a more detailed recipe or try to narrow the issue more precisely. This might not be a Corfu issue since Corfu has worked well before, and it still works in my test.

[mpenet]

I will try to get a full repro tomorrow. One of my tests was with eglot and clojure-lsp. I didn't upgrade clojure-lsp, only eglot and corfu.

[minad]

I will try to get a full repro tomorrow. One of my tests was with eglot and clojure-lsp. I didn't upgrade clojure-lsp, only eglot and corfu.

Very good. But please note that I cannot test arbitrary servers, since this exceeds my bandwidth. My suspicion is that the issue might have something to do with the Markdown conversion. I've seen there have been some recent changes related to that. I believe @jdtsmith contributed something there. Maybe he has some more insight regarding this problem.

[mpenet]

That should be easy to test, iirc it's possible to turn this off in eglot. I will let you know.

[jdtsmith]

The only markdown change I was involved with was the addition of eglot-prefer-plaintext. If set to non-nil, eglot will request plain text from the server, since some servers send badly mangled markdown (which "works fine" with VSCode hence no luck fixing). You could try toggling this variable, but I'd be very surprised if that changes anything.

[mpenet]

I got a somewhat contained repro here: https://gist.github.com/mpenet/bdd591bd005f721b9abdc8953493cf24

The comment here https://gist.github.com/mpenet/bdd591bd005f721b9abdc8953493cf24?permalink_comment_id=4527589#gistcomment-4527589 tells what to do to get it running.

[minad]

@mpenet Thanks. This goes a bit beyond what I am able to debug with reasonable effort due the various dependencies.

[mpenet]

Actually one of the dependencies is not needed I think, only clojure-lsp should be necessary (it’s a native binary)

[jdtsmith]

The named call will arrive via a timer, not a post-command-hook, so maybe you should debug corfu-popup-info--show? First try just C-M-x after that function to make it source compiled and ensure debug-on-error=t. If that doesn't result in a traceback when you cause the error, you could try edebug on the function (C-u C-M-x) then reproduce the issue and step through to see what is amiss. Maybe eglot is accidentally setting the " corfu-popupinfo" hidden buffer to read-only?

[Icy-Thought]

Also, I managed to reproduce the issue only with the following config:

Requirements:

• cargo & rust-analyzer

(package-initialize)

;; Tab bindings are not necessary for reproducing the error..
(use-package corfu
  :bind (:map corfu-map
              ("TAB" . corfu-next)
              ([tab] . corfu-next)
              ("S-TAB" . corfu-previous)
              ([backtab] . corfu-previous))
  :init (global-corfu-mode)
  :custom (corfu-auto t))

(use-package corfu-popupinfo
  :ensure nil
  :hook (corfu-mode . corfu-popupinfo-mode)
  :custom (corfu-popupinfo-delay '(0.5 . 0.2)))

(use-package rustic
  :mode ("\\.rs$" . rustic-mode)
  :custom (rustic-lsp-client 'eglot))

[Icy-Thought]

@jdtsmith I tried executing the following keybinding C-M-x, but it returns undefined. So we have to wait for mpenet to help us figure out the cause.

[mpenet]

with edebug inside corfu-popupinfo--show at the cancel-timer call I don't get a stacktrace/error. And the read-only buffer error I get without debuging doesn't show when instrumented.

 Edebug: when
when
Go...

Result: [t 25646 28666 105993 nil corfu-popupinfo--show (#("cond" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item #1=(:label #("cond" 0 1 (eglot--lsp-item #1#)) :kind 3 :detail "clojure.core/cond" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cond" :ns "clojure.core")]]))) 1 4 (face completions-common-part))) nil 0 nil]

Result: [t 25646 28666 105993 nil corfu-popupinfo--show (#("cond" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item #1=(:label #("cond" 0 1 (eglot--lsp-item #1#)) :kind 3 :detail "clojure.core/cond" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cond" :ns "clojure.core")]]))) 1 4 (face completions-common-part))) nil 0 nil]

Result: nil

Break
Result: nil
Stop

Result: nil

Result: nil

Result: #<frame  0x11ff0ce08>

Result: nil

Result: nil

Result: nil

backtraces

  (cancel-timer corfu-popupinfo--timer)
  (progn (cancel-timer corfu-popupinfo--timer) (setq corfu-popupinfo--timer nil))
  (if corfu-popupinfo--timer (progn (cancel-timer corfu-popupinfo--timer) (setq corfu-popupinfo--timer nil)))
  corfu-popupinfo--show(#("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #8)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cons" :ns "clojure.core")]]))) 1 4 (face completions-common-part)))
  apply(corfu-popupinfo--show #("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #9)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cons" :ns "clojure.core")]]))) 1 4 (face completions-common-part)))
  timer-event-handler([t 25646 28887 230354 nil corfu-popupinfo--show (#("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #10)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [...]))) 1 4 (face completions-common-part))) nil 0 nil])

  (progn (cancel-timer corfu-popupinfo--timer) (setq corfu-popupinfo--timer nil))
  (if corfu-popupinfo--timer (progn (cancel-timer corfu-popupinfo--timer) (setq corfu-popupinfo--timer nil)))
  corfu-popupinfo--show(#("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #8)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cons" :ns "clojure.core")]]))) 1 4 (face completions-common-part)))
  apply(corfu-popupinfo--show #("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #9)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [["documentation" (:uri "file:///clojure.core.clj" :name "cons" :ns "clojure.core")]]))) 1 4 (face completions-common-part)))
  timer-event-handler([t 25646 28887 230354 nil corfu-popupinfo--show (#("cons" 0 1 (completion-score 1.0 face completions-common-part eglot--lsp-item (:label #("cons" 0 1 (eglot--lsp-item #10)) :kind 3 :detail "clojure.core/cons" :data (:unresolved [...]))) 1 4 (face completions-common-part))) nil 0 nil])

I am not very familiar with edebug so I might miss the obvious.

corfu-info-documentation doesn't work anymore as well

Buffer is read-only: #<killed buffer>

[mpenet]

fyi I opened an issue on eglot to see if they have some insight on what could be causing this.

joaotavora/eglot#1202

[jdtsmith]

Just tried 1.14 here and no issue with popupinfo (with pyright).

And the read-only buffer error I get without debuging doesn't show when instrumented.

Hmmm. And once you C-M-x the error returns? You can test my theory by adding an inhibit-read-only in corfu-popupinfo--show and recompiling it:

(let* ((cand-changed
        (not (and (corfu-popupinfo--visible-p)
                  (equal candidate corfu-popupinfo--candidate))))
       (new-coords (frame-edges corfu--frame 'inner-edges))
       (coords-changed (not (equal new-coords corfu-popupinfo--coordinates)))
       (inhibit-read-only t))
...

[mpenet]

There's a more minimal repro in the eglot discussion joaotavora/eglot#1202 (reply in thread)
It does show with clojure-lsp for sure.
I managed to repro on a coworkers laptop as well and went as far as re-installing emacs on mine and double checked again. I get a suspiciously similar error when trying with company (+company-quickhelp) with the same lsp server.
It would seem it's an eglot bug, or maybe something related to the response of the clojure-lsp server for these candidates. I am a bit out of my depth on this one

[mpenet]

Hmmm. And once you C-M-x the error returns? You can test my theory by adding an inhibit-read-only in corfu-popupinfo--show and recompiling it:

The problem indeed goes away with this change.

[jdtsmith]

Hmmm. And once you C-M-x the error returns? You can test my theory by adding an inhibit-read-only in corfu-popupinfo--show and recompiling it:

The problem indeed goes away with this change.

So that would indicate that for whatever reason, the latest eglot accidentally sets the hidden popupinfo buffer to read-only at some point (but only for your specific LSP server). It probably wouldn't be harmful for corfu to include a defensive shield of this sort, but it's more of a bandaid than a cure.