Skip to content

Commit 4f7fb13

Browse files
committed
[lldb] Don't save empty expressions in the multiline editor history
Right now running `expr` to start the multiline expression editor and then pressing enter causes an empty history empty to be created for the multiline editor. That doesn't seem very useful for users as pressing the 'up' key will now also bring up these empty expressions. I don't think there is ever a use case for recalling a completely empty expression from the history, so instead don't save those entries to the history file and make sure we never recall them when navigating over the expression history. Note: This is actually a Swift downstream patch that got shipped with Apple's LLDB for many years. However, this recently started conflicting with upstream LLDB as D100048 added a test that made sure that empty expression entries don't crash LLDB. Apple's LLDB was never affected by this crash as it never saved empty expressions in the first place. Reviewed By: augusto2112 Differential Revision: https://reviews.llvm.org/D108983
1 parent 9c37eda commit 4f7fb13

File tree

2 files changed

+20
-8
lines changed

2 files changed

+20
-8
lines changed

lldb/source/Host/common/Editline.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,8 +1554,10 @@ bool Editline::GetLines(int first_line_number, StringList &lines,
15541554

15551555
interrupted = m_editor_status == EditorStatus::Interrupted;
15561556
if (!interrupted) {
1557-
// Save the completed entry in history before returning
1558-
m_history_sp->Enter(CombineLines(m_input_lines).c_str());
1557+
// Save the completed entry in history before returning. Don't save empty
1558+
// input as that just clutters the command history.
1559+
if (m_input_lines.size() > 1 || !m_input_lines.front().empty())
1560+
m_history_sp->Enter(CombineLines(m_input_lines).c_str());
15591561

15601562
lines = GetInputAsStringList();
15611563
}

lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,32 @@ def test_nav_arrow_down(self):
7676
@skipIfEditlineSupportMissing
7777
@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr48316')
7878
@skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on buildbot
79-
def test_nav_arrow_up_empty_pr49845(self):
80-
"""Tests that navigating with the up arrow doesn't crash."""
79+
def test_nav_arrow_up_empty(self):
80+
"""
81+
Tests that navigating with the up arrow doesn't crash and skips
82+
empty history entries.
83+
"""
8184
self.launch()
8285

83-
# Create an empty history session by only entering a newline.
86+
# Create a real history entry '456' and then follow up with an
87+
# empty entry (that shouldn't be saved).
88+
self.child.sendline("expr")
89+
self.child.expect_exact("terminate with an empty line to evaluate")
90+
self.child.send("456\n\n")
91+
self.expect_prompt()
92+
8493
self.child.sendline("expr")
8594
self.child.expect_exact("terminate with an empty line to evaluate")
8695
self.child.send("\n")
8796
self.expect_prompt()
8897

89-
# Send just the up arrow in the expression evaluator. This should bring up the previous empty expression.
98+
# The up arrow should recall the actual history entry and not the
99+
# the empty entry (as that one shouldn't have been saved).
90100
self.child.sendline("expr")
91101
self.child.expect_exact("terminate with an empty line to evaluate")
92102
self.child.send(self.arrow_up)
93-
self.child.expect_exact("1: ")
94-
self.child.send("\n")
103+
self.child.expect_exact("456")
104+
self.child.send("\n\n")
95105
self.expect_prompt()
96106

97107
self.quit()

0 commit comments

Comments
 (0)