wtarreau / bootterm

The terminal written for its users by its users
MIT License
215 stars 11 forks source link

src: inform user about current bt mode #8

Closed renard closed 3 years ago

renard commented 3 years ago

This patch allows to notify user that bt enters in escape mode. This is done by issuing a visual bell and by hiding the cursor (prevents user from guessing what current mode is). This helps to inform user about escape mode without polluting the terminal output.

This feature is only activated when output is a tty to prevent from altering captured data.

Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>

wtarreau commented 3 years ago

OK looks good overall, however let's not disable the cursor now that you could inverse the video, since it will make re-appear in menu-based application that already disable it. If you're OK I can simply remove that part from your patch. I also think that it would be nice to slightly rename the functions so that they mention it's only about notification but that they don't change the mode themselves (e.g. notify_escape_mode_in/out or so).

renard commented 3 years ago

Yep I do agree let me a couple of hours to get a clear vision of what we really need her ;-)

Code is almost ready for that.

wtarreau commented 3 years ago

there's no emergency, tomorrow will work as well if it's a fresher day for you :-)

wtarreau commented 3 years ago

So I could finally give it a try. Some cases are not properly handled. For example the cursor remains disabled after a break or a DTR state change. In addition the delay during the blinking seems to occasionally cause some characters to be lost (at 115200 bps, a 100ms pause corresponds to 1152 chars which will easily overflow some device buffers).

Initially I was thinking about inverting the screen and keeping it inverted in escape mode, but I'm realizing that if we later add more features like file transfer, this inversion will become a pain. So possibly that just doing the cursor thing would be enough, I don't know :-/

wtarreau commented 3 years ago

Even the cursor is still causing pain. When you start a full-screen, windowed application (e.g. a hex editor or so), it re-enables the cursor that the application had disabled.

wtarreau commented 3 years ago

Probably that a better approach in the end, would be to either automatically quit escape mode after 1 or 2 seconds, or to wait a few seconds then print on the screen that we're in escape mode and that a keypress is expected (or print the help).

wtarreau commented 3 years ago

After a bit more thinking, I think that one of the least intrusive yet convenient behaviors would be to have a 2-second timeout armed after entering escape mode, and that automatically goes back to normal mode and emits a flash to remind that something was indeed started and aborted. And I even recall some old versions of VI doing more or less this, I think it was when entering escape mode when escape was not followed by any key for 1 second or so.

wtarreau commented 3 years ago

So I finally completed what I proposed above and I think it serves the purpose well. I personally find the two second delay a bit long but shorter might be too short for new users who need to think a moment before figuring the right key to press. The flash is visible enough to show that something was aborted, and now you don't have the risk to remain in escape mode anymore without being aware of it.

Please have a look, try it and let me know so that we can close these two PRs.

renard commented 3 years ago

Yep works nicely. I'm still wondering if it should also blink when user hits Ctrl-[ for the second time or more generally when escape mode is exited.

wtarreau commented 3 years ago

No, really, because I tried that (which was your initial proposal) and found it confusing and really painful. Flashes or beeps are usually employed to notify about errors or alerts, and here that systematically makes the user think he did a mistake or some action was ignored or aborted, while it's the exact opposite, everything went as expected.

wtarreau commented 3 years ago

Now closing since a reasonably close solution was implemented. Thanks for the brainstorming and for chasing the ANSI codes I was looking for.