From fc42516dfbb860faa07dcc641600f11643f40b68 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 7 Jun 2020 14:53:42 -0700 Subject: [PATCH] Unwind some calls to common_get_width from inside screen common_get_width will "lazily" decide the screen width, which means changing the environment variable stack. This is a surprising thing to do from the middle of screen rendering. Switch to passing in widths explicitly to screen. --- src/reader.cpp | 17 +++++++++-------- src/screen.cpp | 36 ++++++++++++++++++------------------ src/screen.h | 19 ++++++++++++------- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/reader.cpp b/src/reader.cpp index 3fa37496f..152e06dd0 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -799,8 +799,9 @@ void reader_data_t::repaint() { size_t cursor_position = focused_on_pager ? pager.cursor_position() : cmd_line->position(); // Prepend the mode prompt to the left prompt. - s_write(&screen, mode_prompt_buff + left_prompt_buff, right_prompt_buff, full_line, - cmd_line->size(), colors, indents, cursor_position, current_page_rendering, + int screen_width = common_get_width(); + s_write(&screen, screen_width, mode_prompt_buff + left_prompt_buff, right_prompt_buff, + full_line, cmd_line->size(), colors, indents, cursor_position, current_page_rendering, focused_on_pager); repaint_needed = false; @@ -974,7 +975,7 @@ void reader_data_t::repaint_if_needed() { if (needs_reset) { exec_prompt(); - s_reset(&screen, screen_reset_mode_t::current_line_and_prompt); + s_reset(&screen, common_get_width(), screen_reset_mode_t::current_line_and_prompt); screen_reset_needed = false; } @@ -2327,7 +2328,7 @@ void reader_pop() { reader_interactive_destroy(); } else { s_end_current_loop = false; - s_reset(&new_reader->screen, screen_reset_mode_t::abandon_line); + s_reset(&new_reader->screen, common_get_width(), screen_reset_mode_t::abandon_line); } } @@ -2666,7 +2667,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat // elsewhere, we detect if the mode output is empty. exec_mode_prompt(); if (!mode_prompt_buff.empty()) { - s_reset(&screen, screen_reset_mode_t::current_line_and_prompt); + s_reset(&screen, common_get_width(), screen_reset_mode_t::current_line_and_prompt); screen_reset_needed = false; repaint(); break; @@ -2679,7 +2680,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat if (!rls.coalescing_repaints) { rls.coalescing_repaints = true; exec_prompt(); - s_reset(&screen, screen_reset_mode_t::current_line_and_prompt); + s_reset(&screen, common_get_width(), screen_reset_mode_t::current_line_and_prompt); screen_reset_needed = false; repaint(); } @@ -2962,7 +2963,7 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat // already be printed, all we need to do is repaint. wcstring_list_t argv(1, el->text()); event_fire_generic(parser(), L"fish_posterror", &argv); - s_reset(&screen, screen_reset_mode_t::abandon_line); + s_reset(&screen, common_get_width(), screen_reset_mode_t::abandon_line); mark_repaint_needed(); } @@ -3483,7 +3484,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { history_search.reset(); - s_reset(&screen, screen_reset_mode_t::abandon_line); + s_reset(&screen, common_get_width(), screen_reset_mode_t::abandon_line); event_fire_generic(parser(), L"fish_prompt"); exec_prompt(); diff --git a/src/screen.cpp b/src/screen.cpp index 39678b63e..e23ce17c2 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -459,7 +459,7 @@ static void s_check_status(screen_t *s) { // modeled cursor y-pos to its earlier value. int prev_line = s->actual.cursor.y; write_loop(STDOUT_FILENO, "\r", 1); - s_reset(s, screen_reset_mode_t::current_line_and_prompt); + s_reset(s, common_get_width(), screen_reset_mode_t::current_line_and_prompt); s->actual.cursor.y = prev_line; } } @@ -488,7 +488,7 @@ static void s_desired_append_char(screen_t *s, wchar_t b, highlight_spec_t c, in current.clear(); s->desired.cursor.x = 0; } else { - int screen_width = common_get_width(); + int screen_width = s->desired.screen_width; int cw = bwidth; s->desired.create_line(line_no); @@ -531,7 +531,7 @@ static void s_move(screen_t *s, int new_x, int new_y) { // If we are at the end of our window, then either the cursor stuck to the edge or it didn't. We // don't know! We can fix it up though. - if (s->actual.cursor.x == common_get_width()) { + if (s->actual.cursor.x == s->actual.screen_width) { // Either issue a cr to go back to the beginning of this line, or a nl to go to the // beginning of the next one, depending on what we think is more efficient. if (new_y <= s->actual.cursor.y) { @@ -617,7 +617,7 @@ static void s_write_char(screen_t *s, wchar_t c, size_t width) { scoped_buffer_t outp(*s); s->actual.cursor.x += width; s->outp().writech(c); - if (s->actual.cursor.x == s->actual_width && allow_soft_wrap()) { + if (s->actual.cursor.x == s->actual.screen_width && allow_soft_wrap()) { s->soft_wrap_location = screen_data_t::cursor_t{0, s->actual.cursor.y + 1}; // Note that our cursor position may be a lie: Apple Terminal makes the right cursor stick @@ -699,8 +699,6 @@ static void s_update(screen_t *scr, const wcstring &left_prompt, const wcstring const size_t right_prompt_width = cached_layouts.calc_prompt_layout(right_prompt).last_line_width; - int screen_width = common_get_width(); - // Figure out how many following lines we need to clear (probably 0). size_t actual_lines_before_reset = scr->actual_lines_before_reset; scr->actual_lines_before_reset = 0; @@ -709,17 +707,19 @@ static void s_update(screen_t *scr, const wcstring &left_prompt, const wcstring bool need_clear_screen = scr->need_clear_screen; bool has_cleared_screen = false; - if (scr->actual_width != screen_width) { + const int screen_width = scr->desired.screen_width; + + if (scr->actual.screen_width != screen_width) { // Ensure we don't issue a clear screen for the very first output, to avoid issue #402. - if (scr->actual_width > 0) { + if (scr->actual.screen_width > 0) { need_clear_screen = true; s_move(scr, 0, 0); - s_reset(scr, screen_reset_mode_t::current_line_contents); + s_reset(scr, screen_width, screen_reset_mode_t::current_line_contents); need_clear_lines = need_clear_lines || scr->need_clear_lines; need_clear_screen = need_clear_screen || scr->need_clear_screen; } - scr->actual_width = screen_width; + scr->actual.screen_width = screen_width; } scr->need_clear_lines = false; @@ -792,7 +792,7 @@ static void s_update(screen_t *scr, const wcstring &left_prompt, const wcstring } if (next_line_will_change) { skip_remaining = - std::min(skip_remaining, static_cast(scr->actual_width - 2)); + std::min(skip_remaining, static_cast(scr->actual.screen_width - 2)); } } } @@ -1097,8 +1097,8 @@ static screen_layout_t compute_layout(screen_t *s, size_t screen_width, return result; } -void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_prompt, - const wcstring &commandline, size_t explicit_len, +void s_write(screen_t *s, int screen_width, const wcstring &left_prompt, + const wcstring &right_prompt, const wcstring &commandline, size_t explicit_len, const std::vector &colors, const std::vector &indent, size_t cursor_pos, const page_rendering_t &pager, bool cursor_is_within_pager) { static relaxed_atomic_t s_repaints{0}; @@ -1123,7 +1123,6 @@ void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_pro } s_check_status(s); - const size_t screen_width = common_get_width(); // Completely ignore impossibly small screens. if (screen_width < 4) { @@ -1138,7 +1137,8 @@ void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_pro s->autosuggestion_is_truncated = !autosuggestion.empty() && autosuggestion != layout.autosuggestion; - // Clear the desired screen. + // Clear the desired screen and set its width. + s->desired.screen_width = screen_width; s->desired.resize(0); s->desired.cursor.x = s->desired.cursor.y = 0; @@ -1189,7 +1189,8 @@ void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_pro s_update(s, layout.left_prompt, layout.right_prompt); s_save_status(s); } -void s_reset(screen_t *s, screen_reset_mode_t mode) { + +void s_reset(screen_t *s, int screen_width, screen_reset_mode_t mode) { assert(s && "Null screen"); bool abandon_line = false, repaint_prompt = false, clear_to_eos = false; @@ -1245,9 +1246,8 @@ void s_reset(screen_t *s, screen_reset_mode_t mode) { if (abandon_line) { // Do the PROMPT_SP hack. - int screen_width = common_get_width(); wcstring abandon_line_string; - abandon_line_string.reserve(screen_width + 32); // should be enough + abandon_line_string.reserve(screen_width + 32); // Don't need to check for fish_wcwidth errors; this is done when setting up // omitted_newline_char in common.cpp. diff --git a/src/screen.h b/src/screen.h index 2f85a5d75..6805f44eb 100644 --- a/src/screen.h +++ b/src/screen.h @@ -79,12 +79,18 @@ class screen_data_t { std::vector line_datas; public: + /// The width of the screen in this rendering. + /// -1 if not set, i.e. we have not rendered before. + int screen_width{-1}; + + /// Where the cursor is in (x, y) coordinates. struct cursor_t { int x{0}; int y{0}; cursor_t() = default; cursor_t(int a, int b) : x(a), y(b) {} - } cursor; + }; + cursor_t cursor; line_t &add_line(void) { line_datas.resize(line_datas.size() + 1); @@ -135,9 +141,6 @@ class screen_t { wcstring actual_left_prompt{}; /// Last right prompt width. size_t last_right_prompt_width{0}; - /// The actual width of the screen at the time of the last screen write, or negative if not yet - /// set. - int actual_width{-1}; /// If we support soft wrapping, we can output to this location without any cursor motion. maybe_t soft_wrap_location{}; /// Whether the last-drawn autosuggestion (if any) is truncated, or hidden entirely. @@ -171,6 +174,7 @@ class screen_t { /// screen in order to render the desired output using as few terminal commands as possible. /// /// \param s the screen on which to write +/// \param int screen_width the width of the screen to render /// \param left_prompt the prompt to prepend to the command line /// \param right_prompt the right prompt, or NULL if none /// \param commandline the command line @@ -181,8 +185,8 @@ class screen_t { /// \param cursor_pos where the cursor is /// \param pager_data any pager data, to append to the screen /// \param cursor_is_within_pager whether the position is within the pager line (first line) -void s_write(screen_t *s, const wcstring &left_prompt, const wcstring &right_prompt, - const wcstring &commandline, size_t explicit_len, +void s_write(screen_t *s, int screen_width, const wcstring &left_prompt, + const wcstring &right_prompt, const wcstring &commandline, size_t explicit_len, const std::vector &colors, const std::vector &indent, size_t cursor_pos, const page_rendering_t &pager_data, bool cursor_is_within_pager); @@ -202,13 +206,14 @@ enum class screen_reset_mode_t { /// this function when some other function than s_write has written to the screen. /// /// \param s the screen to reset +/// \param screen_width the current width of the screen /// \param mode the sort of screen reset to apply /// /// If reset_cursor is incorrectly set to false, this may result in screen contents being erased. If /// it is incorrectly set to true, it may result in one or more lines of garbage on screen on the /// next repaint. If this happens during a loop, such as an interactive resizing, there will be one /// line of garbage for every repaint, which will quickly fill the screen. -void s_reset(screen_t *s, screen_reset_mode_t mode); +void s_reset(screen_t *s, int screen_width, screen_reset_mode_t mode); /// Stat stdout and stderr and save result as the current timestamp. void s_save_status(screen_t *s);