From: Phillip Wood Date: Thu, 10 Mar 2022 16:06:29 GMT Subject: Re: [PATCH 4/4] terminal: restore settings on SIGTSTP Message-ID: <7fff6841-968d-b741-2589-fa737499113d@gmail.com> In-Reply-To: Hi Dscho On 09/03/2022 12:19, Johannes Schindelin wrote: > Hi Phillip, > > On Fri, 4 Mar 2022, Phillip Wood wrote: > >> From: Phillip Wood >> >> If the user suspends git while it is waiting for a keypress reset the >> terminal before stopping and restore the settings when git resumes. If >> the user tries to resume in the background print an error >> message (taking care to use async safe functions) before stopping >> again. Ideally we would reprint the prompt for the user when git >> resumes but this patch just restarts the read(). >> >> The signal handler is established with sigaction() rather than using >> sigchain_push() as this allows us to control the signal mask when the >> handler is invoked and ensure SA_RESTART is used to restart the >> read() when resuming. > > This description makes sense. From my understanding of signals, the code > also does make sense, but it is unfortunate that it has to be so much code > to implement something as straight-forward as suspend/resume. Yes it is a lot of code. It would be a bit simpler if we omitted the warning about resuming in the background but I think that is worth having. There's also a lot of changing signal masks to avoid stopping twice if the user presses ^Z a second time while the signal handler is active. > FWIW I tested the `add -p` command with these patches on Windows and it > still works as well as when I had developed it. Thanks for testing this on Windows, I don't think we have any meaningful test coverage for interactive.singlekey and it is probably tricky to add because it relies on having a tty. Best Wishes Phillip > Thank you, > Dscho