From ed59cc87778c09fe56cc06bf4c33cb0e5ba6b857 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 19 Nov 2024 10:46:57 -0800 Subject: [PATCH 1/2] [lldb] Improve editline completion formatting (#116456) This patch improves the formatting of editline completions. The current implementation is naive and doesn't account for the terminal width. Concretely, the old implementation suffered from the following issues: - We would unconditionally pad to the longest completion. If that completion exceeds the width of the terminal, that would result in a lot of superfluous white space and line wrapping. - When printing the description, we wouldn't account for the presence of newlines, and they would continue without leading padding. The new code accounts for both. If the completion exceeds the available terminal width, we show what fits on the current lined followed by ellipsis. We also no longer pad beyond the length of the current line. Finally, we print the description line by line, with the proper leading padding. If a line of the description exceeds the available terminal width, we print ellipsis and won't print the next line. Before: ``` Available completions: _regexp-attach -- Attach to process by ID or name. _regexp-break -- Set a breakpoint using one of several shorthand formats. _regexp-bt -- Show backtrace of the current thread's call sta ck. Any numeric argument displays at most that many frames. The argument 'al l' displays all threads. Use 'settings set frame-format' to customize the pr inting of individual frames and 'settings set thread-format' to customize th e thread header. Frame recognizers may filter thelist. Use 'thread backtrace -u (--unfiltered)' to see them all. _regexp-display -- Evaluate an expression at every stop (see 'help target stop-hook'.) ``` After: ``` Available completions: _regexp-attach -- Attach to process by ID or name. _regexp-break -- Set a breakpoint using one of several shorth... _regexp-bt -- Show backtrace of the current thread's call ... _regexp-display -- Evaluate an expression at every stop (see 'h... ``` rdar://135818198 (cherry picked from commit dd78d7c7be5b8948cf5841e8033e59adebf230ad) --- lldb/include/lldb/Host/Editline.h | 2 + lldb/source/Host/common/Editline.cpp | 89 +++++++++++++++++-- .../API/terminal/TestEditlineCompletions.py | 64 +++++++++++++ 3 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 lldb/test/API/terminal/TestEditlineCompletions.py diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h index 57e2c831e3499..e8e8a6c0d4f67 100644 --- a/lldb/include/lldb/Host/Editline.h +++ b/lldb/include/lldb/Host/Editline.h @@ -238,6 +238,8 @@ class Editline { /// Convert the current input lines into a UTF8 StringList StringList GetInputAsStringList(int line_count = UINT32_MAX); + size_t GetTerminalWidth() { return m_terminal_width; } + 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/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index f95f854c5f220..1dc1c3fc18084 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -927,12 +927,86 @@ unsigned char Editline::BufferEndCommand(int ch) { static void PrintCompletion(FILE *output_file, llvm::ArrayRef results, - size_t max_len) { + size_t max_completion_length, size_t max_length) { + 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); + for (const CompletionResult::Completion &c : results) { - fprintf(output_file, "\t%-*s", (int)max_len, c.GetCompletion().c_str()); - if (!c.GetDescription().empty()) - fprintf(output_file, " -- %s", c.GetDescription().c_str()); - fprintf(output_file, "\n"); + if (c.GetCompletion().empty()) + continue; + + // Print the leading padding. + fprintf(output_file, " "); + + // Print the completion with trailing padding to the description column if + // that fits on the screen. Otherwise print whatever fits on the screen + // followed by ellipsis. + const size_t completion_length = c.GetCompletion().size(); + if (padding_length + completion_length < max_length) { + fprintf(output_file, "%-*s", + static_cast(description_col - padding_length), + c.GetCompletion().c_str()); + } else { + // If the completion doesn't fit on the screen, print ellipsis and don't + // bother with the description. + fprintf(output_file, "%.*s...\n\n", + static_cast(max_length - padding_length - ellipsis_length), + c.GetCompletion().c_str()); + continue; + } + + // If we don't have a description, or we don't have enough space left to + // print the separator followed by the ellipsis, we're done. + if (c.GetDescription().empty() || + description_col + separator_length + ellipsis_length >= max_length) { + fprintf(output_file, "\n"); + continue; + } + + // Print the separator. + fprintf(output_file, " -- "); + + // Descriptions can contain newlines. We want to print them below each + // other, aligned after the separator. For example, foo has a + // two-line description: + // + // foo -- Something that fits on the line. + // More information below. + // + // However, as soon as a line exceed the available screen width and + // print ellipsis, we don't print the next line. For example, foo has a + // three-line description: + // + // foo -- Something that fits on the line. + // Something much longer that doesn't fit... + // + // Because we had to print ellipsis on line two, we don't print the + // third line. + bool first = true; + for (llvm::StringRef line : llvm::split(c.GetDescription(), '\n')) { + if (line.empty()) + break; + if (!first) + fprintf(output_file, "%*s", + static_cast(description_col + separator_length), ""); + + first = false; + const size_t position = description_col + separator_length; + const size_t description_length = line.size(); + if (position + description_length < max_length) { + fprintf(output_file, "%.*s\n", static_cast(description_length), + line.data()); + } else { + fprintf(output_file, "%.*s...\n", + static_cast(max_length - position - ellipsis_length), + line.data()); + continue; + } + } } } @@ -953,7 +1027,8 @@ void Editline::DisplayCompletions( const size_t max_len = longest->GetCompletion().size(); if (results.size() < page_size) { - PrintCompletion(editline.m_output_file, results, max_len); + PrintCompletion(editline.m_output_file, results, max_len, + editline.GetTerminalWidth()); return; } @@ -963,7 +1038,7 @@ void Editline::DisplayCompletions( size_t next_size = all ? remaining : std::min(page_size, remaining); PrintCompletion(editline.m_output_file, results.slice(cur_pos, next_size), - max_len); + max_len, editline.GetTerminalWidth()); cur_pos += next_size; diff --git a/lldb/test/API/terminal/TestEditlineCompletions.py b/lldb/test/API/terminal/TestEditlineCompletions.py new file mode 100644 index 0000000000000..7fa6f95c130c6 --- /dev/null +++ b/lldb/test/API/terminal/TestEditlineCompletions.py @@ -0,0 +1,64 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +from lldbsuite.test.lldbpexpect import PExpectTest + + +class EditlineCompletionsTest(PExpectTest): + @skipIfAsan + @skipIfEditlineSupportMissing + def test_completion_truncated(self): + """Test that the completion is correctly truncated.""" + self.launch(dimensions=(10, 20)) + self.child.send("_regexp-\t") + self.child.expect(" _regexp-a...") + self.child.expect(" _regexp-b...") + + @skipIfAsan + @skipIfEditlineSupportMissing + def test_description_truncated(self): + """Test that the description is correctly truncated.""" + self.launch(dimensions=(10, 70)) + self.child.send("_regexp-\t") + self.child.expect( + " _regexp-attach -- Attach to process by ID or name." + ) + self.child.expect( + " _regexp-break -- Set a breakpoint using one of several..." + ) + + @skipIfAsan + @skipIfEditlineSupportMissing + def test_separator_omitted(self): + """Test that the separated is correctly omitted.""" + self.launch(dimensions=(10, 32)) + self.child.send("_regexp-\t") + self.child.expect(" _regexp-attach \r\n") + self.child.expect(" _regexp-break \r\n") + + @skipIfAsan + @skipIfEditlineSupportMissing + def test_separator(self): + """Test that the separated is correctly printed.""" + self.launch(dimensions=(10, 33)) + self.child.send("_regexp-\t") + self.child.expect(" _regexp-attach -- A...") + self.child.expect(" _regexp-break -- S...") + + @skipIfAsan + @skipIfEditlineSupportMissing + def test_multiline_description(self): + """Test that multi-line descriptions are correctly padded and truncated.""" + self.launch(dimensions=(10, 72)) + self.child.send("k\t") + 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(" kill -- Terminate the current target process.") From bf1c3c2deea988b6bb04c46cc9817c826cdf6cad Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 19 Nov 2024 16:07:51 -0800 Subject: [PATCH 2/2] [lldb] Update TestSwiftREPLCompletion for completion changes --- .../test/API/lang/swift/completion/TestSwiftREPLCompletion.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/lang/swift/completion/TestSwiftREPLCompletion.py b/lldb/test/API/lang/swift/completion/TestSwiftREPLCompletion.py index 83b4ea98fbe8d..52fdb0e07aac7 100644 --- a/lldb/test/API/lang/swift/completion/TestSwiftREPLCompletion.py +++ b/lldb/test/API/lang/swift/completion/TestSwiftREPLCompletion.py @@ -29,8 +29,8 @@ def test_basic_completion(self): # Try completing something that has multiple completions. self.child.send("Hash\t") self.child.expect_exact("Available completions:") - self.child.expect_exact("\tHash --") - self.child.expect_exact("\tHash --") + self.child.expect_exact(" Hash --") + self.child.expect_exact(" Hash --") self.child.sendline("") self.quit()