[Lldb-commits] [lldb] 3dfc1d9 - [lldb] Use the terminal height for paging editline completions (#119914)

via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 16 11:11:22 PST 2024


Author: Jonas Devlieghere
Date: 2024-12-16T11:11:17-08:00
New Revision: 3dfc1d9b0bc41eaf63e551ca357b44a71636b152

URL: https://github.com/llvm/llvm-project/commit/3dfc1d9b0bc41eaf63e551ca357b44a71636b152
DIFF: https://github.com/llvm/llvm-project/commit/3dfc1d9b0bc41eaf63e551ca357b44a71636b152.diff

LOG: [lldb] Use the terminal height for paging editline completions (#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 #116456.

Added: 
    

Modified: 
    lldb/include/lldb/API/SBDebugger.h
    lldb/include/lldb/Core/Debugger.h
    lldb/include/lldb/Host/Editline.h
    lldb/source/API/SBDebugger.cpp
    lldb/source/Core/CoreProperties.td
    lldb/source/Core/Debugger.cpp
    lldb/source/Host/common/Editline.cpp
    lldb/test/API/functionalities/completion/TestCompletion.py
    lldb/test/API/terminal/TestEditlineCompletions.py
    lldb/tools/driver/Driver.cpp
    lldb/tools/driver/Driver.h

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 1f42ec3cdc7d51..787bd040dd15bb 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/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;

diff  --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 1d5f2fcc20626c..70f4c4216221c6 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/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;

diff  --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index e8e8a6c0d4f67e..26deba38f8471c 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/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;

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 4efec747aacff1..4e6b22492a0d1c 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/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);
 

diff  --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td
index e11aad2660b461..d3816c3070bbc5 100644
--- a/lldb/source/Core/CoreProperties.td
+++ b/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">,

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index bb0a78790a9649..6ceb209269c9e7 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/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");

diff  --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index c9e890304ae1ec..6e35b15d69651d 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -924,10 +924,11 @@ 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;
@@ -935,7 +936,14 @@ PrintCompletion(FILE *output_file,
   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(); }

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py
index bf19a990cf6e30..bf043c795fac68 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/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']."""

diff  --git a/lldb/test/API/terminal/TestEditlineCompletions.py b/lldb/test/API/terminal/TestEditlineCompletions.py
index 7fa6f95c130c64..b4ea0f39ec10c5 100644
--- a/lldb/test/API/terminal/TestEditlineCompletions.py
+++ b/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")

diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 98c3643f75c97b..771663eb7bf0af 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/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,8 +629,9 @@ 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) {
@@ -636,7 +639,7 @@ void sigwinch_handler(int signo) {
   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);
     }
   }
 }

diff  --git a/lldb/tools/driver/Driver.h b/lldb/tools/driver/Driver.h
index 83e0d8a41cfdb1..ed400ca43575d4 100644
--- a/lldb/tools/driver/Driver.h
+++ b/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;


        


More information about the lldb-commits mailing list