Author: jacob
Revision Date: 2014-10-21 11:33:33 UTC

Cross-reference the description of winadj@putty.projects.tartarus.org
to its bug-compatibility mode.

Author: Simon Tatham
Revision Date: 2023-09-03 09:20:58 UTC

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!)