[lldb] Use the terminal height for paging editline completions (llvm#119914)

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.

This builds on the improvements of llvm#116456.

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;
     }
 
@@ -990,6 +1000,8 @@ PrintCompletion(FILE *output_file,
     for (llvm::StringRef line : llvm::split(c.GetDescription(), '\n')) {
       if (line.empty())
         break;
+      if (max_height && lines_printed >= *max_height)
+        break;
       if (!first)
         fprintf(output_file, "%*s",
                 static_cast<int>(description_col + separator_length), "");
@@ -1000,14 +1012,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 +1031,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 +1045,12 @@ 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), max_len,
+                        editline.GetTerminalWidth(),
+                        all ? std::nullopt : std::optional<size_t>(page_size));
 
     if (cur_pos >= results.size())
       break;
@@ -1525,6 +1535,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/functionalities/completion/TestCompletion.py

@@ -335,13 +335,22 @@ def test_settings_replace_target_ru(self):
         )
 
     def test_settings_show_term(self):
-        self.complete_from_to("settings show term-", "settings show term-width")
+        self.complete_from_to("settings show term-w", "settings show term-width")
 
     def test_settings_list_term(self):
-        self.complete_from_to("settings list term-", "settings list term-width")
+        self.complete_from_to("settings list term-w", "settings list term-width")
+
+    def test_settings_show_term(self):
+        self.complete_from_to("settings show term-h", "settings show term-height")
+
+    def test_settings_list_term(self):
+        self.complete_from_to("settings list term-h", "settings list term-height")
+
+    def test_settings_remove_term(self):
+        self.complete_from_to("settings remove term-w", "settings remove term-width")
 
     def test_settings_remove_term(self):
-        self.complete_from_to("settings remove term-", "settings remove term-width")
+        self.complete_from_to("settings remove term-h", "settings remove term-height")
 
     def test_settings_s(self):
         """Test that 'settings s' completes to ['set', 'show']."""

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);
     }
   }
 }