search

minad / corfu

:desert_island: corfu.el - COmpletion in Region FUnction
GNU General Public License v3.0
1.14k stars 43 forks source link

add-hook capf's leave behind `hook--depth-alist entries #133

Closed titaniumbones closed 2 years ago

titaniumbones commented 2 years ago

I strongly doubt this is a corfu issue, but googling led me only to https://github.com/minad/consult/issues/193 so I thought I would ask you here. I am noticing that I cannot fully delete entries from the hook--depth-alist property of completion-at-point-functions added via add-hook with the local flag set. I have several such mode hooks set, now that' I've switched to corfu and am using it for all my completion. So for instance,I have this from tempel: 

 ;; Setup completion at point
  (defun tempel-setup-capf ()
    ;; Add the Tempel Capf to `completion-at-point-functions'.
    ;; The depth is set to -1, such that `tempel-expand' is tried *before* the
    ;; programming mode Capf. If a template name can be completed it takes
    ;; precedence over the programming mode completion. `tempel-expand' only
    ;; triggers on exact matches. Alternatively use `tempel-complete' if you
    ;; want to see all matches, but then Tempel will probably trigger too
    ;; often when you don't expect it.
    (add-hook 'completion-at-point-functions #'tempel-expand -10 t))

  (add-hook 'prog-mode-hook 'tempel-setup-capf)
  (add-hook 'text-mode-hook 'tempel-setup-capf)

After I open a bunch of files (via desktop-session) I get this value for the variable completion-at-point-functions:

(tempel-expand
 forge-topic-completion-at-point
 pcomplete-completions-at-point
 t)

which is great; but the hook--depth-alist properthy is enormous:

'((elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (sh-completion-at-point-function . 0) (comint-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (sh-completion-at-point-function . 0) (comint-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (sh-completion-at-point-function . 0) (comint-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (mwp-lsp-super-capf . -20) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (mwp-lsp-super-capf . -20) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (mwp-lsp-super-capf . -20) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (css-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (latex-complete-data . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (sh-completion-at-point-function . 0) (comint-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (mwp-lsp-super-capf . -20) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . -1) (elisp-completion-at-point . 0) (forge-topic-completion-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (tempel-expand . -10) (pcomplete-completions-at-point . 0) (cape-rfc1345 . 5) (cape-keyword . -5))

the big problem here is that sometimes capf's I have explicitly removed from the hook somehow come bak as zombies and provide ocmpletion. I wonder if htis is also related to the slowedowns I am sometimes seeing.

Is this a bug? Is it the emacs bug you point to in that other issue? Or do I just not understand how the symbol property is supposed to work?

Thanks again for all your help, and Ihope this is ano OK place to ask once again.

minad commented 2 years ago

Thank you for the report. There seems to be something broken in add/remove-hook. I pushed fixes in a few of my projects, where I've observed dangling entries in the alist. Generally it seems to be a good idea to avoid hook priorities. Please let me know if the issue with the alist entries remains if you apply the configuration changes described in the Tempel README.

However the dangling entries should have nothing to do with the zombies:

the big problem here is that sometimes capf's I have explicitly removed from the hook somehow come bak as zombies and provide ocmpletion.

The problem here could be that hooks are for some reason marked as permanently local and then somehow return. I am not sure what causes this, but there have also been an issue on the Emacs tracker if I recall correctly.

minad commented 2 years ago

Also my Consult package is affected by this :(

https://github.com/minad/consult/commit/d0261c0ecdf95ab543ad73d466c450bfedb8b1e7

minad commented 2 years ago

However the issue is really that I don't know how the bug can be fixed, since as soon as we add local hooks with priorities, the list cannot be cleaned up anymore since Emacs cannot know if the hook is still in use in some other buffer. The issue becomes particularly problematic when we dynamically generate symbols as in consult. So it seems to me that using local hooks with priorities is a no-go? cc @monnier

titaniumbones commented 2 years ago

ah shoot. Thank you for the update! But that's a bummer -- really hard to fine tune the system without priorities. Does the fix need to happen in upstream emacs?

minad commented 2 years ago

@titaniumbones In the updated tempel example I manually adjust the list. This is what I recommend. Just configure the capfs explicitly, if you really need this degree of fine-tuning.

(add-hook 'emacs-lisp-mode (lambda ()
  (setq-local completion-at-point-functions (list #'tempel-expand ... #'cape-file ...)))

Nevertheless it would be nice to improve the situation upstream since the hook infrastructure should ideally be very solid.

titaniumbones commented 2 years ago

I may have to reduce the baroque elegance of my fine-tuning somewhat if I have to do it mode by mode... :-)

minad commented 2 years ago

Oh no, you can write your own even more baroque configuration macros which solve that elegantly!

minad commented 2 years ago

Closing. With the adjusted configurations the issue seems to be fixed mostly. The conclusion from this is that local hooks must not be used together with priorities. Local hooks itself work fine and global hooks with priorities work just fine. Only the combination of local+priority is problematic. But it seems the issue is going to be fixed finally for good on Emacs 29.