[Lldb-commits] [lldb] [lldb] Apply pending terminal resize before repainting the prompt (PR #203045)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

Editline defers ApplyTerminalSizeChange() (el_resize plus refreshing the cached terminal dimensions) to its read loop on the main thread, but Refresh() and PrintAsync() repaint the prompt synchronously from the resize notification on the signal thread, ahead of that apply. The repaint therefore used the dimensions cached before the resize. Apply any pending resize first via a shared helper. The output stream lock already serializes these paths, and it is a no-op when nothing is pending.

Editline keeps querying its own size (el_resize/EL_GETTC) rather than using the dimensions the driver reports: el_resize must update libedit's internal line-wrapping model regardless (libedit exposes no way to set it), and editline's cursor arithmetic must use the same width libedit wraps at. EL_GETTC reads back exactly that value, so the two cannot diverge, while sourcing the width from the driver's separate ioctl could.

---
Full diff: https://github.com/llvm/llvm-project/pull/203045.diff


3 Files Affected:

- (modified) lldb/include/lldb/Host/Editline.h (+6) 
- (modified) lldb/source/Host/common/Editline.cpp (+8-2) 
- (modified) lldb/unittests/Editline/EditlineTest.cpp (+35) 


``````````diff
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 947ad3bfe5ec6..39e2c6b73eef6 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -384,6 +384,12 @@ class Editline {
 
   void ApplyTerminalSizeChange();
 
+  /// Apply a resize signaled by TerminalSizeChanged() if one is pending. The
+  /// output stream lock must be held. Repaints issued from the resize
+  /// notification thread call this so they draw at the new dimensions rather
+  /// than the dimensions cached before the resize.
+  void ApplyPendingTerminalSizeChange();
+
   // The following set various editline parameters.  It's not any less
   // verbose to put the editline calls into a function, but it
   // provides type safety, since the editline functions take varargs
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 833516b0b2c2d..483d236ac0bc4 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -575,8 +575,7 @@ int Editline::GetCharacter(EditLineGetCharType *c) {
     m_current_line_rows = new_line_rows;
   }
 
-  if (m_terminal_size_has_changed)
-    ApplyTerminalSizeChange();
+  ApplyPendingTerminalSizeChange();
 
   // This mutex is locked by our caller (GetLine). Unlock it while we read a
   // character (blocking operation), so we do not hold the mutex
@@ -1590,6 +1589,11 @@ void Editline::ApplyTerminalSizeChange() {
   }
 }
 
+void Editline::ApplyPendingTerminalSizeChange() {
+  if (m_terminal_size_has_changed)
+    ApplyTerminalSizeChange();
+}
+
 const char *Editline::GetPrompt() { return m_set_prompt.c_str(); }
 
 uint32_t Editline::GetCurrentLine() { return m_current_line_index; }
@@ -1706,6 +1710,7 @@ void Editline::PrintAsync(lldb::LockableStreamFileSP stream_sp, const char *s,
                           size_t len) {
   LockedStreamFile locked_stream = m_output_stream_sp->Lock();
   if (m_editor_status == EditorStatus::Editing) {
+    ApplyPendingTerminalSizeChange();
     SaveEditedLine();
     MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart);
     fprintf(locked_stream.GetFile().GetStream(), ANSI_CLEAR_BELOW);
@@ -1721,6 +1726,7 @@ void Editline::Refresh() {
   if (!m_editline || !m_output_stream_sp)
     return;
   LockedStreamFile locked_stream = m_output_stream_sp->Lock();
+  ApplyPendingTerminalSizeChange();
   el_set(m_editline, EL_REFRESH);
 }
 
diff --git a/lldb/unittests/Editline/EditlineTest.cpp b/lldb/unittests/Editline/EditlineTest.cpp
index 2afc33680f348..1770773f906ad 100644
--- a/lldb/unittests/Editline/EditlineTest.cpp
+++ b/lldb/unittests/Editline/EditlineTest.cpp
@@ -17,6 +17,7 @@
 #define EDITLINE_TEST_DUMP_OUTPUT 0
 
 #include <stdio.h>
+#include <sys/ioctl.h>
 #include <unistd.h>
 
 #include "gmock/gmock.h"
@@ -63,6 +64,8 @@ class EditlineAdapter {
 
   void ConsumeAllOutput();
 
+  bool SetTerminalSize(unsigned short columns, unsigned short rows);
+
 private:
   bool IsInputComplete(lldb_private::Editline *editline,
                        lldb_private::StringList &lines);
@@ -214,6 +217,14 @@ void EditlineAdapter::ConsumeAllOutput() {
   }
 }
 
+bool EditlineAdapter::SetTerminalSize(unsigned short columns,
+                                      unsigned short rows) {
+  struct winsize ws = {};
+  ws.ws_col = columns;
+  ws.ws_row = rows;
+  return ::ioctl(_el_primary_file->GetDescriptor(), TIOCSWINSZ, &ws) == 0;
+}
+
 class EditlineTestFixture : public ::testing::Test {
   SubsystemRAII<FileSystem, HostInfo> subsystems;
   EditlineAdapter _el_adapter;
@@ -289,4 +300,28 @@ TEST_F(EditlineTestFixture, EditlineReceivesMultiLineText) {
   EXPECT_THAT(reported_lines, testing::ContainerEq(input_lines));
 }
 
+TEST_F(EditlineTestFixture, TerminalResizeIsAppliedBeforeRefresh) {
+  EditlineAdapter &adapter = GetEditlineAdapter();
+  Editline &editline = adapter.GetEditline();
+
+  // Drive one line so the EditLine instance is configured and has queried the
+  // initial terminal size.
+  EXPECT_TRUE(adapter.SendLine("hello"));
+  std::string line;
+  bool interrupted = false;
+  EXPECT_TRUE(adapter.GetLine(line, interrupted, TIMEOUT_MILLIS));
+
+  const size_t initial_width = editline.GetTerminalWidth();
+  const unsigned short new_width = initial_width == 80 ? 100 : 80;
+  ASSERT_TRUE(adapter.SetTerminalSize(new_width, 24));
+
+  // A resize signaled out of band stays pending until a repaint consumes it.
+  // Refresh performs that repaint, so it must redraw at the new size rather
+  // than the size cached before the resize.
+  editline.TerminalSizeChanged();
+  editline.Refresh();
+
+  EXPECT_EQ(editline.GetTerminalWidth(), static_cast<size_t>(new_width));
+}
+
 #endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/203045