copilot-emacs / copilot.el

An unofficial Copilot plugin for Emacs.
MIT License
1.79k stars 126 forks source link

Fix compatibility with smartparens-mode #87

Closed JP-Ellis closed 1 year ago

JP-Ellis commented 1 year ago

The simultaneous display of the copilot and smartparens overlay prevented developers from accepting the copilot suggestion, and resulted in either additional indentation or spurious parens being added.

This fixes the issue by adding the appropriate keymaps to sp-pair-overlay-keymap as suggested by @Fuco1 in Fuco1/smartparens#1145.

This fies #86.

ryankhart commented 1 year ago

I haven't used electric-pair yet, but that also seems to break copilot according to that issue #86. I think that it is not enabled by default in Doom Emacs like smartparens is, but it might not hurt anything to include another set of keymaps for that mode as well in the config.

JP-Ellis commented 1 year ago

I wouldn't opt of adding another configuration, as that quickly results in clutter. I would however mention in the comment that if someone is using electric-pair instead of smartparens, they should replace sp-pair-overlay-keymap with whatever the keymap is for electric-pair. I do not kow what that key map is though; if someone lets me know, I'll happily update the PR to make reference to it.

ryankhart commented 1 year ago

Ok, I just did some testing for issue #86 by disabling smartparens and enabling electric-pair, restarting my Emacs, not just a doom/reload, and electric-pair and copilot seem to be already compatible as is, according to my potentially surface level testing.

So, unless I missed something the changes in this PR should be good to go.

LemonBreezes commented 1 year ago

So I haven't done additional testing on why this happened to me with electric-pair in C++ code specifically, but I am sharing here my current config for use with smartparens in Doom Emacs:

;; Model our Copilot interface after Fish completions.
  (map! (:map copilot-completion-map
         "<right>" #'copilot-accept-completion
         "C-f" #'copilot-accept-completion
         "M-<right>" #'copilot-accept-completion-by-word
         "M-f" #'copilot-accept-completion-by-word
         "C-e" #'copilot-accept-completion-by-line
         "<end>" #'copilot-accept-completion-by-line
         "M-n" #'copilot-next-completion
         "M-p" #'copilot-previous-completion)
        (:when (modulep! :config default +smartparens) ;My config disables
                                                       ;Smartparens if the
                                                       ;+smartparens flag is not
                                                       ;set.
         :map sp-pair-overlay-keymap
         "<right>" #'cae-copilot-accept-completion-maybe
         "C-f" #'cae-copilot-accept-completion-maybe
         "M-<right>" #'cae-copilot-accept-completion-by-word-maybe
         "M-f" #'cae-copilot-accept-completion-by-word-maybe
         "C-e" #'cae-copilot-accept-completion-by-line-maybe
         "<end>" #'cae-copilot-accept-completion-by-line-maybe
         "M-n" #'cae-copilot-next-completion-maybe
         "M-p" #'cae-copilot-previous-completion-maybe))
;;;###autoload
(defun cae-copilot-accept-completion-maybe ()
  "Accept the completion at point if there one or execute the
command currently typed."
  (interactive)
  (if (copilot--overlay-visible)
      (copilot-accept-completion)
    (call-interactively
     (or (lookup-key (current-local-map)
                     (this-command-keys))
         (lookup-key (current-global-map)
                     (this-command-keys))))))

;;;###autoload
(defun cae-copilot-accept-completion-by-word-maybe ()
  "Accept the completion at point if there one or execute the
command currently typed."
  (interactive)
  (if (copilot--overlay-visible)
      (copilot-accept-completion-by-word)
    (when-let ((command (or (lookup-key (current-local-map)
                                        (this-command-keys))
                            (lookup-key (current-global-map)
                                        (this-command-keys)))))
      (call-interactively command))))

;;;###autoload
(defun cae-copilot-accept-completion-by-line-maybe ()
  "Accept the completion at point if there one or execute the
command currently typed."
  (interactive)
  (if (copilot--overlay-visible)
      (copilot-accept-completion-by-line)
    (when-let ((command (or (lookup-key (current-local-map)
                                        (this-command-keys))
                            (lookup-key (current-global-map)
                                        (this-command-keys)))))
      (call-interactively command))))

;;;###autoload
(defun cae-copilot-next-completion-maybe ()
  "Accept the completion at point if there one or execute the
command currently typed."
  (interactive)
  (if (copilot--overlay-visible)
      (copilot-next-completion)
    (when-let ((command (or (lookup-key (current-local-map)
                                        (this-command-keys))
                            (lookup-key (current-global-map)
                                        (this-command-keys)))))
      (call-interactively command))))

;;;###autoload
(defun cae-copilot-previous-completion-maybe ()
  "Accept the completion at point if there one or execute the
command currently typed."
  (interactive)
  (if (copilot--overlay-visible)
      (copilot-previous-completion)
    (when-let ((command (or (lookup-key (current-local-map)
                                        (this-command-keys))
                            (lookup-key (current-global-map)
                                        (this-command-keys)))))
      (call-interactively command))))
JP-Ellis commented 1 year ago

I am closing this PR as the recent changes introduced in 1ce3e16 have fixed the issue and make this PR redundant.