Skip to content

fix -w clears or messes up terminal - #227

Open
Pistonight wants to merge 2 commits into
simonlindholm:mainfrom
Pistonight:kill_less_fix
Open

Pistonight wants to merge 2 commits into
simonlindholm:mainfrom
Pistonight:kill_less_fix

Conversation

@Pistonight

Copy link
Copy Markdown

When killing less, terminal gets messed up so there's no way to restore it other than clearing it completely, which is a bit annoying.

@simonlindholm

Copy link
Copy Markdown
Owner

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

Copy link
Copy Markdown
Author

I will do more testing and get back

@Pistonight

Copy link
Copy Markdown
Author

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

Pistonight commented Nov 10, 2026 •

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Owner

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

Pistonight commented Nov 10, 2026 •

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Author

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

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.

2 participants