Fix eglot-completion-at-point so it can be used from the minibuffer

[andyleejordan]

I have been playing with a completing-read version of
completion-at-point, and got it just about working, except with
minibuffer-force-complete was called I’d run into errors. Debugged it
to the exit-function, which was written assuming the current buffer
was still the code buffer. The fix was fairly trivial: change the
current buffer to the one from which the minibuffer was called, if
applicable. An alternative considered was to apply this logic in
eglot--current-server-or-lose and eglot--TextDocumentPositionParams,
which is were the errors came from, but that didn’t make sense.

[andyleejordan]

This is where I'm using it.

[andyleejordan]

@joaotavora, did you have a chance to look at this? I can rebase.

[andyleejordan]

Should be up to date now.

[joaotavora]

Hi @andschwa I'm sorry for the delay. I've had a looked at this some days ago, and I remember thinking of an objection that I can't seem to remember now. I'll think a bit harder in the next few days, or just let it go, since it doesn't look bad in theory.

[andyleejordan]

Thanks @joaotavora! I've been using this patch locally (with the capf variant I linked) for a while now and it's working well at least in that scenario.

[andyleejordan]

Hm, I don't remember those tests failing before I rebased 🤔

[andyleejordan]

It looks like these failing tests are unrelated, as they also failed in #515 which only touched the readme:

   FAILED  eglot-eldoc-after-completions
   FAILED  eglot-single-line-eldoc

[joaotavora]

Hi @andschwa , I'm looking at this again. Can you explain how this works, from a user's standpoint? I'm having trouble visualizing its use. Is it that you complete something in the minibuffer but it somehow has an effect in the buffer of the window selected just before you switched to the minibuffer? A small video or .gif animation would be ideal, even if it requires you launching Eglot with more configuration.

Also, can you comment on whether or not you think your patch should be applied to all of Emacs' use of :exit-function?

[andyleejordan]

Absolutely!

So instead of using company-mode, as someone who really likes completing-read (as you know! Especially with fido-mode), I use this function as my completion-in-region-function:

;; Inspirations: oantolin’s `completing-read-in-region’, selectrum’s
;; `selectrum-completion-in-region’, minibuffer’s
;; `completion--do-completion’, and `minibuffer-completion-help’.
;; TBH I can't believe something like this isn't available in Emacs because
;; *completions* is archaic compared to `completing-read`.
(defun completion-in-region+ (start end collection &optional predicate)
  "Completion in region using `completing-read’.
See `completion-in-region’ for START END COLLECTION PREDICATE.
Set `completion-in-region-function’ to use, remember to press ?
to open `*completions*’ buffer (perhaps with annotations)."
  (if (and (minibufferp) (not (string= (minibuffer-prompt) "Eval: ")))
      (unless completion-fail-discreetly
        (ding) (completion--message "Press `?’ for `*completions*’ buffer") nil)
    (let* ((input (buffer-substring-no-properties start end))
           (limit (car (completion-boundaries input collection predicate "")))
           (md (completion--field-metadata start))
           (all (completion-all-completions input collection predicate
                                            (- end start) md))
           (comp (cond
                  ((atom all) nil)
                  ((and (consp all) (atom (cdr all)))
                   (concat (substring input 0 limit) (car all)))
                  (t (completing-read
                      "Completion: " collection predicate t input)))))
      (if (null comp)
          (unless completion-fail-discreetly
            (ding) (completion--message "No completions") nil)
        (delete-region start end)
        (insert (substring-no-properties comp))
        t))))

(setq completion-in-region-function #'completion-in-region+)

Here's it in use when Eglot is broken in this way:

Hitting RET causes that json-rpc error message, which I tracked to icomplete-fido-ret eventually calling completion-complete-and-exit which calls this given exit function.

I exited the second time using M-j which for fido-mode (I don't know why I'm telling you this 😅) calls icomplete-fido-exit which does not end up calling the exit function.

That error message is:

jsonrpc-error: jsonrpc-error: "No current JSON-RPC connection", (jsonrpc-error-code . 32603), (jsonrpc-error-message . "No current JSON-RPC connection")

Which I tracked to two places, eglot--current-server-or-lose and eglot--TextDocumentPositionParams, which both assume that the current buffer is the buffer in which we're doing completions. However, in this edge case, the current buffer is, in fact, the minibuffer. So the fix I propose here is to avoid this all together by letting the exit function operated under the environment it expected in the first place, the current buffer visiting the file, not the minibuffer.

And here's it working with RET with this patch, since the exit function now works as intended with the current buffer being the originating buffer, and not the minibuffer:

For what it's worth, there's probably a better way to fix this, it's just what I came up with. I'm no expert here.

[andyleejordan]

Also, can you comment on whether or not you think your patch should be applied to all of Emacs' use of :exit-function?

I have no idea honestly. I think anything that uses a completing-read replacement for completions would run into this, so perhaps it would make sense for all exit-functions to work like this. I ran into similar issues with lsp-mode (but couldn't figure out how to fix their code).

[joaotavora]

I added a comment and pushed this, presumably fixing your problem.

I think you must now open an Emacs bug report (with M-x report-emacs-bug)

1. Ask for the with-current-buffer call to be included in the completion--done function in Emacs itself. This would obviate the need for Eglot doing so, but it would only do so for Emacs >= 28, so the line does need to be there.

2. Pose this pressing question, which I very much agree with:

;; TBH I can't believe something like this isn't available in Emacs because
;; completions is archaic compared to completing-read.

Please include me (and Stefan Monnier (@monnier)) in that bug report (using the X-Debbugs-CC: header, not the CC: header)

Also, Andrew, I think you must soon sign a copyright assigment to Emacs if you want to keep contributing code. I marked this small contribution as a "Copyright-paperwork-exempt: yes".

[andyleejordan]

I added a comment and pushed this, presumably fixing your problem.

Sweet, thank you! And I will do 1) and 2) 😄

Also, Andrew, I think you must soon sign a copyright assigment to Emacs if you want to keep contributing code. I marked this small contribution as a "Copyright-paperwork-exempt: yes".

I have one signed! However, it's stuck in the FSF copyright office...let me follow up with them again to see if everything is settled 😅