[lldb] Correctly restore the cursor column after resizing the statusline

[JDevlieghere]

This PR ensures we correctly restore the cursor column after resizing the statusline. To ensure we have space for the statusline, we have to emit a newline to move up everything on screen. The newline causes the cursor to move to the start of the next line, which needs to be undone.

Normally, we would use escape codes to save & restore the cursor position, but that doesn't work here, as the cursor position may have (purposely) changed. Instead, we move the cursor up one line using an escape code, but we weren't restoring the column.

Interestingly, Editline was able to recover from this issue through the LineInfo struct which contains the buffer and the cursor location, which allows us to compute the column. This PR addresses the bug by having Editline "refresh" the cursor position.

Fixes #134064

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This PR ensures we correctly restore the cursor column after resizing the statusline. To ensure we have space for the statusline, we have to emit a newline to move up everything on screen. The newline causes the cursor to move to the start of the next line, which needs to be undone.

Normally, we would use escape codes to save & restore the cursor position, but that doesn't work here, as the cursor position may have (purposely) changed. Instead, we move the cursor up one line using an escape code, but we weren't restoring the column.

Interestingly, Editline was able to recover from this issue through the LineInfo struct which contains the buffer and the cursor location, which allows us to compute the column. This PR addresses the bug by having Editline "refresh" the cursor position.

Fixes #134064

---

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

7 Files Affected:

• (modified) lldb/include/lldb/Core/Debugger.h (+2)
• (modified) lldb/include/lldb/Core/IOHandler.h (+4)
• (modified) lldb/include/lldb/Host/Editline.h (+2)
• (modified) lldb/source/Core/Debugger.cpp (+7)
• (modified) lldb/source/Core/IOHandler.cpp (+7)
• (modified) lldb/source/Core/Statusline.cpp (+11-8)
• (modified) lldb/source/Host/common/Editline.cpp (+7)

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 2087ef2a11562..504f936fe317a 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -227,6 +227,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   const char *GetIOHandlerHelpPrologue();
 
+  void RefreshIOHandler();
+
   void ClearIOHandlers();
 
   bool EnableLog(llvm::StringRef channel,
diff --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h
index 2fb3d7a7c9cc3..2672bbe5da2b3 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -90,6 +90,8 @@ class IOHandler {
 
   virtual void TerminalSizeChanged() {}
 
+  virtual void Refresh() {}
+
   virtual const char *GetPrompt() {
     // Prompt support isn't mandatory
     return nullptr;
@@ -404,6 +406,8 @@ class IOHandlerEditline : public IOHandler {
 
   void PrintAsync(const char *s, size_t len, bool is_stdout) override;
 
+  void Refresh() override;
+
 private:
 #if LLDB_ENABLE_LIBEDIT
   bool IsInputCompleteCallback(Editline *editline, StringList &lines);
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index c202a76758e13..947ad3bfe5ec6 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -267,6 +267,8 @@ class Editline {
 
   size_t GetTerminalHeight() { return m_terminal_height; }
 
+  void Refresh();
+
 private:
   /// Sets the lowest line number for multi-line editing sessions.  A value of
   /// zero suppresses line number printing in the prompt.
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 445baf1f63785..ed674ee1275c7 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1445,6 +1445,13 @@ bool Debugger::PopIOHandler(const IOHandlerSP &pop_reader_sp) {
   return true;
 }
 
+void Debugger::RefreshIOHandler() {
+  std::lock_guard<std::recursive_mutex> guard(m_io_handler_stack.GetMutex());
+  IOHandlerSP reader_sp(m_io_handler_stack.Top());
+  if (reader_sp)
+    reader_sp->Refresh();
+}
+
 StreamUP Debugger::GetAsyncOutputStream() {
   return std::make_unique<StreamAsynchronousIO>(*this,
                                                 StreamAsynchronousIO::STDOUT);
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 8aac507eaa0c2..f65a1113f3592 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -663,3 +663,10 @@ void IOHandlerEditline::PrintAsync(const char *s, size_t len, bool is_stdout) {
 #endif
   }
 }
+
+void IOHandlerEditline::Refresh() {
+#if LLDB_ENABLE_LIBEDIT
+  if (m_editline_up)
+    m_editline_up->Refresh();
+#endif
+}
diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp
index 8a8640805cac0..8ec57c9fa5bac 100644
--- a/lldb/source/Core/Statusline.cpp
+++ b/lldb/source/Core/Statusline.cpp
@@ -103,20 +103,23 @@ void Statusline::UpdateScrollWindow(ScrollWindowMode mode) {
       (mode == DisableStatusline) ? m_terminal_height : m_terminal_height - 1;
 
   LockedStreamFile locked_stream = stream_sp->Lock();
+
+  if (mode == EnableStatusline) {
+    // Move everything on the screen up.
+    locked_stream << '\n';
+    locked_stream.Printf(ANSI_UP_ROWS, 1);
+  }
+
   locked_stream << ANSI_SAVE_CURSOR;
   locked_stream.Printf(ANSI_SET_SCROLL_ROWS, scroll_height);
   locked_stream << ANSI_RESTORE_CURSOR;
-  switch (mode) {
-  case EnableStatusline:
-    // Move everything on the screen up.
-    locked_stream.Printf(ANSI_UP_ROWS, 1);
-    locked_stream << '\n';
-    break;
-  case DisableStatusline:
+
+  if (mode == DisableStatusline) {
     // Clear the screen below to hide the old statusline.
     locked_stream << ANSI_CLEAR_BELOW;
-    break;
   }
+
+  m_debugger.RefreshIOHandler();
 }
 
 void Statusline::Redraw(bool update) {
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 431f931d5f3fc..fe4643da21ee0 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1709,6 +1709,13 @@ void Editline::PrintAsync(lldb::LockableStreamFileSP stream_sp, const char *s,
   }
 }
 
+void Editline::Refresh() {
+  if (!m_editline || !m_output_stream_sp)
+    return;
+  LockedStreamFile locked_stream = m_output_stream_sp->Lock();
+  MoveCursor(CursorLocation::EditingPrompt, CursorLocation::EditingCursor);
+}
+
 bool Editline::CompleteCharacter(char ch, EditLineGetCharType &out) {
 #if !LLDB_EDITLINE_USE_WCHAR
   if (ch == (char)EOF)

[labath]

This looks good. Any chance of a test case? This sounds like something pexpect should manage to handle.

[JDevlieghere]

This looks good. Any chance of a test case? This sounds like something pexpect should manage to handle.

I'm somewhat skeptical of the value, but I can add a test case to check for the escape codes to make sure the new Editline code is called.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.