[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()
wallace marked an inline comment as done. wallace added a comment. @aprantl , it turns out that there are no tests for this. I also don't know how easy it'd be to test this very specific feature, because it relies on the terminal not being affected by stuff like test runners. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159031/new/ https://reviews.llvm.org/D159031 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()
wallace updated this revision to Diff 554712.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159031/new/
https://reviews.llvm.org/D159031
Files:
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/source/Core/IOHandler.cpp
lldb/source/Expression/REPL.cpp
Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
current_code.append(m_code.CopyList());
IOHandlerEditline = static_cast(io_handler);
- const StringList *current_lines = editline.GetCurrentLines();
- if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
- for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
- current_code.append("\n");
- current_code.append(line_cstr);
-}
+ StringList current_lines = editline.GetCurrentLines();
+ const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+ if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+ const char *line_cstr = current_lines.GetStringAtIndex(i);
+ if (line_cstr) {
+current_code.append("\n");
+current_code.append(line_cstr);
}
}
}
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -508,6 +508,21 @@
return m_curr_line_idx;
}
+StringList IOHandlerEditline::GetCurrentLines() const {
+#if LLDB_ENABLE_LIBEDIT
+ if (m_editline_up)
+return m_editline_up->GetInputAsStringList();
+#endif
+ // When libedit is not used, the current lines can be gotten from
+ // `m_current_lines_ptr`, which is updated whenever a new line is processed.
+ // This doesn't happen when libedit is used, in which case
+ // `m_current_lines_ptr` is only updated when the full input is terminated.
+
+ if (m_current_lines_ptr)
+return *m_current_lines_ptr;
+ return StringList();
+}
+
bool IOHandlerEditline::GetLines(StringList , bool ) {
m_current_lines_ptr =
Index: lldb/include/lldb/Host/Editline.h
===
--- lldb/include/lldb/Host/Editline.h
+++ lldb/include/lldb/Host/Editline.h
@@ -227,6 +227,9 @@
void PrintAsync(Stream *stream, const char *s, size_t len);
+ /// Convert the current input lines into a UTF8 StringList
+ StringList GetInputAsStringList(int line_count = UINT32_MAX);
+
private:
/// Sets the lowest line number for multi-line editing sessions. A value of
/// zero suppresses
@@ -282,9 +285,6 @@
/// Save the line currently being edited
void SaveEditedLine();
- /// Convert the current input lines into a UTF8 StringList
- StringList GetInputAsStringList(int line_count = UINT32_MAX);
-
/// Replaces the current multi-line session with the next entry from history.
unsigned char RecallHistory(HistoryOperation op);
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -403,7 +403,7 @@
void SetInterruptExits(bool b) { m_interrupt_exits = b; }
- const StringList *GetCurrentLines() const { return m_current_lines_ptr; }
+ StringList GetCurrentLines() const;
uint32_t GetCurrentLineIndex() const;
Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
current_code.append(m_code.CopyList());
IOHandlerEditline = static_cast(io_handler);
- const StringList *current_lines = editline.GetCurrentLines();
- if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
- for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
- current_code.append("\n");
- current_code.append(line_cstr);
-}
+ StringList current_lines = editline.GetCurrentLines();
+ const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+ if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+ const char *line_cstr = current_lines.GetStringAtIndex(i);
+ if (line_cstr) {
+current_code.append("\n");
+current_code.append(line_cstr);
}
}
}
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++
[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()
wallace added inline comments.
Comment at: lldb/source/Expression/REPL.cpp:535
+ if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+ const char *line_cstr = current_lines.GetStringAtIndex(i);
aprantl wrote:
> `for (const char *line_cstr : current_lines)` ?
actually the current code is fine because we don't want to iterate through the
entire set, we just want the first `current_line_idx + 1` elements
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159031/new/
https://reviews.llvm.org/D159031
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()
aprantl added a comment.
Is this covered by any tests?
Comment at: lldb/source/Core/IOHandler.cpp:512
+StringList IOHandlerEditline::GetCurrentLines() const {
+#if LLDB_ENABLE_LIBEDIT
+ if (m_editline_up)
I think this would benefit from a comment explaining the difference?
Comment at: lldb/source/Expression/REPL.cpp:535
+ if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+ const char *line_cstr = current_lines.GetStringAtIndex(i);
`for (const char *line_cstr : current_lines)` ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159031/new/
https://reviews.llvm.org/D159031
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D159031: [LLDB] Fix IOHandlerEditline::GetCurrentLines()
wallace created this revision.
wallace added reviewers: bulbazord, aprantl, JDevlieghere.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This method was working as expected if `LLDB_ENABLE_LIBEDIT` is false, however,
if it was true, then the variable `m_current_lines_ptr` was always pointing to
an empty list, because Editline only updates its contents once the full input
has been completed (see
https://github.com/llvm/llvm-project/blob/main/lldb/source/Host/common/Editline.cpp#L1576).
A simple fix is to invoke `Editline::GetInputAsStringList()` from
`GetCurrentLines()`, which is already used in many places as the common way to
get the full input list.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D159031
Files:
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/source/Core/IOHandler.cpp
lldb/source/Expression/REPL.cpp
Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
current_code.append(m_code.CopyList());
IOHandlerEditline = static_cast(io_handler);
- const StringList *current_lines = editline.GetCurrentLines();
- if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
- for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
- current_code.append("\n");
- current_code.append(line_cstr);
-}
+ StringList current_lines = editline.GetCurrentLines();
+ const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+ if (current_line_idx < current_lines.GetSize()) {
+for (uint32_t i = 0; i < current_line_idx; ++i) {
+ const char *line_cstr = current_lines.GetStringAtIndex(i);
+ if (line_cstr) {
+current_code.append("\n");
+current_code.append(line_cstr);
}
}
}
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -508,6 +508,16 @@
return m_curr_line_idx;
}
+StringList IOHandlerEditline::GetCurrentLines() const {
+#if LLDB_ENABLE_LIBEDIT
+ if (m_editline_up)
+return m_editline_up->GetInputAsStringList();
+#endif
+ if (m_current_lines_ptr)
+return *m_current_lines_ptr;
+ return StringList();
+}
+
bool IOHandlerEditline::GetLines(StringList , bool ) {
m_current_lines_ptr =
Index: lldb/include/lldb/Host/Editline.h
===
--- lldb/include/lldb/Host/Editline.h
+++ lldb/include/lldb/Host/Editline.h
@@ -227,6 +227,9 @@
void PrintAsync(Stream *stream, const char *s, size_t len);
+ /// Convert the current input lines into a UTF8 StringList
+ StringList GetInputAsStringList(int line_count = UINT32_MAX);
+
private:
/// Sets the lowest line number for multi-line editing sessions. A value of
/// zero suppresses
@@ -282,9 +285,6 @@
/// Save the line currently being edited
void SaveEditedLine();
- /// Convert the current input lines into a UTF8 StringList
- StringList GetInputAsStringList(int line_count = UINT32_MAX);
-
/// Replaces the current multi-line session with the next entry from history.
unsigned char RecallHistory(HistoryOperation op);
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -403,7 +403,7 @@
void SetInterruptExits(bool b) { m_interrupt_exits = b; }
- const StringList *GetCurrentLines() const { return m_current_lines_ptr; }
+ StringList GetCurrentLines() const;
uint32_t GetCurrentLineIndex() const;
Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -528,17 +528,15 @@
current_code.append(m_code.CopyList());
IOHandlerEditline = static_cast(io_handler);
- const StringList *current_lines = editline.GetCurrentLines();
- if (current_lines) {
-const uint32_t current_line_idx = editline.GetCurrentLineIndex();
-
-if (current_line_idx < current_lines->GetSize()) {
- for (uint32_t i = 0; i < current_line_idx; ++i) {
-const char *line_cstr = current_lines->GetStringAtIndex(i);
-if (line_cstr) {
- current_code.append("\n");
- current_code.append(line_cstr);
-}
+ StringList current_lines = editline.GetCurrentLines();
+ const uint32_t current_line_idx = editline.GetCurrentLineIndex();
+
+ if (current_line_idx < current_lines.GetSize()) {
+for
