dimmer.el icon indicating copy to clipboard operation
dimmer.el copied to clipboard

How to make dimmer compatible with corfu?

Open Eason0210 opened this issue 2 years ago • 8 comments

Hi @gonewest818

Thanks for making this awesome package.

Recently I try to setup corfu(it is a new completion font end made by @minad ) with dimmer, and I found that dimmer.el will affect corfu's popup window which uses child frames to show the popup.

I tried to add following config, but no effect, do you have any advice for making dimmer works with corfu?

(use-package dimmer
  :hook (after-init . dimmer-mode)
  :config
  (setq-default dimmer-fraction 0.15)
  (add-to-list 'dimmer-exclusion-regexp-list "^ \\*corfu\\*$"))

my corfu config:

(use-package corfu
  :init
  (setq corfu-cycle t)
  (setq corfu-auto t)
  (setq corfu-quit-at-boundary t)
  (setq corfu-quit-no-match t)
  (setq corfu-preview-current nil)
  (corfu-global-mode))

Eason0210 avatar Jan 19 '22 08:01 Eason0210

Completion popups in general have been hard to get right, it seems to me. If you have time I would check that the regexp is correct, first of all. (It looks reasonable to me but I’m not familiar with Corfu).

Second I would look for a predicate you can use for dimmer-prevent-dimming-predicates

Which should have the benefit not only preventing the child frame from being highlighted but also prevents anything else from being dimmed while the child frame is active.

It’s certainly possible there’s something deeper I need to do about child frames, and when time permits I can also try these experiments if you’re not in a position to.

On Jan 19, 2022, at 12:25 AM, Eason Huang @.***> wrote:

 Hi @gonewest818

Thanks for making this awesome package.

Recently I try to setup corfu(it is an new completion font end make by @minad ) with dimmer, and I found that dimmer.el will affect corfu's popup window which uses child frames to show the popup.

I tried to add following config, but no effect, do you have any advice for making dimmer works with corfu?

(use-package dimmer :hook (after-init . dimmer-mode) :config (setq-default dimmer-fraction 0.15) (add-to-list 'dimmer-exclusion-regexp-list "^ \corfu\$")) my corfu config:

(use-package corfu :init (setq corfu-cycle t) (setq corfu-auto t) (setq corfu-quit-at-boundary t) (setq corfu-quit-no-match t) (setq corfu-preview-current nil) (corfu-global-mode)) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

gonewest818 avatar Jan 19 '22 15:01 gonewest818

@gonewest818 Thanks for your advice, I will try the dimmer-prevent-dimming-predicates when I have time.

It’s certainly possible there’s something deeper I need to do about child frames

Actually, I saw dimmer.el already support posframe which is also implemented by child frame. And corfu use its own implementation.

Eason0210 avatar Jan 19 '22 16:01 Eason0210

True. Under the hood all we’re really doing about posframe is set a regexp

(add-to-list 'dimmer-exclusion-regexp-list "^ \*.posframe.buffer.\$"))

Because posframe frames tend to have names that include the text “posframe” and “buffer”. It might be similar with Corfu actually.

On Jan 19, 2022, at 8:06 AM, Eason Huang @.***> wrote:

 @gonewest818 Thanks for your advice, I will try the dimmer-prevent-dimming-predicates when I have time.

It’s certainly possible there’s something deeper I need to do about child frames

Actually, I saw dimmer.el already support posframe which is also implemented by child frame. And corfu use its own implementation.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

gonewest818 avatar Jan 19 '22 16:01 gonewest818

For me the solution buzztaki had for lsp-doc works here as well:

(defun advise-dimmer-config-change-handler ()
  "Advise to only force process if no predicate is truthy."
  (let ((ignore (cl-some (lambda (f) (and (fboundp f) (funcall f)))
                         dimmer-prevent-dimming-predicates)))
    (unless ignore
      (when (fboundp 'dimmer-process-all)
        (dimmer-process-all t)))))

(defun corfu-frame-p ()
  "Check if the buffer is a corfu frame buffer."
  (string-match-p "\\` \\*corfu" (buffer-name)))

(defun dimmer-configure-corfu ()
  "Convenience settings for corfu users."
  (add-to-list
   'dimmer-prevent-dimming-predicates
   #'corfu-frame-p))

(use-package dimmer
  :config
  (advice-add
   'dimmer-config-change-handler
   :override 'advise-dimmer-config-change-handler)
  (dimmer-configure-corfu)
  (dimmer-mode t)
  :defer 1)

Walheimat avatar Apr 09 '22 09:04 Walheimat

@Walheimat Thanks for sharing your solution. I works for me too.

Eason0210 avatar Apr 30 '22 12:04 Eason0210

keeping this issue open as a reminder to improve documentation or add to the implementation.

gonewest818 avatar May 15 '22 17:05 gonewest818

I tried putting everything under one use package declaration,

(use-package dimmer
  :ensure
  :defer
  :init
    (defun advise-dimmer-config-change-handler ()
      "Advise to only force process if no predicate is truthy."
      (let ((ignore (cl-some (lambda (f) (and (fboundp f) (funcall f)))
                             dimmer-prevent-dimming-predicates)))
        (unless ignore
          (when (fboundp 'dimmer-process-all)
            (dimmer-process-all t)))))

    (defun corfu-frame-p ()
      "Check if the buffer is a corfu frame buffer."
      (string-match-p "\\` \\*corfu" (buffer-name)))

    (defun dimmer-configure-corfu ()
      "Convenience settings for corfu users."
      (add-to-list
       'dimmer-prevent-dimming-predicates
       #'corfu-frame-p))
  :config
    (advice-add
     'dimmer-config-change-handler
     :override 'advise-dimmer-config-change-handler)
    (dimmer-configure-corfu)
    (dimmer-mode t))

Anyways, could the (dimmer-configure-corfu) not be added to the source?

simurgh9 avatar Nov 21 '23 07:11 simurgh9

@simurgh9 thanks for the config

would love to see this get merged as well

sho-87 avatar Jan 14 '24 09:01 sho-87