muffinmad / emacs-mini-frame

Show minibuffer in child frame on read-from-minibuffer
GNU General Public License v3.0
322 stars 20 forks source link

mini-frame-resize = t causes mini-frame to flicker #38

Closed Alexander-Miller closed 3 years ago

Alexander-Miller commented 3 years ago

It looks like the resizing happens after the frame is displayed, so it becomes visible as an annoying flickering:

mini-frame-flicker

I am on the latest version of mini-frame, my config looks like this:

(setf
 mini-frame-resize t
 mini-frame-ignore-commands
 '(eval-expression
   "edebug-eval-expression"
   debugger-eval-expression
   ".*helm.*")
 mini-frame-show-parameters
 '((background-color . "#2E2E32")
   (left . 0.5)
   (top . 40)
   (width . 0.6)
   (height . 0.5)))

In case it's relevant: I am also using selectrum and marginalia.

muffinmad commented 3 years ago

Set the height parameter to 10 in the mini-frame-show-parameters variable.

Alexander-Miller commented 3 years ago

With that I see a flicker for mini-frames that are smaller than those 10 lines.

muffinmad commented 3 years ago

And what about the default height . 1? Also note that you can use custom function as the mini-frame-show-parameters value and set height according to current command etc.

Alexander-Miller commented 3 years ago

Just height . 1 mostly works. However when I use the dynamic function the height component does not seem to be honored properly and the mini-frame shrinks down again with another flicker. It's not visible on a gif, so here's a webm: https://imgur.com/a/gciX8Sh

I have set the height of the imenu to 20 lines like this, so I have no idea where the 10 lines it shrinks down to come from.

(pcase this-command
     (`consult-imenu
      '((background-color . "#2E2E32")
        (left . 0.5)
        (top . 40)
        (width . 0.6)
        (height . 20)))
     (_
      '((background-color . "#2E2E32")
        (left . 0.5)
        (top . 40)
        (width . 0.9)
        (height . 1))))
muffinmad commented 3 years ago

What completion framework do you use?

Alexander-Miller commented 3 years ago

I'm using selectrum. I just started migrating away from helm, so nothing's customized yet.

The extra info you see in the gifs comes from marginalia, but turning it off does not change anything.

muffinmad commented 3 years ago

According to the Selectrum's README

By default, ten candidates are shown in the minibuffer at any given time

That's where 10 lines come from.

Alexander-Miller commented 3 years ago

Ah, that makes sense. I checked now and it looks like selectrum takes total control of the mini-frame's height. So that's something I'll have to take up with selectrum.

But still, even with selectrum-mode disabled ther's still flickering. With a config like this:

(pcase this-command
     (`imenu
      '((background-color . "#2E2E32")
        (left . 0.5)
        (top . 40)
        (width . 0.6)
        (height . 10)))
     (_
      '((background-color . "#2E2E32")
        (left . 0.5)
        (top . 40)
        (width . 0.9)
        (height . 1))))

I still see this (the flicker is only visible the first but, but it's actually always present):

imenu

muffinmad commented 3 years ago

No need to set height to 10 with selectrum-mode disabled. I don't know when exactly Emacs fits minibuffer-frame to buffer, but looks like shortly after the frame became visible. Try to set the height parameter to 1. It can be less distracting.

Alexander-Miller commented 3 years ago

Try to set the height parameter to 1. It can be less distracting.

That a good default for the usual minibuffer interactions like y-or-no-p questions, or simple text input. However for a few windows, like imenu I prefer to have a static size. And even with a default height of 1 the frame size adjustment is still visible, but since the frame is always growing it looks more like a drop-down than a flicker.

Popping up dynamically sized child-frames without flickering is quite doable. posframe does it, and I did it myself with my own private hack for packages like helm. The frame's size just needs to be set before it becomes visible. It can be done with a quick PoC as below, but of course your code relies quite a bit on resize-mini-frames, so a proper change would be much more complicated.

modified   mini-frame.el
@@ -346,11 +346,14 @@ ALIST is passed to `window--display-buffer'."
     (when (and (frame-live-p mini-frame-completions-frame)
                (frame-visible-p mini-frame-completions-frame))
       (make-frame-invisible mini-frame-completions-frame))
-    (make-frame-visible mini-frame-frame)
-    (redirect-frame-focus mini-frame-selected-frame mini-frame-frame)
-    (select-frame-set-input-focus mini-frame-frame)
-    (setq default-directory dd)
-    (apply fn args)))
+    (let ((height (alist-get 'height show-parameters))
+          (resize-mini-frames nil))
+      (set-frame-height mini-frame-frame height)
+      (make-frame-visible mini-frame-frame)
+      (redirect-frame-focus mini-frame-selected-frame mini-frame-frame)
+      (select-frame-set-input-focus mini-frame-frame)
+      (setq default-directory dd)
+      (apply fn args))))

It'd be great if mini-frame could support this kind of behavior, the idea is great, but that flicker is a pet-peeve that I just cannot get rid of.

muffinmad commented 3 years ago

The resize-mini-frames variable is already set here:

https://github.com/muffinmad/emacs-mini-frame/blob/41afb3d79cd269726e955ef0896dc077562de0f5/mini-frame.el#L410-L413

If you want the resize-mini-frames to be nil, set the mini-frame-resize to nil.

Alexander-Miller commented 3 years ago

The resizing's necessary for the cases when the frame should be small because there are no completion candidates, but that can be achieved in other ways: mini-frame-show-parameters should be made aware of what function is currently being displayed and what it's arguments are. That way you'll be able to know if read-string is being used, or whether read-from-minibuffer is called without a collection argument.

That's a change that doesn't require a deep knowledge of mini-frame's internals, so I suggest that I'll drop a PR in a few days.

aaronjensen commented 3 years ago

I came to report this as well. As an aside, Emacs 28 does not flicker in the same way. Specifically, it never blanks and redraws, so the flicker is less jarring. It still shows up temporarily as the wrong size and then quickly shifts to the right size. It's still irritating even without the blanking.

I notice it more when the window closes. Just before it closes, it resizes back to the minimum I have set (2 lines):

CleanShot 2021-02-24 at 23 13 54@2x
Alexander-Miller commented 3 years ago

Apologies for leaving this issue around for so long. Long story short finding out the number of completion candidates before the mini-frame is displayed is a lot more complicated than expected, and I have only recently settled on a setup that mostly works for me (at least with selectrum).

My config looks like this now:

First selectrum needs to be advised so I know how many completion candidates there are.

(defvar std::selectrum-candidates nil)

(cl-defun std::selection::set-selectrum-candidates
    (_ collection &rest other-args &key mc-table &allow-other-keys)
  (setf std::selectrum-candidates (or collection mc-table)))

(std::add-advice #'std::selection::set-selectrum-candidates
    :before #'selectrum--read)

Then my mini-frame-show-parameters can use that knowledge to dynamically set selectrum's max height:

(defun std::mini-frame-show-parameters ()
  (let ((width 0.9)
        height)
    (pcase this-command
      ('consult-imenu (setf height 15 width 0.6))
      ('find-file     (setf height 10))
      ('find-library  (setf height 10))
      ((guard (not (null std::selectrum-candidates)))
       (setf height (if (listp std::selectrum-candidates)
                        (min 8 (1+ (length std::selectrum-candidates)))
                      8)))
      (_ (setf height 2)))
    (setf selectrum-max-window-height (1- height)
          std::selectrum-candidates nil)
    `((background-color . "#2E2E32")
      (left . 0.5)
      (top . 40)
      (height . ,height)
      (width . ,width))))

That works well enough in 99% of cases. Only magit's checkout-branch commands seems to only get the full list of branches after you press TAB, so the frame is oftentimes too small, but other than that the issue is solved as far as I'm concerned.

aaronjensen commented 3 years ago

Interesting, for some reason when I do this it makes the flicker more pronounced. Specifically, it starts at the right size, then resizes to my minimum and then resizes back to the right height.

CleanShot 2021-07-21 at 21 34 45

Also, i get a flicker of contents--it appears with old contents before switching to new contents, so if, for example, I do a describe variable and then an M-x, i'll see the describe variable remnants before it renders M-x.