[Lldb-commits] [lldb] [lldb] Improve editline completion formatting (PR #116456)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 18 10:11:42 PST 2024
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/116456
>From 3434c7de701e4851b4eefe25f23e49d415ba4dd3 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 15 Nov 2024 16:06:47 -0800
Subject: [PATCH 1/2] [lldb] Improve editline completion formatting
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
---
lldb/include/lldb/Host/Editline.h | 2 +
lldb/source/Host/common/Editline.cpp | 95 ++++++++++++++++++++++++++--
2 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 57e2c831e3499d..e8e8a6c0d4f67e 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 f95f854c5f220c..109cc897b853f3 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -927,12 +927,92 @@ unsigned char Editline::BufferEndCommand(int ch) {
static void
PrintCompletion(FILE *output_file,
llvm::ArrayRef<CompletionResult::Completion> results,
- size_t max_len) {
+ size_t max_completion_length, size_t max_lenght) {
+
+ constexpr size_t ellipsis_length = 3;
+ constexpr size_t tab_legnth = 8;
+ constexpr size_t separator_length = 4;
+ const size_t description_col =
+ std::min(max_completion_length + tab_legnth, max_lenght);
+
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");
+ // Print the leading tab-sized padding.
+ fprintf(output_file, " ");
+ size_t cursor = tab_legnth;
+
+ if (!c.GetCompletion().empty()) {
+ const size_t completion_length = c.GetCompletion().size();
+ if (cursor + completion_length < max_lenght) {
+ fprintf(output_file, "%s", c.GetCompletion().c_str());
+ cursor = cursor + completion_length;
+ } else {
+ // If the completion doesn't fit on the screen, print ellipsis and don't
+ // bother with the description.
+ fprintf(output_file, "%s...\n",
+ c.GetCompletion()
+ .substr(0, max_lenght - cursor - ellipsis_length)
+ .c_str());
+ continue;
+ }
+ }
+
+ if (!c.GetDescription().empty()) {
+ // If we have a description, we need at least 4 columns for the separator.
+ if (cursor + separator_length < max_lenght) {
+ // Add padding before the separator.
+ if (cursor < description_col) {
+ std::string padding(description_col - cursor, ' ');
+ fprintf(output_file, "%s", padding.c_str());
+ cursor = description_col;
+ }
+
+ // Print the separator.
+ fprintf(output_file, " -- ");
+ cursor = cursor + separator_length;
+
+ // 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.
+ llvm::StringRef tail = c.GetDescription();
+ while (!tail.empty()) {
+ llvm::StringRef head;
+ std::tie(head, tail) = tail.split('\n');
+
+ const size_t description_lenth = head.size();
+ if (cursor + description_lenth < max_lenght) {
+ fprintf(output_file, "%s\n", head.str().c_str());
+ cursor = cursor + description_lenth;
+ } else {
+ fprintf(output_file, "%s...\n",
+ head.substr(0, max_lenght - cursor - ellipsis_length)
+ .str()
+ .c_str());
+ continue;
+ }
+
+ if (!tail.empty()) {
+ std::string padding(description_col + 4, ' ');
+ fprintf(output_file, "%s", padding.c_str());
+ cursor = description_col + 4;
+ }
+ }
+ }
+ } else {
+ fprintf(output_file, "\n");
+ }
}
}
@@ -953,7 +1033,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 +1044,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;
>From f6c8827579a72d828acf779d257f7ff11b9b65d4 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 18 Nov 2024 10:11:26 -0800
Subject: [PATCH 2/2] Address Pavel's code review comments
---
lldb/source/Host/common/Editline.cpp | 142 +++++++++++++--------------
1 file changed, 69 insertions(+), 73 deletions(-)
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 109cc897b853f3..3ecf973f29f269 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -927,92 +927,88 @@ unsigned char Editline::BufferEndCommand(int ch) {
static void
PrintCompletion(FILE *output_file,
llvm::ArrayRef<CompletionResult::Completion> results,
- size_t max_completion_length, size_t max_lenght) {
-
+ size_t max_completion_length, size_t max_length) {
constexpr size_t ellipsis_length = 3;
- constexpr size_t tab_legnth = 8;
+ constexpr size_t padding_length = 8;
constexpr size_t separator_length = 4;
+
const size_t description_col =
- std::min(max_completion_length + tab_legnth, max_lenght);
+ std::min(max_completion_length + padding_length, max_length);
for (const CompletionResult::Completion &c : results) {
- // Print the leading tab-sized padding.
+ if (c.GetCompletion().empty())
+ continue;
+ ;
+
+ // Print the leading padding.
fprintf(output_file, " ");
- size_t cursor = tab_legnth;
- if (!c.GetCompletion().empty()) {
- const size_t completion_length = c.GetCompletion().size();
- if (cursor + completion_length < max_lenght) {
- fprintf(output_file, "%s", c.GetCompletion().c_str());
- cursor = cursor + completion_length;
+ // 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<int>(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",
+ c.GetCompletion()
+ .substr(0, max_length - padding_length - ellipsis_length)
+ .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<int>(description_col + separator_length), "");
+
+ first = false;
+ const size_t position = description_col + separator_length;
+ const size_t description_lenth = line.size();
+ if (position + description_lenth < max_length) {
+ fprintf(output_file, "%s\n", line.str().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",
- c.GetCompletion()
- .substr(0, max_lenght - cursor - ellipsis_length)
+ line.substr(0, max_length - position - ellipsis_length)
+ .str()
.c_str());
continue;
}
}
-
- if (!c.GetDescription().empty()) {
- // If we have a description, we need at least 4 columns for the separator.
- if (cursor + separator_length < max_lenght) {
- // Add padding before the separator.
- if (cursor < description_col) {
- std::string padding(description_col - cursor, ' ');
- fprintf(output_file, "%s", padding.c_str());
- cursor = description_col;
- }
-
- // Print the separator.
- fprintf(output_file, " -- ");
- cursor = cursor + separator_length;
-
- // 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.
- llvm::StringRef tail = c.GetDescription();
- while (!tail.empty()) {
- llvm::StringRef head;
- std::tie(head, tail) = tail.split('\n');
-
- const size_t description_lenth = head.size();
- if (cursor + description_lenth < max_lenght) {
- fprintf(output_file, "%s\n", head.str().c_str());
- cursor = cursor + description_lenth;
- } else {
- fprintf(output_file, "%s...\n",
- head.substr(0, max_lenght - cursor - ellipsis_length)
- .str()
- .c_str());
- continue;
- }
-
- if (!tail.empty()) {
- std::string padding(description_col + 4, ' ');
- fprintf(output_file, "%s", padding.c_str());
- cursor = description_col + 4;
- }
- }
- }
- } else {
- fprintf(output_file, "\n");
- }
}
}
More information about the lldb-commits
mailing list