Author: Jonas Devlieghere Date: 2019-12-03T08:12:10-08:00 New Revision: 0e9b0b6d11e882efec8505d97c4b65e1562e6715
URL: https://github.com/llvm/llvm-project/commit/0e9b0b6d11e882efec8505d97c4b65e1562e6715 DIFF: https://github.com/llvm/llvm-project/commit/0e9b0b6d11e882efec8505d97c4b65e1562e6715.diff LOG: [EditLine] Fix RecallHistory to make it go in the right direction. 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. Differential revision: https://reviews.llvm.org/D70932 Added: Modified: lldb/include/lldb/Host/Editline.h lldb/source/Host/common/Editline.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h index 65bf15531bc4..0cb2c6c5b6a1 100644 --- a/lldb/include/lldb/Host/Editline.h +++ b/lldb/include/lldb/Host/Editline.h @@ -133,6 +133,15 @@ enum class CursorLocation { /// session BlockEnd }; + +/// Operation for the history. +enum class HistoryOperation { + Oldest, + Older, + Current, + Newer, + Newest +}; } using namespace line_editor; @@ -258,11 +267,7 @@ class Editline { 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. diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index 3ae837866faf..45d3db24bbc6 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -97,6 +97,32 @@ bool IsOnlySpaces(const EditLineStringType &content) { 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 @@ StringList Editline::GetInputAsStringList(int line_count) { 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 @@ unsigned char Editline::RecallHistory(bool earlier) { // Treat moving from the "live" entry diff erently 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 @@ unsigned char Editline::RecallHistory(bool earlier) { // 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 @@ unsigned char Editline::PreviousLineCommand(int ch) { SaveEditedLine(); if (m_current_line_index == 0) { - return RecallHistory(true); + return RecallHistory(HistoryOperation::Older); } // Start from a known location @@ -747,7 +794,7 @@ unsigned char Editline::NextLineCommand(int ch) { // 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::NextLineCommand(int ch) { 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) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
