llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> On a terminal resize the statusline cleared the whole screen (ESC[2J) and redrew, because recomputing in place was buggy: the statusline wrapped and duplicated. The clear also wiped the visible scrollback on every resize. I got lots of feedback that this wasn't a great user experience so I spent some time taking another stab at this. This PR reverts back to recomputing the statusline. After a resize the terminal still shows the old statusline: - Making the terminal smaller (horizontally) reflows the full-width line into ceil(prev_width / width) rows at the bottom - Making the terminal larger (vertically) leaves it stranded at its old row Clear only the rows it can still occupy and redraw, preserving the scrollback above. Disable autowrap while drawing so a line briefly wider than the terminal is clipped at the margin rather than wrapping onto the row above. Removing the clear exposed that Editline::Refresh() redrew the prompt via libedit's EL_REFRESH, which uses a cursor model that is stale once the statusline has moved the cursor, tiling the prompt once per resize. Repaint the prompt and input from our own tracked position instead. Fixes #<!-- -->146919 --- Full diff: https://github.com/llvm/llvm-project/pull/202691.diff 4 Files Affected: - (modified) lldb/include/lldb/Core/Statusline.h (+5-2) - (modified) lldb/source/Core/Statusline.cpp (+38-6) - (modified) lldb/source/Host/common/Editline.cpp (+11-1) - (modified) lldb/test/API/functionalities/statusline/TestStatusline.py (+36) ``````````diff diff --git a/lldb/include/lldb/Core/Statusline.h b/lldb/include/lldb/Core/Statusline.h index eacdec306ccf9..1c716d9e1e8be 100644 --- a/lldb/include/lldb/Core/Statusline.h +++ b/lldb/include/lldb/Core/Statusline.h @@ -48,8 +48,11 @@ class Statusline { ResizeStatusline, }; - /// Set the scroll window for the given mode. - void UpdateScrollWindow(ScrollWindowMode mode); + /// Set the scroll window for the given mode. On a resize, \p prev_width and + /// \p prev_height are the dimensions the statusline was last drawn at, used + /// to clear the rows it still occupies. + void UpdateScrollWindow(ScrollWindowMode mode, uint64_t prev_width = 0, + uint64_t prev_height = 0); Debugger &m_debugger; diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index d49735b3c64c2..609b63059d386 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -29,6 +29,8 @@ #define ANSI_TO_START_OF_ROW ESCAPE "[%u;1f" #define ANSI_REVERSE_VIDEO ESCAPE "[7m" #define ANSI_UP_ROWS ESCAPE "[%dA" +#define ANSI_DISABLE_AUTO_WRAP ESCAPE "[?7l" +#define ANSI_ENABLE_AUTO_WRAP ESCAPE "[?7h" using namespace lldb; using namespace lldb_private; @@ -40,10 +42,15 @@ Statusline::Statusline(Debugger &debugger) Statusline::~Statusline() { Disable(); } void Statusline::TerminalSizeChanged() { + // The dimensions the statusline was last drawn at, needed to clear it before + // redrawing at the new size. + const uint64_t prev_width = m_terminal_width; + const uint64_t prev_height = m_terminal_height; + m_terminal_width = m_debugger.GetTerminalWidth(); m_terminal_height = m_debugger.GetTerminalHeight(); - UpdateScrollWindow(ResizeStatusline); + UpdateScrollWindow(ResizeStatusline, prev_width, prev_height); // Redraw the old statusline. Redraw(std::nullopt); @@ -71,6 +78,9 @@ void Statusline::Draw(std::string str) { LockedStreamFile locked_stream = stream_sp->Lock(); locked_stream << ANSI_SAVE_CURSOR; + // A statusline wider than the terminal (e.g. a stale width mid-resize) would + // wrap onto the row above; with autowrap off it is clipped at the margin. + locked_stream << ANSI_DISABLE_AUTO_WRAP; locked_stream.Printf(ANSI_TO_START_OF_ROW, static_cast<unsigned>(m_terminal_height)); @@ -81,10 +91,12 @@ void Statusline::Draw(std::string str) { locked_stream << str; locked_stream << ANSI_NORMAL; + locked_stream << ANSI_ENABLE_AUTO_WRAP; locked_stream << ANSI_RESTORE_CURSOR; } -void Statusline::UpdateScrollWindow(ScrollWindowMode mode) { +void Statusline::UpdateScrollWindow(ScrollWindowMode mode, uint64_t prev_width, + uint64_t prev_height) { assert(m_terminal_width != 0 && m_terminal_height != 0); lldb::LockableStreamFileSP stream_sp = m_debugger.GetOutputStreamSP(); @@ -114,13 +126,33 @@ void Statusline::UpdateScrollWindow(ScrollWindowMode mode) { // Clear the screen below to hide the old statusline. locked_stream << ANSI_CLEAR_BELOW; break; - case ResizeStatusline: - // Clear the screen and update the scroll window. - // FIXME: Find a better solution (#146919). - locked_stream << ANSI_CLEAR_SCREEN; + case ResizeStatusline: { + // The old statusline is still on screen after a resize: a width shrink + // reflows that full-width line into ceil(prev_width / width) rows at the + // bottom, and growing taller strands it at its old row. Clear from the + // topmost row it can occupy to the bottom (preserving the scrollback + // above), then re-establish the scroll region. DECSTBM homes the cursor, + // so save and restore it. + const unsigned height = static_cast<unsigned>(m_terminal_height); + unsigned reflow = 1; + if (prev_width > m_terminal_width && m_terminal_width > 0) + reflow = (prev_width + m_terminal_width - 1) / m_terminal_width; + if (reflow >= height) + reflow = height - 1; + unsigned first_row = height - reflow + 1; + if (prev_height > 0 && prev_height < first_row) + first_row = static_cast<unsigned>(prev_height); + if (first_row < 1) + first_row = 1; + + locked_stream << ANSI_SAVE_CURSOR; + locked_stream.Printf(ANSI_TO_START_OF_ROW, first_row); + locked_stream << ANSI_CLEAR_BELOW; locked_stream.Printf(ANSI_SET_SCROLL_ROWS, reduced_scroll_rows); + locked_stream << ANSI_RESTORE_CURSOR; break; } + } } m_debugger.RefreshIOHandler(); } diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index 833516b0b2c2d..fcf516c1c61c8 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -1721,7 +1721,17 @@ void Editline::Refresh() { if (!m_editline || !m_output_stream_sp) return; LockedStreamFile locked_stream = m_output_stream_sp->Lock(); - el_set(m_editline, EL_REFRESH); + if (m_editor_status == EditorStatus::Editing) { + // EL_REFRESH redraws from libedit's cursor model, which is stale once the + // statusline has moved the cursor, so it reprints the prompt at the wrong + // column. Repaint from our own tracked position instead. + SaveEditedLine(); + MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart); + DisplayInput(); + MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingCursor); + } else { + el_set(m_editline, EL_REFRESH); + } } bool Editline::CompleteCharacter(char ch, EditLineGetCharType &out) { diff --git a/lldb/test/API/functionalities/statusline/TestStatusline.py b/lldb/test/API/functionalities/statusline/TestStatusline.py index f5c6a817d1309..727942d6a1eac 100644 --- a/lldb/test/API/functionalities/statusline/TestStatusline.py +++ b/lldb/test/API/functionalities/statusline/TestStatusline.py @@ -117,6 +117,42 @@ def test_resize(self): self.child.expect(re.escape("\x1b[1;19r")) self.child.expect("(lldb)") + @skipIfEditlineSupportMissing + def test_resize_recomputes_without_clearing(self): + """Resizing should recompute the statusline in place, not clear the + entire screen. Clearing the screen (ESC[2J) was the old workaround for + the statusline wrapping/duplicating on resize; it also wiped the visible + scrollback on every resize.""" + self.launch() + self.resize() + self.expect("set set show-statusline true", ["no target"]) + + # Capture the output emitted during the resize. The tee handles both + # str- and bytes-mode spawns. + class Tee: + def __init__(self): + self.data = b"" + + def write(self, s): + self.data += s if isinstance(s, bytes) else s.encode( + "latin-1", "replace" + ) + return len(s) + + def flush(self): + pass + + tee = Tee() + self.child.logfile_read = tee + self.resize(20, 60) + # Wait for the resize redraw to finish before inspecting the capture. + self.child.expect(re.escape("\x1b[1;19r")) + self.child.expect("(lldb)") + self.child.logfile_read = None + + # The resize recomputes in place; it must not clear the whole screen. + self.assertNotIn(b"\x1b[2J", tee.data) + @skipUnlessPlatform(["linux"]) def test_target_symbols_add(self): """Regression test: adding symbols while the statusline is enabled `````````` </details> https://github.com/llvm/llvm-project/pull/202691 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
