From 611fe37882414578fe31d3d13c8b0087ca7257b8 Mon Sep 17 00:00:00 2001 From: Chris Antos Date: Fri, 16 Sep 2022 12:43:33 -0700 Subject: [PATCH] Improve terminal resize. 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. --- clink/lib/include/lib/display_readline.h | 17 +++++ clink/lib/src/display_readline.cpp | 83 ++++++++++++++++++++++++ clink/lib/src/rl/rl_module.cpp | 69 ++++++++++++++------ clink/terminal/src/termcap.cpp | 1 + 4 files changed, 151 insertions(+), 19 deletions(-) diff --git a/clink/lib/include/lib/display_readline.h b/clink/lib/include/lib/display_readline.h index 1f01b37d6..b340c24a4 100644 --- a/clink/lib/include/lib/display_readline.h +++ b/clink/lib/include/lib/display_readline.h @@ -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; +}; diff --git a/clink/lib/src/display_readline.cpp b/clink/lib/src/display_readline.cpp index 745921cae..386f771f2 100644 --- a/clink/lib/src/display_readline.cpp +++ b/clink/lib/src/display_readline.cpp @@ -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 { @@ -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 = ""; @@ -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(); } diff --git a/clink/lib/src/rl/rl_module.cpp b/clink/lib/src/rl/rl_module.cpp index 5e9e1f50a..dbd35ec65 100644 --- a/clink/lib/src/rl/rl_module.cpp +++ b/clink/lib/src/rl/rl_module.cpp @@ -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) @@ -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; } @@ -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; } @@ -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; @@ -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; @@ -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)) }; @@ -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(); diff --git a/clink/terminal/src/termcap.cpp b/clink/terminal/src/termcap.cpp index 3634b15f3..0c686254f 100644 --- a/clink/terminal/src/termcap.cpp +++ b/clink/terminal/src/termcap.cpp @@ -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;