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

Stop all threads and write ANSI-Reset on Ctrl-C #2727

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

th1000s
Copy link

@th1000s th1000s commented Feb 1, 2024

On Ctrt-C stop all threads currently writing to the terminal, ignoring whatever they have locked, and write the ANSI-Reset code directly to the terminal, then exit.

This uses pthread_kill() or SuspendThread(), then libc::write() or WriteConsoleA() on Unix or Windows, respectively.


So far only tested and enabled on Linux, macOS and Windows (cmd.exe, Powershell, Mingw64), but I am sure this will
work on the BSDs and several others which have pthread_kill() - but I'd rather have someone confirm this first.

And to compare the before/after, running rg with a -j of 7, 11, 22, 33.. etc. will disable this handler.

@th1000s
Copy link
Author

th1000s commented Feb 1, 2024

When suggesting to someone to use rg instead of "legacy" grep -R .. | grep -v -e .git -e build or git grep ..
on the very first try (maybe spooked by the speed of the output) after ^C the terminal remained blood red and I had a slightly embarrassed look on my face.

A lot of users have shells or prompts which handle this automatically. But now that rg is packaged by most Linux distros, more users which don't customize their shells use it, and first impressions matter as to not end up as "newfangled nonsense" :)

@BurntSushi
Copy link
Owner

I've only skimmed this PR so far, but there is a ton of code here. And it also introduces a new dependency on winapi, which I hope to be getting rid of soon.

Separate from that, ripgrep used to have handling like this, but it ended up offering a worse user experience. Namely, ^C wouldn't always terminate ripgrep. And I believe it worked very poorly on Windows.

A lot of users have shells or prompts which handle this automatically.

I just use a simple little script to fix coloring if and when it happens (which is rare): https://github.com/BurntSushi/dotfiles/blob/eb14900839a210f31b826f0ab070506528ea6a99/bin/reset-term

But now that rg is packaged by most Linux distros

This is not a recent development. This has been true for years.

On Ctrl-C stop all threads currently writing to the terminal, ignoring
whatever they have locked, and write the ANSI-Reset code directly
to the terminal, then exit.

This uses `pthread_kill()` or `SuspendThread()`, then `libc::write()`
or `WriteConsoleA()` on Unix or Windows, respectively.
@th1000s
Copy link
Author

th1000s commented Feb 2, 2024

winapi -> windows-sys

Both crates work fine, I added a commit which uses windows-sys instead.

Separate from that, ripgrep used to have handling like this, but it ended up offering a worse user experience. Namely, ^C wouldn't always terminate ripgrep. And I believe it worked very poorly on Windows.

I've seen the previous behavior, this is fixed in this PR. It does this by (safely) ignoring any held locks which could slow down a fast exit, so a few syscalls have to be issued directly.

[rg packaged by most Linux distros is] not a recent development. This has been true for years.

I must be thinking in Debian releases, there it has only been included for one or two releases I think :)

@BurntSushi
Copy link
Owner

ripgrep has been in Debian for years.

I'm not sure if I'm going to take this. It is so much code to solve an issue that I don't think is a big deal.

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.

None yet

2 participants