[Lldb-commits] [lldb] [lldb] Use the terminal height for paging editline completions (PR #119914)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 16 11:07:07 PST 2024
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 <jonas at devlieghere.com>
Date: Thu, 22 Feb 2024 21:54:07 -0800
Subject: [PATCH 1/5] [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 <jonas at devlieghere.com>
Date: Fri, 13 Dec 2024 12:57:30 -0800
Subject: [PATCH 2/5] 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 <jonas at devlieghere.com>
Date: Mon, 16 Dec 2024 08:34:02 -0800
Subject: [PATCH 3/5] 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++;
>From ae4e7a10d6f88e9c3d8fb7d47365c9f5b8ee2f2b Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 16 Dec 2024 10:25:24 -0800
Subject: [PATCH 4/5] Address Pavel's feedback
- Always pass the whole list.
- Truncate multi-line descriptions if they're about to overflow.
---
lldb/source/Host/common/Editline.cpp | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 62c27f82eeeacd..a18560fdd219d4 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -939,8 +939,6 @@ PrintCompletion(FILE *output_file,
size_t lines_printed = 0;
size_t results_printed = 0;
for (const CompletionResult::Completion &c : results) {
- // 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;
@@ -1002,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), "");
@@ -1047,11 +1047,8 @@ void Editline::DisplayCompletions(
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);
-
cur_pos += PrintCompletion(
- editline.m_output_file, results.slice(cur_pos, next_size), max_len,
+ editline.m_output_file, results.slice(cur_pos), max_len,
editline.GetTerminalWidth(),
all ? std::nullopt : std::optional<size_t>(page_size));
>From e46386b8a7dc0cb7c1bf47f22fcea03b396f519c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 16 Dec 2024 11:06:48 -0800
Subject: [PATCH 5/5] Appease the formatter
---
lldb/source/Host/common/Editline.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index a18560fdd219d4..6e35b15d69651d 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1047,10 +1047,10 @@ void Editline::DisplayCompletions(
size_t cur_pos = 0;
while (cur_pos < results.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));
+ 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;
More information about the lldb-commits
mailing list