search

milkypostman / powerline

emacs powerline
Other
793 stars 118 forks source link

`defadvice select-window` considered harmful #161

Closed Alexander-Shukaev closed 5 years ago

Alexander-Shukaev commented 5 years ago

Instead of

(defadvice select-window (after powerline-select-window activate)
  "makes powerline aware of window changes"
  (powerline-set-selected-window))

why not add a hook to buffer-list-update-hook (as recommended)?

The problem is that (select-window window 'norecord) is used by Emacs Lisp developers to "programatically" select windows without them producing side effects (such as flickering and/or buffer list updates, see the documentation of select-window for more information). The way select-window is advised now, aggressively invokes powerline updates even during such programmatic window selections. The solution is to either change to around advice and check the value of norecord argument or to use buffer-list-update-hook instead.

milkypostman commented 5 years ago

this is currently in maintenance mode and while i see the impact here, it's not something i have time to work on. happy to accept a patch.

On Fri, Jan 18, 2019 at 7:50 PM Alexander Shukaev notifications@github.com wrote:

Instead of

(defadvice select-window (after powerline-select-window activate) "makes powerline aware of window changes" (powerline-set-selected-window))

why not add a hook to buffer-list-update-hook (as recommended)?

The problem is that (select-window window 'norecord) is used by Emacs Lisp developers to "programatically" select windows without them producing side effects (such as flickering and/or buffer list updates, see the documentation of select-window for more information). The way select-window is advised now, aggressively invokes powerline updates even during such programmatic window selections. The solution is to either change to around advice and check the value of norecord argument or to use buffer-list-update-hook instead.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/milkypostman/powerline/issues/161, or mute the thread https://github.com/notifications/unsubscribe-auth/AACUsjvaZzzh7vK3dTUYcuIgSoiyFfvAks5vEnnngaJpZM4aI5jx .

Alexander-Shukaev commented 5 years ago

Did you have a chance to review the updated PR?

milkypostman commented 5 years ago

Should be submitted now.

On Tue, Jan 29, 2019 at 12:14 Alexander Shukaev notifications@github.com wrote:

Did you have a chance to review the updated PR?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/milkypostman/powerline/issues/161#issuecomment-458647584, or mute the thread https://github.com/notifications/unsubscribe-auth/AACUssls8IhOLnJpu_lAlUiMbJh1o0Y0ks5vII-LgaJpZM4aI5jx .

Alexander-Shukaev commented 5 years ago

Thanks!