tumashu / vertico-posframe

GNU General Public License v3.0
106 stars 16 forks source link

vertico-posframe does not actually move the minibuffer #7

Closed minad closed 2 years ago

minad commented 2 years ago

Since vertico-posframe does not actually move the minibuffer the overlays present in the minibuffer are not shown in the popup. For example consult-ripgrep adds the process status indicator. In comparison my mini-popup package move the minibuffer to the popup. The same approach is taken in vertico-buffer, where the actual minibuffer is shown in a separate window.

Furthermore I noticed that you use a vertico-posframe--minibuffer-cover here, which is quite inefficient. You can instead scroll the minibuffer window to hide the content. See https://github.com/minad/vertico/blob/ef499b6e2fdbc2d489cfe76a5dd88d62bfb64db9/extensions/vertico-buffer.el#L88-L90.

tumashu commented 2 years ago

cool, i will try these approach

-- 发自我的网易邮箱手机智能版

在 2022-01-04 23:57:40,"Daniel Mendler" @.***> 写道:

Since vertico-posframe does not actually move the minibuffer the overlays present in the minibuffer are not shown in the popup. For example consult-ripgrep adds the process status indicator. In comparison my mini-popup package move the minibuffer to the popup. The same approach is taken in vertico-buffer, where the actual minibuffer is shown in a separate window.

Furthermore I noticed that you use a vertico-posframe--minibuffer-cover here, which is quite inefficient. You can instead scroll the minibuffer window to hide the content. See https://github.com/minad/vertico/blob/ef499b6e2fdbc2d489cfe76a5dd88d62bfb64db9/extensions/vertico-buffer.el#L88-L90.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.Message ID: @.***>

tumashu commented 2 years ago

https://github.com/tumashu/vertico-posframe/commit/7840617866c6f0518ad85e077db2be90b86c556e

gcv commented 2 years ago

@tumashu: Something about these changes made vertico-posframe stop working for me. I only have it enabled for M-x like this:

(add-to-list 'vertico-multiform-commands '(execute-extended-command indexed posframe))

Steps to reproduce the problem:

  1. Start with a fresh Emacs session. Load vertico and vertico-posframe. Apply configuration.
  2. Hit M-x. Hit C-g to cancel out of it.
  3. Now hit M-x again and try to use arrow keys to scroll. Press Enter to select any command. It will error out saying "Text is read-only".

The only way to get out of this state is to execute (posframe-delete-all).

Edit: It worked fine in commit 0679636610e8, though I had a problem with the minibuffer overlay frame being one line too high.

tumashu commented 2 years ago

Does this function work?

(defun vertico-posframe--display (_lines)
  "Display LINES in posframe."
  (let* ((show-minibuffer-p (vertico-posframe--show-minibuffer-p))
         (minibuffer-window (active-minibuffer-window))
         (point (point)))
    (setq-local inhibit-read-only t)
    (setq vertico-posframe--buffer (current-buffer))
    (window-resize minibuffer-window
                   (- (window-pixel-height minibuffer-window))
                   nil nil 'pixelwise)
    (set-window-vscroll minibuffer-window 100)
    (when show-minibuffer-p
      (set-window-vscroll minibuffer-window 0))
    (with-selected-window (vertico-posframe-last-window)
      (vertico-posframe--show vertico-posframe--buffer point))))
gcv commented 2 years ago

No, now it says "Match required" instead. The point is moved incorrectly outside of the normal input area. Screencast attached

https://user-images.githubusercontent.com/10327/148298128-8c7cae04-033c-4f44-8bcb-1d41a17b8793.mov

.

tumashu commented 2 years ago
Now hit M-x again and try to use arrow keys to scroll. Press Enter to select any command. It will error out saying "Text is read-only".

In my machine I have no this problem, please use vertico-posframe 0.4.7 and send me a screencast.

gcv commented 2 years ago

After some debugging, I narrowed the problem down to an incompatibility with Perspective. Among other things, Perspective has code (much simplified) like this:

(defun testfn (frame)
  (with-selected-frame frame
    ;; do something
    ))

(add-hook 'delete-frame-functions 'testfn)

If you evaluate this code, you'll reproduce the problem. This is with vertico-posframe 0.4.7. It doesn't seem to like with-selected-frame.

For completeness, I start Emacs like this (adjust paths as needed):

emacs -Q -l ~/path/to/vertico-20220104.337/vertico-autoloads.el \
  -l ~/path/to/posframe-20211222.256/posframe-autoloads.el \
  -l ~/path/to/vertico-posframe-0.4.7/vertico-posframe-autoloads.el \
  --eval '(vertico-mode 1)' \
  --eval '(vertico-multiform-mode 1)' \
  --eval "(add-to-list 'vertico-multiform-commands '(execute-extended-command indexed posframe))"

Screencast:

https://user-images.githubusercontent.com/10327/148414468-687a5689-572f-493c-978e-82e9582f264c.mov

tumashu commented 2 years ago

please try this function @gcv

(defun posframe-delete-frame (buffer-or-name)
  "Delete posframe pertaining to BUFFER-OR-NAME.
BUFFER-OR-NAME can be a buffer or a buffer name."
  (dolist (frame (frame-list))
    (let ((delete-frame-functions nil)
          (buffer-info (frame-parameter frame 'posframe-buffer))
          (buffer (get-buffer buffer-or-name)))
      (when (or (equal buffer-or-name (car buffer-info))
                (equal buffer-or-name (cdr buffer-info)))
        (when buffer
          (with-current-buffer buffer
            (dolist (timer '(posframe--refresh-timer
                             posframe--timeout-timer))
              (when (timerp timer)
                (cancel-timer timer)))))
        (delete-frame frame)))))
gcv commented 2 years ago

That works! Thanks!

tumashu commented 2 years ago

@gcv ok, I will update posframe.

tumashu commented 2 years ago

@gcv posframe has been updated