ch11ng / exwm

Emacs X Window Manager
2.86k stars 137 forks source link

Make sure the buffer that generates the event is current #900

Closed nbarrientos closed 1 year ago

nbarrientos commented 1 year ago

I've seen that, in some cases, after switching from a non-EXWM buffer to an EXWM one and a mouse click event is triggered, (current-buffer) returns still the old buffer when exwm-input--fake-last-command (called by exwm-input--on-ButtonPress) is executed.

If I temporary add this debug statement to exwm-input--fake-last-command:

(defun exwm-input--fake-last-command ()
  "Fool some packages into thinking there is a change in the buffer."
  (let ((exwm-debug t))
    (exwm--log "current-buffer=%S selected-window=%S"
               (current-buffer) (selected-window))
    (setq last-command #'exwm-input--noop)
    (run-hooks 'pre-command-hook)
    (run-hooks 'post-command-hook)))

and, once an ERC channel buffer (#<buffer #erc>) is current and displayed in a window, I run:

M-x eval-expression (browse-url "http://www.gnu.org") RET

an EXWM buffer containing Firefox takes over the window and, when left-clicking on it, *XELB-DEBUG* contains:

exwm-input--fake-last-command:
current-buffer=#<buffer #erc>
selected-window=#<window 7 on F# The GNU Operating System and the Free Software Mov... @ www.gnu.org>

(note that current-buffer is still #<buffer #erc>, whereas it should be something like #<buffer F# The GNU Operating System and the Free Software Mov... @ www.gnu.org>.

This, apart from being wrong in my opinion, is also problematic if the previous buffer's post-command-hook contains statements that require the buffer to be visible in a window, for instance calls to recenter. If that's the case, EXWM bails out when calling exwm-input--fake-last-command and ultimately the mouse click event is not processed. Error message:

error in process filter: exwm-input--fake-last-command: ‘recenter’ing a window that does not display current-buffer.

This "synthetic" reproducer is actually the case of ERC buffers where erc-scrolltobottom-mode is enabled (erc-scroll-to-bottom, which eventually calls recenter, is added to the buffer's post-command-hook).

I don't really know why in this scenario (current-buffer) does not return the EXWM buffer that's active, but this patch makes sure that the current buffer is the one generating the click event and hence the post-command-hook that's executed is the one that's supposed to be (not #<buffer #erc>'s). Once the patch is applied:

exwm-input--on-buffer-list-update:
current-buffer=#<buffer "F# The GNU Operating System and the Free Software Mov... @ www.gnu.org">
selected-window=#<window 7 on F# The GNU Operating System and the Free Software Mov... @ www.gnu.org>

is written to *XELB-DEBUG* when using the reproducer above as expected.

medranocalvo commented 1 year ago

Dear @nbarrientos, thank you very much for this improvement. I detest that it’s taken so long.

I wondered why with-current-buffer is invoked multiple times in this function. Can’t we wrap it all in with-current-buffer? I think the answer is “no”: this function makes xcb request/replies, and anything can happen while waiting for the reply. (I’m just thinking out loud).

As I mentioned in https://github.com/ch11ng/exwm/pull/897#issuecomment-1324282700, this error class is pervasive in EXWM. Thank you.

I will adjust the commit message and merge soon.

medranocalvo commented 1 year ago

Merged! This is the first change since December: congratulations! Hopefully we'll cut a release soon.

nbarrientos commented 1 year ago

Thanks for taking a look!

medranocalvo commented 9 months ago

Just released 0.28, featuring this fix. Thank you for improving EXWM!