mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-01-09 01:18:00 +00:00
Rationalise the code that resets terminal scrollback.
Recently I encountered a CLI tool that took tens of seconds to run, and produced no _visible_ output, but wrote ESC[0m to the terminal a few times during its operation. (Probably by mistake. In other modes it does print colourful messages, so I expect a 'reset colour' call was accidentally outside the 'if' statement containing the rest of the diagnostic it followed. Or something along those lines.) I noticed this because every ESC[0m reset my pterm scrollback to the bottom, which wasn't very helpful, and was unintentional on pterm's part (as _well_ as on the part of the tool). But I can fix pterm! At first glance the code _looked_ sensible: terminal.c contains calls to seen_disp_event(term) whenever terminal output does something that requires a redraw of the terminal window. Those are also the updates that should count as 'reset scrollback on display activity'. And ESC[0m, along with the rest of the SGR handler, correctly contained no such call. So how did a display update happen at all? The code was confusingly tangled up with the code that responds to terminal activity by resetting the phase of the blinking cursor (if any). term_reset_cblink() was calling seen_disp_event() (when surely it should be the other way round!), and also, term_reset_cblink() was called whenever _any_ terminal output data arrived. That combination meant that any byte output to the terminal at all turned out to count as display activity, whether or not it changed the screen contents. Additionally, the other scrollback-reset flag, 'reset scrollback on keypress', was handled by calling seen_disp_event() from the keyboard handler. But display events and keyboard events are supposed to be _independent_ potential causes of scrollback resets - it doesn't make any sense to handle one by treating it as the other! So I've reorganised the code completely: - the seen_disp_event *flag* is now gone. Instead, the seen_disp_event function tests the scroll_on_disp flag, and if set, resets the scroll position immediately and sets the general 'scrollbar needs updating' flag. - keyboard input is handled by doing exactly the same thing except testing the scroll_on_key flag, so the two systems are properly independent. That code calls term_schedule_update so that the terminal will be redrawn as a result of the scroll, but doesn't also call seen_disp_event() for the rest of the full treatment. - the term_update code that does the scrollbar update is much simpler, since now it only needs to test that one flag. - I also had to set that flag explicitly in scroll() so that the scrollbar would still be updated as a result of the scrollback size changing. I think that must have been happening entirely by accident before. - term_reset_cblink is subsumed into seen_disp_event, so that only _substantive_ display updates cause the cursor blink phase to reset to the start of the solid period. Result: if programs output no-op sequences like ESC[0m, or if you press keys that don't echo, then the cursor will carry on blinking normally, and (if you don't also have scroll_on_key set) the scrollback won't be reset. And the code is slightly shorter than it was before, and hopefully more sensible too. (However, other classes of no-op activity _will_ still cause a cursor blink phase change and a scrollback reset, such as sending a cursor-positioning sequence that puts the cursor in the same place it was already - even something as simple as ^M when already at the start of the line. It might be nice to fix that, but it's much more difficult: you'd have to either put a complicated and error-prone test at every seen_disp_event call site, or else expensively diff the entire visible terminal state against how it was before. And to avoid a nondeterministic dependency on the terminal update cooldown, that diff would have to be done at the granularity of individual control sequences rather than a bounded number of times a second. I'd rather not!)
This commit is contained in:
parent
bd27962cd9
commit
bdbd5f429c
@ -1342,12 +1342,21 @@ static void term_schedule_update(Terminal *term)
|
||||
}
|
||||
|
||||
/*
|
||||
* Call this whenever the terminal window state changes, to queue
|
||||
* an update.
|
||||
* Call this whenever the terminal window state changes, to queue an
|
||||
* update. This also resets the phase of cursor blinking, so that the
|
||||
* cursor remains visible as it moves with the output, and sets a flag
|
||||
* to indicate that if we have the 'reset scrollback on display
|
||||
* activity' setting enabled, then we should activate it.
|
||||
*/
|
||||
static void seen_disp_event(Terminal *term)
|
||||
{
|
||||
term->seen_disp_event = true; /* for scrollback-reset-on-activity */
|
||||
if (term->scroll_on_disp) {
|
||||
term->disptop = 0;
|
||||
term->win_scrollbar_update_pending = true;
|
||||
}
|
||||
term->cblinker = true;
|
||||
term->cblink_pending = false;
|
||||
term_schedule_cblink(term);
|
||||
term_schedule_update(term);
|
||||
}
|
||||
|
||||
@ -1382,17 +1391,6 @@ static void term_schedule_cblink(Terminal *term)
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Call to reset cursor blinking on new output.
|
||||
*/
|
||||
static void term_reset_cblink(Terminal *term)
|
||||
{
|
||||
seen_disp_event(term);
|
||||
term->cblinker = true;
|
||||
term->cblink_pending = false;
|
||||
term_schedule_cblink(term);
|
||||
}
|
||||
|
||||
/*
|
||||
* Call to begin a visual bell.
|
||||
*/
|
||||
@ -1559,17 +1557,10 @@ void term_update(Terminal *term)
|
||||
}
|
||||
|
||||
if (win_setup_draw_ctx(term->win)) {
|
||||
bool need_sbar_update = term->seen_disp_event ||
|
||||
term->win_scrollbar_update_pending;
|
||||
term->win_scrollbar_update_pending = false;
|
||||
if (term->seen_disp_event && term->scroll_on_disp) {
|
||||
term->disptop = 0; /* return to main screen */
|
||||
term->seen_disp_event = false;
|
||||
need_sbar_update = true;
|
||||
}
|
||||
|
||||
if (need_sbar_update)
|
||||
if (term->win_scrollbar_update_pending) {
|
||||
term->win_scrollbar_update_pending = false;
|
||||
update_sbar(term);
|
||||
}
|
||||
do_paint(term);
|
||||
win_set_cursor_pos(
|
||||
term->win, term->curs.x, term->curs.y - term->disptop);
|
||||
@ -1602,9 +1593,10 @@ void term_seen_key_event(Terminal *term)
|
||||
/*
|
||||
* Reset the scrollback on keypress, if we're doing that.
|
||||
*/
|
||||
if (term->scroll_on_key) {
|
||||
term->disptop = 0; /* return to main screen */
|
||||
seen_disp_event(term);
|
||||
if (term->scroll_on_key && term->disptop != 0) {
|
||||
term->disptop = 0;
|
||||
term->win_scrollbar_update_pending = true;
|
||||
term_schedule_update(term);
|
||||
}
|
||||
}
|
||||
|
||||
@ -2666,6 +2658,12 @@ static void scroll(Terminal *term, int topline, int botline,
|
||||
*/
|
||||
if (term->disptop > -term->savelines && term->disptop < 0)
|
||||
term->disptop--;
|
||||
|
||||
/*
|
||||
* We've just modified the data that the terminal's
|
||||
* scrollbar is based on, so remember to update it.
|
||||
*/
|
||||
term->win_scrollbar_update_pending = true;
|
||||
}
|
||||
resizeline(term, line, term->cols);
|
||||
clear_line(term, line);
|
||||
@ -7756,7 +7754,6 @@ static void term_added_data(Terminal *term, bool called_from_term_data)
|
||||
{
|
||||
if (!term->in_term_out) {
|
||||
term->in_term_out = true;
|
||||
term_reset_cblink(term);
|
||||
term_out(term, called_from_term_data);
|
||||
term->in_term_out = false;
|
||||
}
|
||||
|
@ -146,7 +146,6 @@ struct terminal_tag {
|
||||
long vbell_end;
|
||||
bool app_cursor_keys, app_keypad_keys, vt52_mode;
|
||||
bool repeat_off, srm_echo, cr_lf_return;
|
||||
bool seen_disp_event;
|
||||
bool big_cursor;
|
||||
|
||||
bool xterm_mouse_forbidden;
|
||||
|
Loading…
Reference in New Issue
Block a user