Skip to content

Commit

Permalink
Improve terminal resize.
Browse files Browse the repository at this point in the history
Buffer all output from terminal resize and/or readline display into a
single OS output call.  This avoids race conditions where the OS resizes
the terminal window asynchronously in between smaller output calls,
potentially causing conflicts between concurrent wrapping and cursor
movement.
  • Loading branch information
chrisant996 committed Sep 16, 2022
1 parent 5e564c4 commit 611fe37
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 19 deletions.
17 changes: 17 additions & 0 deletions clink/lib/include/lib/display_readline.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,20 @@
extern void reset_readline_display();
extern void display_readline();
extern unsigned int get_readline_display_top();

//------------------------------------------------------------------------------
class display_accumulator
{
public:
display_accumulator();
~display_accumulator();
void flush();
private:
void restore();
static void fwrite_proc(FILE*, const char*, int);
static void fflush_proc(FILE*);
void (*m_saved_fwrite)(FILE*, const char*, int) = nullptr;
void (*m_saved_fflush)(FILE*) = nullptr;
bool m_active = false;
static int s_nested;
};
83 changes: 83 additions & 0 deletions clink/lib/src/display_readline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,85 @@ display_line* display_lines::next_line(unsigned int start)



//------------------------------------------------------------------------------
int display_accumulator::s_nested = 0;
static str_moveable s_buf;

//------------------------------------------------------------------------------
display_accumulator::display_accumulator()
{
assert(!m_saved_fwrite);
assert(!m_saved_fflush);
assert(rl_fwrite_function);
assert(rl_fflush_function);
assert(s_nested || s_buf.empty());
m_saved_fwrite = rl_fwrite_function;
m_saved_fflush = rl_fflush_function;
m_active = true;
rl_fwrite_function = fwrite_proc;
rl_fflush_function = fflush_proc;
++s_nested;
}

//------------------------------------------------------------------------------
display_accumulator::~display_accumulator()
{
if (m_active)
{
if (s_nested > 1)
restore();
else
flush();
assert(!m_active);
}
--s_nested;
}

//------------------------------------------------------------------------------
void display_accumulator::flush()
{
if (s_nested > 1)
return;

restore();

if (s_buf.length())
{
rl_fwrite_function(_rl_out_stream, s_buf.c_str(), s_buf.length());
rl_fflush_function(_rl_out_stream);
s_buf.clear();
}
}

//------------------------------------------------------------------------------
void display_accumulator::restore()
{
assert(m_active);
assert(m_saved_fwrite);
assert(m_saved_fflush);
rl_fwrite_function = m_saved_fwrite;
rl_fflush_function = m_saved_fflush;
m_saved_fwrite = nullptr;
m_saved_fflush = nullptr;
m_active = false;
}

//------------------------------------------------------------------------------
void display_accumulator::fwrite_proc(FILE* out, const char* text, int len)
{
assert(out == _rl_out_stream);
dbg_ignore_scope(snapshot, "display_readline");
s_buf.concat(text, len);
}

//------------------------------------------------------------------------------
void display_accumulator::fflush_proc(FILE*)
{
// No-op, since the destructor automatically flushes.
}



//------------------------------------------------------------------------------
class display_manager
{
Expand Down Expand Up @@ -814,6 +893,8 @@ void display_manager::display()
_rl_block_sigint();
RL_SETSTATE(RL_STATE_REDISPLAYING);

display_accumulator coalesce;

if (!rl_display_prompt)
rl_display_prompt = "";

Expand Down Expand Up @@ -1055,6 +1136,8 @@ void display_manager::display()
if (!_rl_rprompt_shown_len && can_show_rprompt)
tputs_rprompt(rl_rprompt);

coalesce.flush();

RL_UNSETSTATE(RL_STATE_REDISPLAYING);
_rl_release_sigint();
}
Expand Down
69 changes: 50 additions & 19 deletions clink/lib/src/rl/rl_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ extern "C" {
extern int find_streqn (const char *a, const char *b, int n);
extern void rl_replace_from_history(HIST_ENTRY *entry, int flags);
extern int _rl_get_inserted_char(void);
extern char* tgetstr(const char*, char**);
extern int tputs(const char* str, int affcnt, int (*putc_func)(int));
extern char* tgoto(const char* base, int x, int y);
extern Keymap _rl_dispatching_keymap;
#define HIDDEN_FILE(fn) ((fn)[0] == '.')
#if defined (COLOR_SUPPORT)
Expand Down Expand Up @@ -750,13 +753,14 @@ static void terminal_write_thunk(FILE* stream, const char* chars, int char_count
}

//------------------------------------------------------------------------------
static int s_puts_face = 0;
static void terminal_log_write(FILE* stream, const char* chars, int char_count)
{
if (stream == out_stream)
{
assert(g_printer);
LOGCURSORPOS();
LOG("RL_OUTSTREAM \"%.*s\", %d", char_count, chars, char_count);
LOG("%s \"%.*s\", %d", s_puts_face ? "PUTSFACE" : "RL_OUTSTREAM", char_count, chars, char_count);
g_printer->print(chars, char_count);
return;
}
Expand Down Expand Up @@ -1010,13 +1014,9 @@ static void puts_face_func(const char* s, const char* face, int n)
if (cur_face != '0')
out.concat(c_normal);

if (g_debug_log_terminal.get())
{
LOGCURSORPOS();
LOG("PUTSFACE \"%.*s\", %d", out.length(), out.c_str(), out.length());
}

g_printer->print(out.c_str(), out.length());
++s_puts_face;
rl_fwrite_function(_rl_out_stream, out.c_str(), out.length());
--s_puts_face;
}


Expand Down Expand Up @@ -2994,12 +2994,17 @@ void rl_module::on_terminal_resize(int columns, int rows, const context& context
// can start a new prompt and overwrite the old one.

#ifdef NO_READLINE_RESIZE_TERMINAL
CONSOLE_SCREEN_BUFFER_INFO csbi;
HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
GetConsoleScreenBufferInfo(h, &csbi);
refresh_terminal_size();
// The console size may have changed asynchronously, so reset columns to
// ensure consistent measurements.
columns = _rl_screenwidth;
#endif

display_accumulator coalesce;

int remaining = columns;
int line_count = 1;

Expand All @@ -3013,14 +3018,32 @@ void rl_module::on_terminal_resize(int columns, int rows, const context& context
case ecma48_code::type_chars:
for (str_iter i(code.get_pointer(), code.get_length()); i.more(); )
{
int n = clink_wcwidth(i.next());
remaining -= n;
if (remaining > 0)
continue;

++line_count;

remaining = columns - ((remaining < 0) << 1);
int n;
const char* chars = code.get_pointer();
const int c = i.next();
if (CTRL_CHAR(*chars) || *chars == RUBOUT)
{
// Control characters.
n = 2;
remaining -= n;
while (remaining <= 0)
{
remaining += columns;
++line_count;
}
}
else
{
n = clink_wcwidth(c);
remaining -= n;
if (remaining <= 0)
{
if (remaining == 0)
n = 0;
remaining = columns - n;
++line_count;
}
}
}
break;

Expand Down Expand Up @@ -3067,9 +3090,7 @@ void rl_module::on_terminal_resize(int columns, int rows, const context& context
int cursor_line = line_count - 1;

// Move cursor to where the top line should be.
CONSOLE_SCREEN_BUFFER_INFO csbi;
HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
GetConsoleScreenBufferInfo(h, &csbi);
#if 0
#ifndef NO_READLINE_RESIZE_TERMINAL
int delta = _rl_last_v_pos - cursor_line;
COORD new_pos = { 0, SHORT(clamp(csbi.dwCursorPosition.Y - delta, 0, csbi.dwSize.Y - 1)) };
Expand All @@ -3079,6 +3100,16 @@ void rl_module::on_terminal_resize(int columns, int rows, const context& context
SetConsoleCursorPosition(h, new_pos);
if (new_pos.Y < csbi.srWindow.Top)
ScrollConsoleRelative(h, new_pos.Y, SCR_ABSOLUTE);
#else
if (cursor_line > 0)
{
char *tmp = tgoto(tgetstr("UP", nullptr), 0, cursor_line);
tputs(tmp, 1, _rl_output_character_function);
}
_rl_cr();
#endif
_rl_last_v_pos = 0;
_rl_last_c_pos = 0;

// Clear to end of screen.
reset_readline_display();
Expand Down
1 change: 1 addition & 0 deletions clink/terminal/src/termcap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ char* tgetstr(const char* name, char** out)
case 'nd': str = CSI(C); break;
case 'ND': str = CSI(%dC); break;
case 'up': str = CSI(A); break;
case 'UP': str = CSI(%dA); break;

// Cursor style
case 've': str = c_default_term_ve; break;
Expand Down

0 comments on commit 611fe37

Please sign in to comment.