[Pistonight]

(LLM assisted)
Fix several terminal UI issues:

• In non-watch mode, Ctrl-C leaves the terminal in echo off
  • open question: should Ctrl-C raise KeyBoardInterrupt (current behavior), or should it exit cleanly like what watch mode does?
• In watch mode, SIGKILL stops less without cleaning up terminal
• Flickering between stopping less and running it again. This is even more obvious with the above fixed (using SIGTERM instead of SIGKILL which cleans up the terminal). Fixed by handling alternate screen ourselves once at start and tears it down once before exit and call less without alternate screen.

Tests:

• Added unit test for terminal behavior
• Added a manual test to check flickering
• Probed behavior of less for every Ubuntu LTS version since 20.04 to ensure what we depend on (such as the terminal setup when calling with -X) is consistent. I didn't actually run the tests in any of those versions though, only the less I have (710)

[simonlindholm]

How are you killing it?

Why are the changes to run_sync not needed in the -w case?

Does terminate() vs kill() affect refresh performance? I feel like that's why I chose kill in the first place

[Pistonight]

I will do more testing and get back

[Pistonight]

Ahh I remembered, the changes to run_sync was to handle Ctrl-C without -w, the terminal will get stuck in a state where inputs aren't visible. In the -w case, main_late already wraps the call to run_async with except KeyboardInterrupt.

I haven't tested the performance question yet

[Pistonight]

I made a (somewhat hacky) benchmark with LLM. The result is kill/terminate doesn't make a big difference in terms of refresh performance, but my change actually makes it faster with terminate() even though kill() is faster at killing the process.

The reason is the change from ret != 0 to ret < 0. SIGTERM is caught and less exits normally with code 15, whereas SIGKILL would report -15, but since SIGTERM cleans up the terminal normally, the tput reset can be skipped, which made it overall faster.

However the difference (considering the tput reset) is only 2-3 ms, which is insignificant to the 100ms fs watch debounce interval and objdump which also takes tens of ms.

Another question is if run_sync should raise KeyboardInterrupt. Right now -w mode captures it and exits normally. My change raises it again after cleaning up to keep existing behavior, but maybe it should also just exit cleanly

[simonlindholm]

Yeah, exiting cleanly is probably nicer.

Nice to hear the absolute perf numbers are better. But just to double-check, because it's what I was really worrying about: does it add visual flickering?

[Pistonight]

Yes it does flicker, let me see what I can do about it.. I didn't catch this because I was running inside neovim's terminal which has a very low refresh rate :<

Even kill flickers but it only flickers to blank instead of whatever was in the terminal before the alternate screen launches which is a little bit better

[Pistonight]

Proof of concept that worked: Run less with -X instead of -+X, it will do all the terminal setup except for alternate screen. Then we can do our own alternate screen handling in watch mode in Python. During the update, less still exits normally, but won't erase the screen or exit alternate screen, so this should eliminate all flickering

@simonlindholm are you ok with this path? It does add ~ 100 lines of code for handling the alternate screen and Ctrl-Z suspend signals ourselves