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

Correctly activate windows even if they're not in the iconic state #32

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

Stebalien
Copy link
Contributor

@Stebalien Stebalien commented Apr 1, 2024

Handle the case where a window is hidden but not iconic. This code would previously error in some cases because a buffer wouldn't have an active window even when it was not in the iconic state, likely due to a focus race somewhere.

fixes #28

* exwm.el (exwm--on-ClientMessage): Handle the case where a window is
hidden but not iconic. This code would previously error in some cases
because a buffer wouldn't have an active window even when it was not in
the iconic state, likely due to a focus race somewhere.

fixes #28
fixes #29
fixes #30
@Stebalien
Copy link
Contributor Author

Unfortunately this patch is more of a fix for the symptom rather than the underlying problem (I think), but it's still more robust than the previous code so I'm reasonably happy with it.

I'm also not able to reliably reproduce this problem, so I'll have to rely on the community to test it.

@minad
Copy link
Member

minad commented Apr 1, 2024

Thanks! I was about to suggest a similar patch. The problem is that the iconic state gets out of sync with the buffer visibility. I think it is better to avoid maintaining "duplicate" state which needs to stay in sync. Maybe it is possible to eliminate some state?

@Stebalien
Copy link
Contributor Author

Unfortunately, we actually have three variables here:

  1. Buffer visibility.
  2. Buffer-local exwm-state.
  3. The WM_STATE window property.

The exwm-state local variable is just a mirror of the WM_STATE window property (likely for performance?).

I think we need to continue using exwm-state, but only in exwm-layout--auto-iconify? Otherwise, yeah, we should just be checking if the buffer is actually visible or not.

@Stebalien
Copy link
Contributor Author

E.g., I think the following need to change:

(unless (or (exwm-layout--iconic-state-p)

(if (exwm-layout--iconic-state-p)

(when (exwm-layout--iconic-state-p)

@minad
Copy link
Member

minad commented Apr 1, 2024

Okay, let's keep this state duplication problem in mind. We don't have to address this now. From my side this PR looks good conceptually, but I haven't tested it yet.

@Stebalien
Copy link
Contributor Author

This patch doesn't cause any problems for me, but I'd still like some feedback from the community to see if it actually fixes any problems. I only ran into it once or twice and can't reliably reproduce it.

@rayslava
Copy link

rayslava commented Apr 2, 2024

It seems that the change helps with focus issue, when some buffers did switch randomly, didn't help with exwm-xim though

Upd: no, the focus issue is not fixed as well :(

@Stebalien
Copy link
Contributor Author

Ok, I think your bug is https://bbs.archlinux.org/viewtopic.php?id=294301.

I'm going to merge this PR as a fix for my issue (that appears to be unrelated).

@Stebalien Stebalien merged commit 236f3ca into master Apr 3, 2024
@Stebalien Stebalien deleted the steb/robust-activate-window branch April 3, 2024 21:08
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

Successfully merging this pull request may close these issues.

Attempts to select 'nil' window
3 participants