jonas/tig

macOS search ('/') is not <Ctrl-C> cancellable

landonb opened this issue · 3 comments

landonb commented

On macOS, when I start a search (using '/'), I cannot cancel out of it.

  • Pressing <Ctrl-C> does nothing.

It seems like the readline call ignores SIGINT.

The only way I can return to the main view is to press <Enter> (and to run the search).

  • Interestingly, I can kill -s 2 $(pgrep tig) from another terminal window, and that will cancel the search.

I do not experience this issue on Linux Mint MATE.

OS and tig versions

The problem affects both Brew tig and my own tig on macOS Sonoma 14.5:

  @macOS $ /opt/homebrew/Cellar/tig/2.5.10/bin/tig --version
  tig version 2.5.10
  ncursesw version 6.5.20240427
  readline version 8.2
  PCRE2 version 10.44 2024-06-07

Here's my own build, using the latest commit (and the latest readline and ncurses from Brew):

  @macOS $ tig --version
  tig version 2.5.10-4-g37ccb67e
  ncursesw version 6.5.20240427
  readline version 8.2
  PCRE2 version 10.44 2024-06-07
  • I can attach my configure and build commands if you'd like to see them.

I do not see the problem on Linux Mint MATE 21.4 21.3, also running the latest commit, albeit not the same readline (though it's the latest from apt):

  @linuxmint $ tig --version
  tig version 2.5.10
  ncursesw version 6.3.20211021
  readline version 8.1
  PCRE2 version 10.39 2021-10-29

(Limited) diagnosis

I am somewhat surprised that I didn't find an existing issue regarding this, but I looked.

  • I also don't think there's anything unique about my host. I can send <Ctrl-C> to the terminal itself and to other apps, and <Ctrl-C> works from tig main view.

I also haven't debugged C code in eons, and I'm not that familiar with readline sources, but I tracked it down to the prompt.c realine call

  line = readline(prompt);
  • Or at least that's the call that blocks until <Enter>, and the sigint_absorb_handler registered above it never gets called.

Poking around some more, I fixed the problem by disabling these two lines:

	rl_deprep_term_function = NULL;
	rl_prep_term_function = NULL;

Those two lines were introduced a while back, 2019-05-02: Piping to tig segfaults on exit #893 / ea43f8be22c7

  • I've only sparingly used macOS since about 2020 (for work), until migrating my personal dev environment onto a new Mac this past spring (that I've started using full-time). But if memory serves, I think this has always been a problem for me... though I'd be happy to checkout and build previous versions if anyone thinks this used to work.

My solution (or the best I could come up with), is to exclude those two lines on macOS builds (#ifndef __APPLE__). Though I'm not saying this is the best solution, just that it appears to work for me.

I also glanced at the readline sources but didn't feel any more or less confident about the fix. There's also docs on the two functions in question:

Furthermore, there are (only) 69 code hits on GitHub for rl_prep_term_function = NULL;:

https://github.com/search?q=%22rl_prep_term_function+%3D+NULL%3B%22&type=code

though a lot of those hits are for the original readline.c, and for tig forks. A few other hits do reference disabling those functions to avoid conflicting with ncurses, but those sources I looked at also disable readline signals (rl_catch_signals = 0).

  • Point being, nulling those functions doesn't seem very common.

  • Though I'm not a readline nor ncurses expert by any means.

Test results

I tested the error mentioned in #893, and it works for me:

echo | tig
:q

I also ran make test on both my branch and the upstream branch, and I see the same results:

   SUMMARY  Failed 56 of 558 assertions in 145 tests (2 skipped)
  • Though it looks like most/all errors have to do with the same 'master' vs. 'main' discrepancy, e.g.:
   $ make test
   ...
   CASE  test/main/emoji-test:emoji-commit-titles-col-300
         | Failed 3 out of 6 test(s)
         | [FAIL] emoji-commit-titles-col-46.screen != expected/emoji-commit-titles-col-46.screen
         | diff --git a/expected/emoji-commit-titles-col-46.screen b/emoji-commit-titles-col-46.screen
         | index 4e7e712..af40a99 100644
         | --- a/expected/emoji-commit-titles-col-46.screen
         | +++ b/emoji-commit-titles-col-46.screen
         | @@ -1,4 +1,4 @@
         | -2009-04-06 01:44 +0000 Committer o [master] 🌏
         | +2009-04-06 01:44 +0000 Committer o [main] 🌏💧
  ...
  • This was my first time running make test, and I didn't investigate further.

Finally, while I only this afternoon made this branch, I'll continue to run it.

  • So far, it appears to work well, I can <Ctrl-C> to cancel search, search again, <Ctrl-C> again, etc., and the prompt seems to behave well.

  • The only oddity I've seen is if you press <Esc> during search, in interprets the next character as part of an escape sequence.

    • E.g., if you start a search '/', type 'foo', then press <Esc>, then b, it moves the cursor back to the first column.

    • Likewise, <Esc> then w deletes to the start of the line.

    • This is similar to normal tig behavior, as far as I can tell, except when <Esc> is followed by <Ctrl-C>:

      • On Brew tig, nothing happens; the next character you type is echoed to the search line.

      • On Linux tig, after <Esc> and <Ctrl-C>, nothing happens until you press any key, and then the search is cancelled.

        • Unless you press <Ctrl-C> again, then tig itself exits.

      • Using my branch on macOS is similar to Linux, but not exact:

        • After <Esc> and <Ctrl-C>, a ^C is immediately echoed.

        • Then it behaves like Linux: if you press <Ctrl-C> again, tig exits; but if you press any other key, the search is cancelled and you return to the main view.

    • (Really the only reason I ever hit <Esc> is when I cannot cancel search using <Ctrl-C>, and then I mash <Esc> out of frustration, hoping that'll work. But now the <Ctrl-C> works, I'll probably stop hitting <Esc>. Also because I now realize readline interprets it as the start of an escape sequence.)

Given all that, I think my branch is an acceptable solution.

  • Though I'd feel better if another Mac user could vouch for the original issue.

  • Also if someone who knows readline more thoroughly could vouch for the proposed solution...

Thanks for reading! A little in-depth, but I wanted to do a thorough diagnosis.

landonb commented

Please see PR #1348

koutcher commented

Thanks for you detailed report. By checking the existing PRs you could have found #1342 was already dealing with this issue. Fixing the remaining annoyances you mentioned will require quite some work to switch to the alternate readline interface and for a somehow limited benefit.

landonb commented

Thanks for your kind response, @koutcher!

So embarrasing! 🤦 I could've sworn I had fetched recently, but somehow I missed the commit from June 20. (Edit: Though checking ~/.bash_history, I did fetch and merge, so perhaps it just hadn't been pushed to GH yet... in any case, I'll check PRs more thoroughly next time)

Thanks for all your hard work on this project, it contends with Vim as my favorite OSS project.

Edit: I tested the June 20 commit and <Ctrl-C> now works to quit search ('/') (Thank you, @intelfx!)

Completely unrelated:

I have an extensive ~/.config/tig/config, but I'm not sure if there's a proper place to highlight community configs, or if that's something that's been discussed... but here's mine: https://github.com/DepoXy/tig-newtons