roman / golden-ratio.el

Automatic resizing of Emacs windows to the golden ratio
MIT License
589 stars 38 forks source link

Add advice on `select-window' #57

Open Alexander-Shukaev opened 9 years ago

Alexander-Shukaev commented 9 years ago

What's the rationale behind not adding an advice for select-window? Instead of adding advice to other-window, one should have advised select-window as the lowest-level function for window selection (which is used by all the other Emacs code that requires this action). Otherwise, this plugin is merely useless.

I'll leave the code here...

  (advice-add #'select-window
              :after
              #'(lambda
                    (select-window-function &rest ...)
                  (unless (minibuffer-window-active-p (selected-window))
                    (golden-ratio)))))

... feel free to adapt it to the old defadvice system.

By the way, in general, the ultimate recipe to ensure some action happening when frames/windows/buffers are switched is

(add-hook 'focus-in-hook              ...)
(add-hook 'focus-out-hook             ...)
(add-hook 'window-configuration-hook  ...)

(advice-add #'select-window
            :after
            #'(lambda
                  (select-window-function &rest ...)
                (unless (minibuffer-window-active-p (selected-window)) ;; Optional
                  ...))))
Alexander-Shukaev commented 9 years ago

For the sake of completeness, I would like to mention another option (which might be even better choice for golden-ratio especially). That is instead of explicitly advising select-window, issue

(add-hook 'buffer-list-update-hook #'golden-ratio)

For instance, on Windows explicitly advising select-window with golden-ratio results in relatively frequent screen blinking/renewal, probably because select-window is called somewhere (non-interactively) in the background (perhaps by some other plugin that I have installed). On the other hand, buffer-list-update-hook does not introduce such a problem.

Alexander-Shukaev commented 9 years ago

After some testing, I've improved this approach even further by minimizing the number of executions of golden-ratio based on whether a new window was really selected. See the Emacs documentation for more details. In brief, select-window is called extremely often (by other Emacs Lisp code), and it's not always intended for visual changes (interactive use). Although, it has the NORECORD parameter to signify that the corresponding call of select-window is a "programmatic" one rather than an interactive one, it seems like many developers ignore that feature. Hence, buffer-list-update-hook is also spammed too often. For instance, I've still noticed some problems with company stuttering (with the buffer-list-update-hook approach). The remedy is to track down the "previous" selected window and compare it to the "currently" selected window. I'll leave the complete code here:

(defvar golden-ratio-selected-window
  (frame-selected-window)
  "Selected window.")

(defun golden-ratio-set-selected-window
    (&optional window)
  "Set selected window to WINDOW."
  (setq-default
    golden-ratio-selected-window (or window (frame-selected-window))))

(defun golden-ratio-selected-window-p
    (&optional window)
  "Return t if WINDOW is selected window."
  (eq (or window (selected-window))
      (default-value 'golden-ratio-selected-window)))

(defun golden-ratio-maybe
    (&optional arg)
  "Run `golden-ratio' if `golden-ratio-selected-window-p' returns nil."
  (interactive "p")
  (unless (golden-ratio-selected-window-p)
    (golden-ratio-set-selected-window)
    (golden-ratio arg)))

(add-hook 'buffer-list-update-hook #'golden-ratio-maybe)
(add-hook 'focus-in-hook           #'golden-ratio)
(add-hook 'focus-out-hook          #'golden-ratio)

Please, consider adding this to golden-ratio.el. If you still don't like this to be active by default, then introduce the corresponding Boolean customization variable and/or function to toggle this feature.

jiri commented 8 years ago

+1 :+1:

Alexander-Shukaev commented 7 years ago

It looks like this package starts to rot. Pity. However, when I have some more free time, I'll be forking and maintaining this.

Sherlock92 commented 7 years ago

@Alexander-Shukaev I just ran into this one. I'll join you in maintaining once i'm a bit more comfortable with lisp.

mrcnski commented 7 years ago

@Alexander-Shukaev I just found this package, it's a great idea but doesn't work with window-numbering.el, so it's useless. Would be awesome if there was an updated alternative.

alexvorobiev commented 7 years ago

@m-cat Until the package is fixed you can use this:

(defadvice select-window-by-number
              (after golden-ratio-resize-window activate)
            (golden-ratio) nil)))
cmm commented 7 years ago

Just to confirm: took the snippet from https://github.com/roman/golden-ratio.el/issues/57#issuecomment-131472709 with some minor cosmetic changes, killed off post-command hooking and advice, overall the code got simpler and things seem to work nicely.