@stefanhaller I’ve addressed the earlier issues you pointed out. I’m not sure if a test is needed for this change, or if there are any other steps you’d expect before merging. Let me know! 🙂

@cowboy8625 The behavior is still the same for me, as described in #3906 (comment). After typing bg, I get the "+ suspended (tty output)" error, and then fg hangs.

(We had a similar problem back when we used $SHELL -i for running custom shell commands; see #3903 (comment) for some discussion. Not sure how much of a connection there is between the two cases; just mentioning it.)

And finally, when I use make run to start lazygit, then ctrl-z doesn't work at all; lazygit does suspend, but the shell prompt doesn't appear. This may be only relevant for developers, not users, but the same could happen for users if they launch lazygit through a wrapper script. I suppose in this case we'd have to send the SIGTSTP to the wrapper script (or to the make process in my case), not to lazygit; but I'm not sure what the criterion for that is. Walk the chain of parent processes until we find one whose name matches $SHELL?

Until these problems are solved I don't feel we can merge this. But I'm also not sure how much time is justified for putting into this, to be honest.

@stefanhaller I was able to better handle the issue where running bg followed by fg would cause lazygit to hang.

Here’s how it looks now on macOS (Kitty terminal):

And here’s how Neovim behaves:

I also resolved the issue that happened when running via make run.
By looking at how Neovim handles suspension, I noticed they call kill(0, SIGSTOP) in their code:
https://github.com/neovim/neovim/blob/c3a4d125296caaf15ff424e7609e731a8b4d37e7/src/nvim/tui/tui.c#L1611-L1627

Previously, I was only sending a stop signal to the lazygit process itself, which doesn’t work properly if it’s launched inside a wrapper or make run.
Using kill(0, SIGSTOP) sends the signal to the whole process group, which solves this.

Thanks a lot for your help!

Nice, seems like we're getting somewhere with this. 😄

I had to make quite a few changes, and I took the opportunity to also restructure the commits to make them reviewable. I didn't want to force-push your master branch, so I pushed a new branch suspend-on-ctrl-z in this repo; you can hard-reset yours to that one after reviewing it.

A few notes about what I changed:

• we need a keybinding config; not because users may want to use a different key (that wouldn't make much sense), but because users might want to disable it
• we need to hook this into the normal keybinding mechanism; simply calling SetKeybinding like you did in your branch doesn't work, this gets erased again the next time you open a different repo or even just a menu
• I decided to handle the keybinding in GlobalController; unfortunately this requires some indirections to call things on Gui, which I'm not entirely happy about, but we do similar things all over the place. Another option would have been to handle it in GetInitialKeybindings, that way we would have been able to work with Gui directly; however, I really wanted the binding to appear right below 'Quit' in the keybindings menu, and handling it in GlobalController seems like the only way to achieve that.
• When you type ctrl-z on Windows, there's an error toast in the status bar.

During testing I still had one or two cases where lazygit didn't come back to the foreground after typing fg, and I had to kill it from another terminal tab. This makes me a little nervous, but it might also have been an intermediate state during refactoring; I can no longer reproduce it. (Shrug)

Yeah, this is a lot cleaner.

@stefanhaller Am I good to open this up for review? 😄

@stefanhaller Am I good to open this up for review? 😄

I'd say so, yes. I'll hold off merging this for now until after we released 0.54.1, which I expect to happen tomorrow or the day after; that's a hotfix (needed because of #4793), and I don't want to include any other changes there.

To be honest I don't really fully understand what we're doing here when we wake up, like what this /dev/tty business is or why we need it. I was trying a little bit to read up on what vim is doing in that case, and it makes me dizzy 😅.

But it seems to work well, so I'm fine with going with this for now.

To my knowledge this is the most reliable way to handle it. By opening /dev/tty, we can always access the controlling terminal regardless of how Lazygit is launched. Standard input/output might be redirected depending on how the program is run, so relying on them isn’t safe. Using /dev/tty ensures we can always call TIOCSPGRP to regain foreground status after receiving a SIGCONT.

@cowboy8625 Merging this at last. I rebased onto master for you to get rid of the "megred-from-master" commits that result in a very ugly commit history. For future PRs, please never merge master into a branch, always rebase onto master instead, thanks.

@cowboy8625 It seems we still have some sort of race condition here. Today I typed ctrl-z (just to check), and when typing fg it would hang there, not coming to the front, so I had to kill it from another terminal window. Then I tried again like 20 times until I got tired, and it worked fine every time.

Would be really nice if we could make this more robust, but without a better reproduction recipe there may not be much we can do. Just letting you know.