friss created this revision.
friss added reviewers: labath, teemperor.
Herald added a project: LLDB.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79654
Files:
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
lldb/test/API/iohandler/resize/TestIOHandlerResize.py
Index: lldb/test/API/iohandler/resize/TestIOHandlerResize.py
===================================================================
--- /dev/null
+++ 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()
Index: lldb/source/Host/common/Editline.cpp
===================================================================
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -560,6 +560,9 @@
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 @@
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 @@
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 = true; }
+
+void Editline::ApplyTerminalSizeChange() {
+ if (!m_editline)
+ return;
+
+ m_terminal_size_has_changed = false;
+ 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;
}
}
Index: lldb/source/Core/IOHandler.cpp
===================================================================
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -289,6 +289,12 @@
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');
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -315,6 +315,9 @@
}
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);
}
Index: lldb/include/lldb/Host/Editline.h
===================================================================
--- lldb/include/lldb/Host/Editline.h
+++ 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
@@ -171,9 +168,7 @@
/// 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 +323,8 @@
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 +347,7 @@
std::string m_set_continuation_prompt;
std::string m_current_prompt;
bool m_needs_prompt_repaint = false;
+ bool m_terminal_size_has_changed = false;
std::string m_editor_name;
FILE *m_input_file;
FILE *m_output_file;
Index: lldb/include/lldb/Core/IOHandler.h
===================================================================
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -95,6 +95,8 @@
virtual void Deactivate() { m_active = false; }
+ virtual void TerminalSizeChanged() {}
+
virtual const char *GetPrompt() {
// Prompt support isn't mandatory
return nullptr;
@@ -369,6 +371,8 @@
void Deactivate() override;
+ void TerminalSizeChanged() override;
+
ConstString GetControlSequence(char ch) override {
return m_delegate.IOHandlerGetControlSequence(ch);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
