https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/119914
>From f6fdfaf4be339a6412019113462b05cbce66c753 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Thu, 22 Feb 2024 21:54:07 -0800 Subject: [PATCH 1/3] [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. --- lldb/include/lldb/API/SBDebugger.h | 4 ++ lldb/include/lldb/Core/Debugger.h | 4 ++ lldb/include/lldb/Host/Editline.h | 3 ++ lldb/source/API/SBDebugger.cpp | 13 ++++++ lldb/source/Core/CoreProperties.td | 4 ++ lldb/source/Core/Debugger.cpp | 28 ++++++++++-- lldb/source/Host/common/Editline.cpp | 44 +++++++++++++------ .../API/terminal/TestEditlineCompletions.py | 34 ++++++++++++++ lldb/tools/driver/Driver.cpp | 7 ++- lldb/tools/driver/Driver.h | 2 +- 10 files changed, 124 insertions(+), 19 deletions(-) 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..707b0c75bab7fd 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; } @@ -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(); } 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; >From 2745e5cd46c019d0a530c70c5bc4fe79837cdd5f Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 13 Dec 2024 12:57:30 -0800 Subject: [PATCH 2/3] Update TestCompletion.py --- .../functionalities/completion/TestCompletion.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) 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'].""" >From 3fe7645b68a9fd61645ac72dbeaa08a3b9f27b16 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Mon, 16 Dec 2024 08:34:02 -0800 Subject: [PATCH 3/3] Account for printed lines exceeding the max height --- lldb/source/Host/common/Editline.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index 707b0c75bab7fd..62c27f82eeeacd 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -939,7 +939,9 @@ PrintCompletion(FILE *output_file, size_t lines_printed = 0; size_t results_printed = 0; for (const CompletionResult::Completion &c : results) { - if (max_height && lines_printed == *max_height) + // It's possible we exceed the max height if the last entry had a + // multi-line description. + if (max_height && lines_printed >= *max_height) break; results_printed++; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits