Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could dedicated popup windows be a thing? #46

Open
angrybacon opened this issue Mar 10, 2023 · 7 comments
Open

Could dedicated popup windows be a thing? #46

angrybacon opened this issue Mar 10, 2023 · 7 comments

Comments

@angrybacon
Copy link

So I realize it might not necessarily be popper's responsibility to handle the dedicated behavior but I was wondering how I could cobble that together as a proof of concept without breaking the cycle feature from popper.

For context, my issue is that with a layout similar to below:

image

Visiting a file from the Magit status using magit-diff-visit-file-other-window sometimes uses my bottom window (undesired). I'm guessing the behavior is reproducible with any -other-window command. So I was thinking it was only a matter of setting popup windows as dedicated (in popper-raise-popup perhaps?) as mentioned above but I fail to see how not to break popper-cycle. Is it something you've worked on/thought about yet?

@angrybacon angrybacon changed the title Could dedicated popper windows be a thing? Could dedicated popup windows be a thing? Mar 10, 2023
@karthink
Copy link
Owner

This shouldn't be happening, might be a bug. What is your value of popper-display-function?

@angrybacon
Copy link
Author

This shouldn't be happening, might be a bug. What is your value of popper-display-function?

The value is popper-select-popup-at-bottom

I took some time to dig around and here is what I can reproduce with:

(progn
  (require 'package)

  (package-initialize)

  (require 'magit)
  (setq-default magit-display-buffer-function 'magit-display-buffer-same-window-except-diff-v1)
  (define-key magit-file-section-map (kbd "RET") #'magit-diff-visit-file-other-window)
  (define-key magit-hunk-section-map (kbd "RET") #'magit-diff-visit-file-other-window)

  (require 'popper)
  (setq-default popper-reference-buffers `(,(rx bos "*Messages*" eos)))
  (popper-mode)

  (require 'shackle)
  (setq-default shackle-rules `(("*Messages*" :align below :popup t)))
  (shackle-mode)

  (advice-add 'shackle--display-buffer-aligned-window :after
    (defun me/shackle-set-window-side (_buffer _alist plist)
      "Set window side parameter for `shackle-last-window' according to PLIST.
This allows other features to filter or select windows based on their function
ie. a side window."
      (with-selected-window shackle-last-window
        (when-let ((window shackle-last-window)
                   (alignment (plist-get plist :align)))
          (set-window-parameter window 'window-side t)))))

  (toggle-frame-maximized)              ; Give more room to reproduce screenshot
  (split-window-right)
  (magit-status)                        ; Assume buffer is vc-backed
  (view-echo-area-messages))            ; Open a popper-managed window
  1. Run the above under emacs -Q
  2. In the Magit buffer hit Return on a file or hunk
  3. The file or hunk is displayed in the bottom popup

I'm using the me/shackle-set-window-side advice to filter out side windows with my Olivetti setup (here is the relevant part if you're interested) ie. if a window is assigned to a side — usually left or bottom — it's safe to assume I don't want to center it with Olivetti margins.

The problem described in this issue only happens when running that advice.

@karthink
Copy link
Owner

I'm using the me/shackle-set-window-side advice to filter out side windows

widowmaker-olivetti--maybe-predicate looks fine, but I'm not sure what me-shackle-set-window-side is doing? It's working by side-effect, not just acting as a predicate/filter.

The problem described in this issue only happens when running that advice.

That narrows it down. I'm guessing that the problem is explicitly setting the window-side window parameter. It's not a boolean, not intended to be set explicitly, and usually has a value like left or bottom.

 ‘side’
      Denotes the side of the frame where the window shall be
      located.  Valid values are ‘left’, ‘top’, ‘right’ and
      ‘bottom’.  If unspecified, the window is located at the bottom
      of the frame.

Could you try changing it to (set-window-parameter window 'window-side alignment) or something like that? Again, I'm not sure what this piece of advice is meant to do.

@angrybacon
Copy link
Author

Thanks for looking into it. The advice is meant to tag whatever windows are handled by shackle. In doing that, widowmaker-olivetti--maybe-predicate can recognize not to mess with it

I'll try your suggestion thanks

@angrybacon
Copy link
Author

angrybacon commented Mar 11, 2023

       (with-selected-window shackle-last-window
         (when-let ((window shackle-last-window)
                    (alignment (plist-get plist :align)))
-          (set-window-parameter window 'window-side t)))))
+          (set-window-parameter window 'window-side alignment)))))

Same result ie. visiting a file from the Magit status reuses the bottom popup

Edit: apparently the value coming from shackle doesn't directly match the allowed values from the documentation you've linked above (for bottom windows it uses below instead). Just to be sure I've tried by using hardcoded bottom but same result

-          (set-window-parameter window 'window-side t)))))
+          (set-window-parameter window 'window-side 'bottom)))))

@karthink
Copy link
Owner

Could you paste here all the window parameters of a popup that's misbehaving? ((window-parameters) in the popup window.)

@angrybacon
Copy link
Author

angrybacon commented Mar 11, 2023

Window parameters for the bottom message buffer as per the test file:

Before hitting return to visit a diff file

 ((internal-region-overlay . #<overlay in no buffer>)
  (window-side . bottom)
  (quit-restore window window
                #<window 3 on magit: emacs>
                #<buffer *Messages*>))

After hitting return on a file from the Magit status window:

 ((internal-region-overlay . #<overlay in no buffer>)  (window-side . bottom)
  (quit-restore other
                (#<buffer *Messages*> 1 #<marker (moves after insertion) at 288 in *Messages*> 31)
                #<window 3 on magit: emacs>
                #<buffer use-display.el>))

After going back to the message buffer with previous-buffer, window-parameters has the exact same output as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants