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

Exit when window is destroyed #53

Merged
merged 1 commit into from Sep 15, 2021
Merged

Exit when window is destroyed #53

merged 1 commit into from Sep 15, 2021

Conversation

TAAPArthur
Copy link
Contributor

If the user closed our window, the program won't automatically be die.
It may look dead as there would be no graphical indication that it was
running, but it still would be using/wasting the same resources.
Now the program will abruptly exit when its window is killed.

Note that forcibly destroying the window is different than sending the WM_DELETE_WINDOW request or forcibly closing the connection. We handled those cases fine.

Choose to exit with status 1 instead of 0 because don't consider this a normal case. Don't have a strong opinion on the matter

If the user closed our window, the program won't automatically be die.
It may look dead as there would be no graphical indication that it was
running, but it still would be using/wasting the same resources.
Now the program will abruptly exit when its window is killed.
@XPhyro
Copy link
Member

XPhyro commented Sep 15, 2021

nsxiv handles bspc node -k well. Is this not what you mean by forcibly closing?

@TAAPArthur
Copy link
Contributor Author

TAAPArthur commented Sep 15, 2021

nsxiv handles bspc node -k well. Is this not what you mean by forcibly closing?

@XPhyro I'm not familiar with that command. I had meant literally destroying the window via XDestroyWindow as opposed to requesting it be destroyed (what a WM should do) or killing the X connection (like with xkill).

If you write a sample program that just destroys the window or call XDestroyWindow in nsxiv right before we enter the event loop, you'll see nsxiv appear to hang with no window visible.

Copy link
Member

@XPhyro XPhyro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bspc is CLI interface for bspwm. I digged in the source code, it looks like bspc node -k uses xcb_kill_client. I do not know if this is equivalent to or related with XDestroyWindow.

I can't imagine it is common for XDestroyWindow to be used, but it would be nice for nsxiv to react correctly nonetheless.


I cannot vouch for the fix in the patch, but nsxiv compiles, and works normally, so I'll approve.

@TAAPArthur
Copy link
Contributor Author

TAAPArthur commented Sep 15, 2021

bspc is CLI interface for bspwm. I digged in the source code, it looks like bspc node -k uses xcb_kill_client. I do not know if this is equivalent to or related with XDestroyWindow.

It is not equivalent. What bspm does is more standard. The ICCCM spec suggests using kill client too

Window managers should not use DestroyWindow requests on a window that has WM_DELETE_WINDOW in its WM_PROTOCOLS property.

I personally don't know of a WM that uses XDestroyWindow (by default at least). Mine gives the option. In case your curious, a use case may be to destroy pop up windows uniformly with a keybinding.

But yeah don't think many people will run into this issue, but thought it was easy enough to address

@XPhyro XPhyro merged commit fe5c4bc into nsxiv:dev Sep 15, 2021
@TAAPArthur TAAPArthur deleted the destroy branch September 15, 2021 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
3 participants