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

Add advice on `select-window' #57

Open
Alexander-Shukaev opened this issue Aug 7, 2015 · 8 comments
Open

Add advice on `select-window' #57

Alexander-Shukaev opened this issue Aug 7, 2015 · 8 comments

Comments

@Alexander-Shukaev
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link

jiri commented Feb 21, 2016

+1 👍

@Alexander-Shukaev
Copy link
Author

Alexander-Shukaev commented Mar 14, 2017

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
Copy link

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

@mrcnski
Copy link

mrcnski commented Mar 30, 2017

@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
Copy link

@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
Copy link

cmm commented May 11, 2017

Just to confirm: took the snippet from #57 (comment) with some minor cosmetic changes, killed off post-command hooking and advice, overall the code got simpler and things seem to work nicely.

cmm pushed a commit to cmm/golden-ratio.el that referenced this issue May 12, 2017
* Took code from
  roman#57 (comment)
  with some modifications.

* Killed off `golden-ratio-extra-commands' (and the associated
  post-command hook) and all defadvices.

* Touch fewer hooks.
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

6 participants