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

Fix #1286 by sending SIGHUP when closing a pane #1320

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

raphCode
Copy link
Contributor

SIGHUP correctly states the intention behind sending a signal when a
pane is closed: The controlling terminal is "hung up".
Also, SIGHUP is better suited than SIGTERM since bash ignores the
latter. This led to the zombie processes observed by some users.
Furthermore, SIGHUP has a special meaning in bash's job control, namely
re-sending the signal to all owned jobs before exiting.

SIGHUP correctly states the intention behind sending a signal when a
pane is closed: The controlling terminal is "hung up".
Also, SIGHUP is better suited than SIGTERM since bash ignores the
latter. This led to the zombie processes observed by some users.
Furthermore, SIGHUP has a special meaning in bash's job control, namely
re-sending the signal to all owned jobs before exiting.
@imsnif
Copy link
Member

imsnif commented Apr 14, 2022

Thank you very much for this! I'm not sure I'll get a chance to fully test this today - worst case will tomorrow.

@imsnif imsnif self-assigned this Apr 14, 2022
@imsnif
Copy link
Member

imsnif commented Apr 15, 2022

Tested this a little bit and it LGTM. Thanks once again for looking into this issue!

@imsnif imsnif merged commit d4b8199 into zellij-org:main Apr 15, 2022
@raphCode
Copy link
Contributor Author

raphCode commented Apr 15, 2022

Thanks for merging!

LGTM

I am sure you mean this in a positive way, but I found it surprising that actually multiple and contrary definitions exist in the context of (code) reviews:

  • lets good to me
  • lets get this merged
  • literally garbage to me

https://www.urbandictionary.com/define.php?term=LGTM

@imsnif
Copy link
Member

imsnif commented Apr 15, 2022

:D Looks Good To Me

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.

2 participants