JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, teemperor, clayborg.
Herald added a subscriber: abidh.
Herald added a project: LLDB.
The naming used by editline for the history operations is counter
intuitive to how it's used in lldb for the REPL.
- The H_PREV operation returns the previous element in the history, which is
**newer** than the current one.
- The H_NEXT operation returns the next element in the history, which is
**older** than the current one.
This exposed itself as a bug in the REPL where the behavior of up- and
down-arrow was inverted. This wasn't immediately obvious because of how we save
the current "live" entry.
This patch fixes the bug and introduces and enum to wrap the editline
operations that match the semantics of lldb.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D70932
Files:
lldb/include/lldb/Host/Editline.h
lldb/source/Host/common/Editline.cpp
Index: lldb/source/Host/common/Editline.cpp
===================================================================
--- lldb/source/Host/common/Editline.cpp
+++ lldb/source/Host/common/Editline.cpp
@@ -97,6 +97,32 @@
return true;
}
+static int GetOperation(HistoryOperation op) {
+ // The naming used by editline for the history operations is counter
+ // intuitive to how it's used here.
+ //
+ // - The H_PREV operation returns the previous element in the history, which
+ // is newer than the current one.
+ //
+ // - The H_NEXT operation returns the next element in the history, which is
+ // older than the current one.
+ //
+ // The naming of the enum entries match the semantic meaning.
+ switch(op) {
+ case HistoryOperation::Oldest:
+ return H_FIRST;
+ case HistoryOperation::Older:
+ return H_NEXT;
+ case HistoryOperation::Current:
+ return H_CURR;
+ case HistoryOperation::Newer:
+ return H_PREV;
+ case HistoryOperation::Newest:
+ return H_LAST;
+ }
+}
+
+
EditLineStringType CombineLines(const std::vector<EditLineStringType> &lines) {
EditLineStringStreamType combined_stream;
for (EditLineStringType line : lines) {
@@ -423,7 +449,8 @@
return lines;
}
-unsigned char Editline::RecallHistory(bool earlier) {
+unsigned char Editline::RecallHistory(HistoryOperation op) {
+ assert(op == HistoryOperation::Older || op == HistoryOperation::Newer);
if (!m_history_sp || !m_history_sp->IsValid())
return CC_ERROR;
@@ -433,27 +460,38 @@
// Treat moving from the "live" entry differently
if (!m_in_history) {
- if (!earlier)
+ switch (op) {
+ case HistoryOperation::Newer:
return CC_ERROR; // Can't go newer than the "live" entry
- if (history_w(pHistory, &history_event, H_FIRST) == -1)
- return CC_ERROR;
-
- // Save any edits to the "live" entry in case we return by moving forward
- // in history (it would be more bash-like to save over any current entry,
- // but libedit doesn't offer the ability to add entries anywhere except the
- // end.)
- SaveEditedLine();
- m_live_history_lines = m_input_lines;
- m_in_history = true;
+ case HistoryOperation::Older: {
+ if (history_w(pHistory, &history_event,
+ GetOperation(HistoryOperation::Newest)) == -1)
+ return CC_ERROR;
+ // Save any edits to the "live" entry in case we return by moving forward
+ // in history (it would be more bash-like to save over any current entry,
+ // but libedit doesn't offer the ability to add entries anywhere except
+ // the end.)
+ SaveEditedLine();
+ m_live_history_lines = m_input_lines;
+ m_in_history = true;
+ } break;
+ default:
+ llvm_unreachable("unsupported history direction");
+ }
} else {
- if (history_w(pHistory, &history_event, earlier ? H_PREV : H_NEXT) == -1) {
- // Can't move earlier than the earliest entry
- if (earlier)
+ if (history_w(pHistory, &history_event, GetOperation(op)) == -1) {
+ switch (op) {
+ case HistoryOperation::Older:
+ // Can't move earlier than the earliest entry.
return CC_ERROR;
-
- // ... but moving to newer than the newest yields the "live" entry
- new_input_lines = m_live_history_lines;
- m_in_history = false;
+ case HistoryOperation::Newer:
+ // Moving to newer-than-the-newest entry yields the "live" entry.
+ new_input_lines = m_live_history_lines;
+ m_in_history = false;
+ break;
+ default:
+ llvm_unreachable("unsupported history direction");
+ }
}
}
@@ -468,8 +506,17 @@
// Prepare to edit the last line when moving to previous entry, or the first
// line when moving to next entry
- SetCurrentLine(m_current_line_index =
- earlier ? (int)m_input_lines.size() - 1 : 0);
+ switch (op) {
+ case HistoryOperation::Older:
+ m_current_line_index = (int)m_input_lines.size() - 1;
+ break;
+ case HistoryOperation::Newer:
+ m_current_line_index = 0;
+ break;
+ default:
+ llvm_unreachable("unsupported history direction");
+ }
+ SetCurrentLine(m_current_line_index);
MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingPrompt);
return CC_NEWLINE;
}
@@ -721,7 +768,7 @@
SaveEditedLine();
if (m_current_line_index == 0) {
- return RecallHistory(true);
+ return RecallHistory(HistoryOperation::Older);
}
// Start from a known location
@@ -747,7 +794,7 @@
// Don't add an extra line if the existing last line is blank, move through
// history instead
if (IsOnlySpaces()) {
- return RecallHistory(false);
+ return RecallHistory(HistoryOperation::Newer);
}
// Determine indentation for the new line
@@ -779,13 +826,13 @@
unsigned char Editline::PreviousHistoryCommand(int ch) {
SaveEditedLine();
- return RecallHistory(true);
+ return RecallHistory(HistoryOperation::Older);
}
unsigned char Editline::NextHistoryCommand(int ch) {
SaveEditedLine();
- return RecallHistory(false);
+ return RecallHistory(HistoryOperation::Newer);
}
unsigned char Editline::FixIndentationCommand(int ch) {
Index: lldb/include/lldb/Host/Editline.h
===================================================================
--- lldb/include/lldb/Host/Editline.h
+++ lldb/include/lldb/Host/Editline.h
@@ -133,6 +133,15 @@
/// session
BlockEnd
};
+
+/// Operation for the history.
+enum class HistoryOperation {
+ Oldest,
+ Older,
+ Current,
+ Newer,
+ Newest
+};
}
using namespace line_editor;
@@ -258,11 +267,7 @@
StringList GetInputAsStringList(int line_count = UINT32_MAX);
/// Replaces the current multi-line session with the next entry from history.
- /// When the parameter is
- /// true it will take the next earlier entry from history, when it is false it
- /// takes the next most
- /// recent.
- unsigned char RecallHistory(bool earlier);
+ unsigned char RecallHistory(HistoryOperation op);
/// Character reading implementation for EditLine that supports our multi-line
/// editing trickery.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
