[lldb] Use the terminal height for paging editline completions

Currently, we arbitrarily paginate editline completions to 40 elements.
On large terminals, that leaves some real-estate unused. On small
terminals, it's pretty annoying to not see the first completions. We can
address both issues by using the terminal height for pagination.

Author: JDevlieghere

lldb/include/lldb/API/SBDebugger.h

@@ -382,6 +382,10 @@ class LLDB_API SBDebugger {
 
   void SetTerminalWidth(uint32_t term_width);
 
+  uint32_t GetTerminalHeight() const;
+
+  void SetTerminalHeight(uint32_t term_height);
+
   lldb::user_id_t GetID();
 
   const char *GetPrompt() const;

lldb/include/lldb/Core/Debugger.h

@@ -280,6 +280,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   bool SetTerminalWidth(uint64_t term_width);
 
+  uint64_t GetTerminalHeight() const;
+
+  bool SetTerminalHeight(uint64_t term_height);
+
   llvm::StringRef GetPrompt() const;
 
   llvm::StringRef GetPromptAnsiPrefix() const;

lldb/include/lldb/Host/Editline.h

@@ -240,6 +240,8 @@ class Editline {
 
   size_t GetTerminalWidth() { return m_terminal_width; }
 
+  size_t GetTerminalHeight() { return m_terminal_height; }
+
 private:
   /// Sets the lowest line number for multi-line editing sessions.  A value of
   /// zero suppresses line number printing in the prompt.
@@ -373,6 +375,7 @@ class Editline {
   std::vector<EditLineStringType> m_input_lines;
   EditorStatus m_editor_status;
   int m_terminal_width = 0;
+  int m_terminal_height = 0;
   int m_base_line_number = 0;
   unsigned m_current_line_index = 0;
   int m_current_line_rows = -1;

lldb/source/API/SBDebugger.cpp

@@ -1405,6 +1405,19 @@ void SBDebugger::SetTerminalWidth(uint32_t term_width) {
     m_opaque_sp->SetTerminalWidth(term_width);
 }
 
+uint32_t SBDebugger::GetTerminalHeight() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  return (m_opaque_sp ? m_opaque_sp->GetTerminalWidth() : 0);
+}
+
+void SBDebugger::SetTerminalHeight(uint32_t term_height) {
+  LLDB_INSTRUMENT_VA(this, term_height);
+
+  if (m_opaque_sp)
+    m_opaque_sp->SetTerminalHeight(term_height);
+}
+
 const char *SBDebugger::GetPrompt() const {
   LLDB_INSTRUMENT_VA(this);
 

lldb/source/Core/CoreProperties.td

@@ -136,6 +136,10 @@ let Definition = "debugger" in {
     Global,
     DefaultUnsignedValue<80>,
     Desc<"The maximum number of columns to use for displaying text.">;
+  def TerminalHeight: Property<"term-height", "UInt64">,
+    Global,
+    DefaultUnsignedValue<24>,
+    Desc<"The number of rows used for displaying text.">;
   def ThreadFormat: Property<"thread-format", "FormatEntity">,
     Global,
     DefaultStringValue<"thread #${thread.index}: tid = ${thread.id%tid}{, ${frame.pc}}{ ${module.file.basename}{`${function.name-with-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{, name = ${ansi.fg.green}'${thread.name}'${ansi.normal}}{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}{, activity = ${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}{, ${thread.info.trace_messages} messages}{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}{\\\\nReturn value: ${thread.return-value}}{\\\\nCompleted expression: ${thread.completed-expression}}\\\\n">,

lldb/source/Core/Debugger.cpp

@@ -371,11 +371,29 @@ uint64_t Debugger::GetTerminalWidth() const {
 }
 
 bool Debugger::SetTerminalWidth(uint64_t term_width) {
+  const uint32_t idx = ePropertyTerminalWidth;
+  const bool success = SetPropertyAtIndex(idx, term_width);
+
   if (auto handler_sp = m_io_handler_stack.Top())
     handler_sp->TerminalSizeChanged();
 
-  const uint32_t idx = ePropertyTerminalWidth;
-  return SetPropertyAtIndex(idx, term_width);
+  return success;
+}
+
+uint64_t Debugger::GetTerminalHeight() const {
+  const uint32_t idx = ePropertyTerminalHeight;
+  return GetPropertyAtIndexAs<uint64_t>(
+      idx, g_debugger_properties[idx].default_uint_value);
+}
+
+bool Debugger::SetTerminalHeight(uint64_t term_height) {
+  const uint32_t idx = ePropertyTerminalHeight;
+  const bool success = SetPropertyAtIndex(idx, term_height);
+
+  if (auto handler_sp = m_io_handler_stack.Top())
+    handler_sp->TerminalSizeChanged();
+
+  return success;
 }
 
 bool Debugger::GetUseExternalEditor() const {
@@ -919,7 +937,11 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton)
       m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
           ePropertyTerminalWidth);
   term_width->SetMinimumValue(10);
-  term_width->SetMaximumValue(1024);
+
+  OptionValueUInt64 *term_height =
+      m_collection_sp->GetPropertyAtIndexAsOptionValueUInt64(
+          ePropertyTerminalHeight);
+  term_height->SetMinimumValue(10);
 
   // Turn off use-color if this is a dumb terminal.
   const char *term = getenv("TERM");

lldb/source/Host/common/Editline.cpp

@@ -924,18 +924,26 @@ unsigned char Editline::BufferEndCommand(int ch) {
 
 /// Prints completions and their descriptions to the given file. Only the
 /// completions in the interval [start, end) are printed.
-static void
+static size_t
 PrintCompletion(FILE *output_file,
                 llvm::ArrayRef<CompletionResult::Completion> results,
-                size_t max_completion_length, size_t max_length) {
+                size_t max_completion_length, size_t max_length,
+                std::optional<size_t> max_height = std::nullopt) {
   constexpr size_t ellipsis_length = 3;
   constexpr size_t padding_length = 8;
   constexpr size_t separator_length = 4;
 
   const size_t description_col =
       std::min(max_completion_length + padding_length, max_length);
 
+  size_t lines_printed = 0;
+  size_t results_printed = 0;
   for (const CompletionResult::Completion &c : results) {
+    if (max_height && lines_printed == *max_height)
+      break;
+
+    results_printed++;
+
     if (c.GetCompletion().empty())
       continue;
 
@@ -956,6 +964,7 @@ PrintCompletion(FILE *output_file,
       fprintf(output_file, "%.*s...\n",
               static_cast<int>(max_length - padding_length - ellipsis_length),
               c.GetCompletion().c_str());
+      lines_printed++;
       continue;
     }
 
@@ -964,6 +973,7 @@ PrintCompletion(FILE *output_file,
     if (c.GetDescription().empty() ||
         description_col + separator_length + ellipsis_length >= max_length) {
       fprintf(output_file, "\n");
+      lines_printed++;
       continue;
     }
 
@@ -1000,14 +1010,17 @@ PrintCompletion(FILE *output_file,
       if (position + description_length < max_length) {
         fprintf(output_file, "%.*s\n", static_cast<int>(description_length),
                 line.data());
+        lines_printed++;
       } else {
         fprintf(output_file, "%.*s...\n",
                 static_cast<int>(max_length - position - ellipsis_length),
                 line.data());
+        lines_printed++;
         continue;
       }
     }
   }
+  return results_printed;
 }
 
 void Editline::DisplayCompletions(
@@ -1016,7 +1029,11 @@ void Editline::DisplayCompletions(
 
   fprintf(editline.m_output_file,
           "\n" ANSI_CLEAR_BELOW "Available completions:\n");
-  const size_t page_size = 40;
+
+  /// Account for the current line, the line showing "Available completions"
+  /// before and the line saying "More" after.
+  const size_t page_size = editline.GetTerminalHeight() - 3;
+
   bool all = false;
 
   auto longest =
@@ -1026,21 +1043,15 @@ void Editline::DisplayCompletions(
 
   const size_t max_len = longest->GetCompletion().size();
 
-  if (results.size() < page_size) {
-    PrintCompletion(editline.m_output_file, results, max_len,
-                    editline.GetTerminalWidth());
-    return;
-  }
-
   size_t cur_pos = 0;
   while (cur_pos < results.size()) {
     size_t remaining = results.size() - cur_pos;
     size_t next_size = all ? remaining : std::min(page_size, remaining);
 
-    PrintCompletion(editline.m_output_file, results.slice(cur_pos, next_size),
-                    max_len, editline.GetTerminalWidth());
-
-    cur_pos += next_size;
+    cur_pos += PrintCompletion(
+        editline.m_output_file, results.slice(cur_pos, next_size), max_len,
+        editline.GetTerminalWidth(),
+        all ? std::nullopt : std::optional<size_t>(page_size));
 
     if (cur_pos >= results.size())
       break;
@@ -1525,6 +1536,13 @@ void Editline::ApplyTerminalSizeChange() {
     m_terminal_width = INT_MAX;
     m_current_line_rows = 1;
   }
+
+  int rows;
+  if (el_get(m_editline, EL_GETTC, "li", &rows, nullptr) == 0) {
+    m_terminal_height = rows;
+  } else {
+    m_terminal_height = INT_MAX;
+  }
 }
 
 const char *Editline::GetPrompt() { return m_set_prompt.c_str(); }

lldb/test/API/terminal/TestEditlineCompletions.py

@@ -62,3 +62,37 @@ def test_multiline_description(self):
             "                      kdp-remote is an abbreviation for 'process conn..."
         )
         self.child.expect("        kill       -- Terminate the current target process.")
+
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_completion_pagination(self):
+        """Test that we use the terminal height for pagination."""
+        self.launch(dimensions=(10, 30))
+        self.child.send("_regexp-\t")
+        self.child.expect("Available completions:")
+        self.child.expect("        _regexp-attach")
+        self.child.expect("        _regexp-break")
+        self.child.expect("        _regexp-bt")
+        self.child.expect("        _regexp-display")
+        self.child.expect("        _regexp-down")
+        self.child.expect("        _regexp-env")
+        self.child.expect("        _regexp-jump")
+        self.child.expect("More")
+
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_completion_multiline_pagination(self):
+        """Test that we use the terminal height for pagination and account for multi-line descriptions."""
+        self.launch(dimensions=(6, 72))
+        self.child.send("k\t")
+        self.child.expect("Available completions:")
+        self.child.expect(
+            "        kdp-remote -- Connect to a process via remote KDP server."
+        )
+        self.child.expect(
+            "                      If no UDP port is specified, port 41139 is assu..."
+        )
+        self.child.expect(
+            "                      kdp-remote is an abbreviation for 'process conn..."
+        )
+        self.child.expect("More")

lldb/tools/driver/Driver.cpp

@@ -451,6 +451,8 @@ int Driver::MainLoop() {
       ::ioctl(STDIN_FILENO, TIOCGWINSZ, &window_size) == 0) {
     if (window_size.ws_col > 0)
       m_debugger.SetTerminalWidth(window_size.ws_col);
+    if (window_size.ws_row > 0)
+      m_debugger.SetTerminalHeight(window_size.ws_row);
   }
 
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
@@ -627,16 +629,17 @@ int Driver::MainLoop() {
   return sb_interpreter.GetQuitStatus();
 }
 
-void Driver::ResizeWindow(unsigned short col) {
+void Driver::ResizeWindow(unsigned short col, unsigned short row) {
   GetDebugger().SetTerminalWidth(col);
+  GetDebugger().SetTerminalHeight(row);
 }
 
 void sigwinch_handler(int signo) {
   struct winsize window_size;
   if ((isatty(STDIN_FILENO) != 0) &&
       ::ioctl(STDIN_FILENO, TIOCGWINSZ, &window_size) == 0) {
     if ((window_size.ws_col > 0) && g_driver != nullptr) {
-      g_driver->ResizeWindow(window_size.ws_col);
+      g_driver->ResizeWindow(window_size.ws_col, window_size.ws_row);
     }
   }
 }

lldb/tools/driver/Driver.h

@@ -92,7 +92,7 @@ class Driver : public lldb::SBBroadcaster {
 
   lldb::SBDebugger &GetDebugger() { return m_debugger; }
 
-  void ResizeWindow(unsigned short col);
+  void ResizeWindow(unsigned short col, unsigned short row);
 
 private:
   lldb::SBDebugger m_debugger;