joaotavora / eglot

A client for Language Server Protocol servers
GNU General Public License v3.0
2.26k stars 200 forks source link

Issue with completion-at-point when using custom completion-in-region-function #577

Closed ghost closed 3 years ago

ghost commented 3 years ago

I'm currently using a custom completion-in-region-function:

(defun init-ui-completing-read-in-region (start end col &optional pred)
  "Completion in region function utilizing `completing-read'."
  (if (minibufferp) (completion--in-region start end col pred)
    (let* ((init (buffer-substring-no-properties start end))
           (all (completion-all-completions init col pred (length init)))
           (completion (cond
                        ((atom all) nil)
                        ((and (consp all) (atom (cdr all))) (car all))
                        (t (completing-read "Completion: " col pred t init))
              (completing-read "Completion: " col pred t init))))
      (if completion
          (progn (delete-region start end) (insert completion) t)
        (message "No completion") nil))))

This function allows me to use the minibuffer (completing-read) for in-buffer completion. However, when used with eglot, instead of the completion being inserted as expected, more text than expected is removed from the buffer. Since this is hard to convey through text, I've attached a short video showing the issue. This occurs with the latest version from GNU ELPA (1.7).

output

joaotavora commented 3 years ago

This function allows me to use the minibuffer (completing-read) for in-buffer completion.

So does Emacs's vanilla M-x completion-at-point, as far as I know. Have you tried that?

ghost commented 3 years ago

Sorry, accidentally submitted the issue before writing everything down. I'll update it in a second.

ghost commented 3 years ago

So does Emacs's vanilla M-x completion-at-point, as far as I know. Have you tried that?

It doesn't. The vanilla completion-at-point shows a *Completions* buffer, but doesn't allow "interactive" completion using the minibuffer. This function uses completing-read to choose the completion candidate.

joaotavora commented 3 years ago

It doesn't.

You're right. I was thinking of something else.

Anyway, you're probably not following the protocol correctly/entirely. For example I don't see you running the completion table's :exit-function, like company-mode and Emacs's completion--in-region do.

joaotavora commented 3 years ago

I think you ought to ask on help-gnu-emacs or emacs-devel for a good way to design this function. It'd make a good addition to Emacs (if it doesn't exist already, which I find surprising). I don't think this is an Eglot problem, and your function in its current state will break with any completion table that is sufficiently complex.

joaotavora commented 3 years ago

(i closed this, but it doesn't mean we can't keep discussing)

ghost commented 3 years ago

I'll be honest, this function is based of a function I've found somewhere a while ago and tweaked to fix some edge cases I've encountered over time, since I couldn't find any information regarding defining a new completion-in-region-function (except for completion-in-region's docstring). I assumed the issue was with Eglot since I haven't encountered issues with this function in a while. However, looking at completion--in-region, I now see my function is probably too simple to handle all cases correctly. Guess I'll go back to tweaking.

joaotavora commented 3 years ago

Guess I'll go back to tweaking.

I once wrote a completion in region function, I think. https://github.com/joaotavora/sly/blob/613f597ecf72eb5719d4e13a4bfdbeb91373bf09/lib/sly-completion.el#L379

Maybe all you have to do is use choose-completion-string instead of bare delete-region and insert and you'll onto the good stuff of completion--in-region. :shrug:

ghost commented 3 years ago

So I've done some more investigating, and I think that there may actually be some issue on Eglot's side after all. I attempted to re-implement the function using try-completion and choose-completion-string, and while in all the usual cases it worked as expected, it claimed no completions were available in Eglot buffers (I'll add the new function to the end of this comment). Using Edebug, I've found that while all-completions returns a list of completions as expected, try-completions returns nil (when attempting to complete in an Eglot C buffer). In addition, even when ignoring the output of try-completion, it seems that choosing a completion with completing-read inserts the completion (and deletes some unrelated characters) before choose-completion-string is called (this does not occur when completing in other buffers). When choose-completion-string is called with the value returned from completing-read, it inserts the completion again (as expected). I'm not really sure what's going on here. Here's the function:

(defun init-ui-completing-read-in-region (start end col &optional pred)
  "Completion in region function utilizing `completing-read'."
  (if (minibufferp) (completion--in-region start end col pred)
    (let* ((buffer (current-buffer))
           (pos (list start end))
           (init (buffer-substring-no-properties start end))
           (all (all-completions init col pred))
           (try (try-completion init col pred)))
      (setq this-command #'completion-at-point)
      (cond ((or (eq try t) ; Sole completion.
                 (and (= (length all) 1)
                      (string= (car all) try)))
             (choose-completion-string (car all) buffer pos)
             t)
            ;; Commented out since try-completion erroneously returns nil.
            ;; ((and (null try) ; No completions.
            ;;       (> (length init) 0))
            ;;  (message "No completion")
            ;;  nil)
            (t ; Some completions.
             (choose-completion-string
              (completing-read "Completion: " col pred t init) buffer pos)
             t)))))

It might be worth noting I've only tested this with ccls as my language server.

joaotavora commented 3 years ago

Ok, I'll have a look

On Wed, Dec 23, 2020, 12:14 dsemy notifications@github.com wrote:

So I've done some more investigating, and I think that there may actually be some issue on Eglot's side after all. I attempted to re-implement the function using try-completion and choose-completion-string, and while in all the usual cases it worked as expected, it claimed no completions were available in Eglot buffers (I'll add the new function to the end of this comment). Using Edebug, I've found that while all-completions returns a list of completions as expected, try-completions returns nil (when attempting to complete in an Eglot C buffer). In addition, even when ignoring the output of try-completion, it seems that choosing a completion with completing-read inserts the completion (and deletes some unrelated characters) before choose-completion-string is called (this does not occur when completing in other buffers). When choose-completion-string is called with the value returned from completing-read, it inserts the completion again (as expected). I'm not really sure what's going on here. Here's the function:

(defun init-ui-completing-read-in-region (start end col &optional pred) "Completion in region function utilizing `completing-read'." (if (minibufferp) (completion--in-region start end col pred) (let* ((buffer (current-buffer)) (pos (list start end)) (init (buffer-substring-no-properties start end)) (all (all-completions init col pred)) (try (try-completion init col pred))) (setq this-command #'completion-at-point) (cond ((or (eq try t) ; Sole completion. (and (= (length all) 1) (string= (car all) try))) (choose-completion-string (car all) buffer pos) t) ;; Commented out since try-completion erroneously returns nil. ;; ((and (null try) ; No completions. ;; (> (length init) 0)) ;; (message "No completion") ;; nil) (t ; Some completions. (choose-completion-string (completing-read "Completion: " col pred t init) buffer pos) t)))))

It might be worth noting I've only tested this with ccls as my language server.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/joaotavora/eglot/issues/577#issuecomment-750242541, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC6PQ5YRP2CPAIPDGOYHRLSWHNK3ANCNFSM4VGCMQHQ .

ghost commented 3 years ago

In addition, even when ignoring the output of try-completion, it seems that choosing a completion with completing-read inserts the completion (and deletes some unrelated characters) before choose-completion-string is called (this does not occur when completing in other buffers). When choose-completion-string is called with the value returned from completing-read, it inserts the completion again (as expected).

I figured out this is caused by the exit function of the completion table. completion--done (called by various functions) calls the exit function of the completion table if it has one. It seems the exit function used by Eglot attempts to insert the completion itself (and fails to do so correctly in this case). I haven't had time yet to look at the implementation on Eglot's side, but I thought you should know in case something isn't behaving properly. I still haven't figured out why try-completion reports no completions.

Edit: I forgot, disabling calling the exit function in completion--done caused the completion to be inserted as expected (effectively fixing the issue).