[Lldb-commits] [lldb] d9166ad - [lldb/Driver] Support terminal resizing
Fred Riss via lldb-commits
lldb-commits at lists.llvm.org
Tue May 12 11:56:16 PDT 2020
Author: Fred Riss
Date: 2020-05-12T11:55:25-07:00
New Revision: d9166ad272847e246799afbb5e0c71874f83aa12
URL: https://github.com/llvm/llvm-project/commit/d9166ad272847e246799afbb5e0c71874f83aa12
DIFF: https://github.com/llvm/llvm-project/commit/d9166ad272847e246799afbb5e0c71874f83aa12.diff
LOG: [lldb/Driver] Support terminal resizing
Summary:
The comment in the Editine.h header made it sound like editline was
just unable to handle terminal resizing. We were not ever telling
editline that the terminal had changed size, which might explain why
it wasn't working.
This patch threads a `TerminalSizeChanged()` callback through the
IOHandler and invokes it from the SIGWINCH handler in the driver. Our
`Editline` class already had a `TerminalSizeChanged()` method which
was invoked only when editline was configured.
This patch also changes `Editline` to not apply the changes right away
in `TerminalSizeChanged()`, but instead defer that to the next
character read. During my testing, it happened once that the signal
was received while our `ConnectionFileDescriptor::Read` was allocating
memory. As `el_resize` seems to allocate memory too, this crashed.
Reviewers: labath, teemperor
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D79654
Added:
lldb/test/API/iohandler/resize/TestIOHandlerResize.py
Modified:
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/source/Core/Debugger.cpp
lldb/source/Core/IOHandler.cpp
lldb/source/Host/common/Editline.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h
index ff75bfb15cfb..539ba04ab84c 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -95,6 +95,8 @@ class IOHandler {
virtual void Deactivate() { m_active = false; }
+ virtual void TerminalSizeChanged() {}
+
virtual const char *GetPrompt() {
// Prompt support isn't mandatory
return nullptr;
@@ -369,6 +371,8 @@ class IOHandlerEditline : public IOHandler {
void Deactivate() override;
+ void TerminalSizeChanged() override;
+
ConstString GetControlSequence(char ch) override {
return m_delegate.IOHandlerGetControlSequence(ch);
}
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 6590a1603bf0..6e9daae42217 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -19,13 +19,10 @@
// good amount of the text will
// disappear. It's still in the buffer, just invisible.
// b) The prompt printing logic for dealing with ANSI formatting characters is
-// broken, which is why we're
-// working around it here.
-// c) When resizing the terminal window, if the cursor moves between rows
-// libedit will get confused. d) The incremental search uses escape to cancel
-// input, so it's confused by
+// broken, which is why we're working around it here.
+// c) The incremental search uses escape to cancel input, so it's confused by
// ANSI sequences starting with escape.
-// e) Emoji support is fairly terrible, presumably it doesn't understand
+// d) Emoji support is fairly terrible, presumably it doesn't understand
// composed characters?
#ifndef LLDB_HOST_EDITLINE_H
@@ -50,6 +47,7 @@
#include <histedit.h>
#endif
+#include <csignal>
#include <mutex>
#include <string>
#include <vector>
@@ -171,9 +169,7 @@ class Editline {
/// editing scenarios.
void SetContinuationPrompt(const char *continuation_prompt);
- /// Required to update the width of the terminal registered for I/O. It is
- /// critical that this
- /// be correct at all times.
+ /// Call when the terminal size changes
void TerminalSizeChanged();
/// Returns the prompt established by SetPrompt()
@@ -328,6 +324,8 @@ class Editline {
bool CompleteCharacter(char ch, EditLineGetCharType &out);
+ void ApplyTerminalSizeChange();
+
private:
#if LLDB_EDITLINE_USE_WCHAR
std::wstring_convert<std::codecvt_utf8<wchar_t>> m_utf8conv;
@@ -350,6 +348,7 @@ class Editline {
std::string m_set_continuation_prompt;
std::string m_current_prompt;
bool m_needs_prompt_repaint = false;
+ volatile std::sig_atomic_t m_terminal_size_has_changed = 0;
std::string m_editor_name;
FILE *m_input_file;
FILE *m_output_file;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 1d4bf0dafb72..546dc9e86e7d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -315,6 +315,9 @@ uint32_t Debugger::GetTerminalWidth() const {
}
bool Debugger::SetTerminalWidth(uint32_t term_width) {
+ if (auto handler_sp = m_io_handler_stack.Top())
+ handler_sp->TerminalSizeChanged();
+
const uint32_t idx = ePropertyTerminalWidth;
return m_collection_sp->SetPropertyAtIndexAsSInt64(nullptr, idx, term_width);
}
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 933da6bfbf75..e29e70c030df 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -289,6 +289,12 @@ void IOHandlerEditline::Deactivate() {
m_delegate.IOHandlerDeactivated(*this);
}
+void IOHandlerEditline::TerminalSizeChanged() {
+#if LLDB_ENABLE_LIBEDIT
+ m_editline_up->TerminalSizeChanged();
+#endif
+}
+
// Split out a line from the buffer, if there is a full one to get.
static Optional<std::string> SplitLine(std::string &line_buffer) {
size_t pos = line_buffer.find('\n');
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index b72e951a2e28..56203e37dec0 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -560,6 +560,9 @@ int Editline::GetCharacter(EditLineGetCharType *c) {
lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
char ch = 0;
+ if (m_terminal_size_has_changed)
+ ApplyTerminalSizeChange();
+
// This mutex is locked by our caller (GetLine). Unlock it while we read a
// character (blocking operation), so we do not hold the mutex
// indefinitely. This gives a chance for someone to interrupt us. After
@@ -1052,7 +1055,7 @@ void Editline::ConfigureEditor(bool multiline) {
m_editline =
el_init(m_editor_name.c_str(), m_input_file, m_output_file, m_error_file);
- TerminalSizeChanged();
+ ApplyTerminalSizeChange();
if (m_history_sp && m_history_sp->IsValid()) {
if (!m_history_sp->Load()) {
@@ -1305,28 +1308,32 @@ void Editline::SetContinuationPrompt(const char *continuation_prompt) {
continuation_prompt == nullptr ? "" : continuation_prompt;
}
-void Editline::TerminalSizeChanged() {
- if (m_editline != nullptr) {
- el_resize(m_editline);
- int columns;
- // This function is documenting as taking (const char *, void *) for the
- // vararg part, but in reality in was consuming arguments until the first
- // null pointer. This was fixed in libedit in April 2019
- // <http://mail-index.netbsd.org/source-changes/2019/04/26/msg105454.html>,
- // but we're keeping the workaround until a version with that fix is more
- // widely available.
- if (el_get(m_editline, EL_GETTC, "co", &columns, nullptr) == 0) {
- m_terminal_width = columns;
- if (m_current_line_rows != -1) {
- const LineInfoW *info = el_wline(m_editline);
- int lineLength =
- (int)((info->lastchar - info->buffer) + GetPromptWidth());
- m_current_line_rows = (lineLength / columns) + 1;
- }
- } else {
- m_terminal_width = INT_MAX;
- m_current_line_rows = 1;
+void Editline::TerminalSizeChanged() { m_terminal_size_has_changed = 1; }
+
+void Editline::ApplyTerminalSizeChange() {
+ if (!m_editline)
+ return;
+
+ m_terminal_size_has_changed = 0;
+ el_resize(m_editline);
+ int columns;
+ // This function is documenting as taking (const char *, void *) for the
+ // vararg part, but in reality in was consuming arguments until the first
+ // null pointer. This was fixed in libedit in April 2019
+ // <http://mail-index.netbsd.org/source-changes/2019/04/26/msg105454.html>,
+ // but we're keeping the workaround until a version with that fix is more
+ // widely available.
+ if (el_get(m_editline, EL_GETTC, "co", &columns, nullptr) == 0) {
+ m_terminal_width = columns;
+ if (m_current_line_rows != -1) {
+ const LineInfoW *info = el_wline(m_editline);
+ int lineLength =
+ (int)((info->lastchar - info->buffer) + GetPromptWidth());
+ m_current_line_rows = (lineLength / columns) + 1;
}
+ } else {
+ m_terminal_width = INT_MAX;
+ m_current_line_rows = 1;
}
}
diff --git a/lldb/test/API/iohandler/resize/TestIOHandlerResize.py b/lldb/test/API/iohandler/resize/TestIOHandlerResize.py
new file mode 100644
index 000000000000..488553fce724
--- /dev/null
+++ b/lldb/test/API/iohandler/resize/TestIOHandlerResize.py
@@ -0,0 +1,36 @@
+"""
+Test resizing in our IOHandlers.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class IOHandlerCompletionTest(PExpectTest):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ # PExpect uses many timeouts internally and doesn't play well
+ # under ASAN on a loaded machine..
+ @skipIfAsan
+ @skipIfEditlineSupportMissing
+ def test_resize(self):
+
+ # Start with a small window
+ self.launch(dimensions=(10,10))
+
+ self.child.send("his is a long sentence missing its first letter.")
+
+ # Now resize to something bigger
+ self.child.setwinsize(100,500)
+
+ # Hit "left" 60 times (to go to the beginning of the line) and insert
+ # a character.
+ self.child.send(60 * "\033[D")
+ self.child.send("T")
+
+ self.child.expect_exact("(lldb) This is a long sentence missing its first letter.")
+ self.quit()
More information about the lldb-commits
mailing list