popper icon indicating copy to clipboard operation
popper copied to clipboard

Order of popper-mode and popper-reference-buffers affects functionality

Open 9s-l-s9 opened this issue 2 years ago • 2 comments

Hello,

I encountered a configuration issue when setting up popper for eshell buffers. When (popper-mode +1) was placed before (setq popper-reference-buffers ... ) in my configuration, e.g. eshell buffers were not being treated as pop-up buffers.

Here is a sample configuration to reproduce the issue:

(require 'popper)
(require 'popper-echo)

(popper-mode +1)
(popper-echo-mode +1)

(setq popper-reference-buffers
      '("\\*Messages\\*"
         "Output\\*$"
         help-mode
         compilation-mode
         "^\\*eshell.*\\*$" eshell-mode ;eshell as a popup
         "^\\*shell.*\\*$"  shell-mode  ;shell as a popup
         "^\\*term.*\\*$"   term-mode   ;term as a popup
         "^\\*vterm.*\\*$"  vterm-mode  ;vterm as a popup
       ))
(setq popper-display-control t)

However, after I placed (popper-mode +1) after popper-reference-buffers, eshell and the other modes started behaving as expected as a pop-up buffer.

It would be helpful if the documentation could mention this configuration order dependency, or if the implementation could be adjusted to avoid this issue.

Thank you for your work on this very useful package.

9s-l-s9 avatar May 30 '23 18:05 9s-l-s9

This is intentional -- when popper-mode is turned on, it sets up some predicates based on popper-reference-buffers that run every time the window configuration changes. It causes performance issues if this runs dynamically with many frames/windows open.

For this reason the configuration examples in the README turn on popper-mode only after setting popper-reference-buffers -- but maybe I should mention this explicitly in the documentation somewhere.

karthink avatar Jun 01 '23 02:06 karthink

This is intentional -- when popper-mode is turned on, it sets up some predicates based on popper-reference-buffers that run every time the window configuration changes. It causes performance issues if this runs dynamically with many frames/windows open.

For this reason the configuration examples in the README turn on popper-mode only after setting popper-reference-buffers -- but maybe I should mention this explicitly in the documentation somewhere.

Probably, a better way of updating these predicates, while avoiding said performance issues, is to make use of the :set keyword of defcustom. Then, those that use setopt (as they should) won't have to worry about such ordering. Note that in this case popper's README should recommend that destructive/in-place modification functions (e.g. add-to-list) should not be used and that a call setopt be made with every change to the variable.

Also, note that with the current ordering popper-mode should probably not be autoloaded (which it currently is) since it is not always an appropriate entry-point.

hab25 avatar Feb 08 '24 08:02 hab25