minad / cape

🦸cape.el - Completion At Point Extensions
GNU General Public License v3.0
608 stars 22 forks source link

Let cape-super-capf run without loading cape #32

Closed daanturo closed 2 years ago

daanturo commented 2 years ago

In README's example, I think it's implied that cape-super-capf is used a hook, like Corfu's wiki

 (defun my/ignore-elisp-keywords (cand)
   (or (not (keywordp cand))
    (eq (char-after (car completion-in-region--data)) ?:)))

 (defun my/setup-elisp ()
   (setq-local completion-at-point-functions
        `(,(cape-super-capf
            (cape-capf-predicate
             #'elisp-completion-at-point
             #'my/ignore-elisp-keywords)
            #'cape-dabbrev)
          cape-file)
        cape-dabbrev-min-length 5))
 (add-hook 'emacs-lisp-mode-hook #'my/setup-elisp)

Then cape is eagerly loaded when emacs-lisp-mode is used for the first time but it's better to lazily load it once completion happens. So I think we should dump cape-super-capf to the autoload file.


;;;###autoload
(progn
 (defun cape-super-capf (&rest capfs)
...))))))))
minad commented 2 years ago

No. Dumping large code blocks to the autoload file is generally a very bad idea. One could consider making it a macro instead. But there is no actual advantage here since the elisp setup function will only be called when creating an elisp buffer.

EDIT: I guess when calling something a "very bad idea" one should elaborate. :smile:

(defun my-super-capf ()
   (fset #'my-super-capf (cape-super-capf ...))
   (my-super-capf))
(setq completion-at-point-functions (list #'my-super-capf))

But I don't think loading cape a little bit earlier is an issue at all here. It is okay to load it as soon as an elisp buffer is created. You are probably loading much heavier libraries at this point, e.g., flymake.

daanturo commented 2 years ago

About not enlargin the autoload file, can we shrink cape-super-capf and delegate its work to another function?

My idea: rename current cape-super-capf to cape--super-capf and define:

;;;###autoload
(progn
  (defun cape-super-capf (&rest capfs)
    "Merge CAPFS and return new Capf which includes all candidates."
    (let ((merged-capf (gensym "cape-super-capf--")))
      (fset merged-capf (lambda ()
                          (fset merged-capf (apply #'cape--super-capf capfs))
                          (funcall merged-capf)))
      merged-capf)))

Or without modifying cape-super-capf, just add:

;;;###autoload
(progn
  (defun cape-lazy-super-capf (&rest capfs)
    "Like `cape-super-capf' but doesn't load `cape' when called.
"
    (let ((merged-capf (gensym "cape-super-capf--")))
      (fset merged-capf (lambda ()
                          (fset merged-capf (apply #'cape-super-capf capfs))
                          (funcall merged-capf)))
      merged-capf)))

A little more complex version that allow passing the CAPF name.

;;;###autoload
(progn
  (defun cape-lazy-super-capf (capfs &optional super-capf-symbol)
    "Like `cape-super-capf' but doesn't load `cape' when called.
With non-nil SUPER-CAPF-NAME, assign the merged capf function to
it and only define once."
    (let ((merged-capf (or super-capf-symbol (gensym "cape-super-capf--"))))
      (unless (fboundp merged-capf)
        (fset merged-capf (lambda ()
                            (fset merged-capf (apply #'cape-super-capf capfs))
                            (funcall merged-capf))))
      merged-capf)))