[Lldb-commits] [lldb] 4f7fb13 - [lldb] Don't save empty expressions in the multiline editor history

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-08-31T18:51:18+02:00
New Revision: 4f7fb13f87e10bd2cd89ccf2be70b026032237a7

URL: https://github.com/llvm/llvm-project/commit/4f7fb13f87e10bd2cd89ccf2be70b026032237a7
DIFF: https://github.com/llvm/llvm-project/commit/4f7fb13f87e10bd2cd89ccf2be70b026032237a7.diff

LOG: [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

Added: 
    

Modified: 
    lldb/source/Host/common/Editline.cpp
    lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index bcc174325195a..5bfb612c63bdb 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1554,8 +1554,10 @@ bool Editline::GetLines(int first_line_number, StringList &lines,
 
   interrupted = m_editor_status == EditorStatus::Interrupted;
   if (!interrupted) {
-    // Save the completed entry in history before returning
-    m_history_sp->Enter(CombineLines(m_input_lines).c_str());
+    // Save the completed entry in history before returning. Don't save empty
+    // input as that just clutters the command history.
+    if (m_input_lines.size() > 1 || !m_input_lines.front().empty())
+      m_history_sp->Enter(CombineLines(m_input_lines).c_str());
 
     lines = GetInputAsStringList();
   }

diff  --git a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
index 369ae3fb57a4a..938934f1d3fa4 100644
--- a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
+++ b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
@@ -76,22 +76,32 @@ def test_nav_arrow_down(self):
     @skipIfEditlineSupportMissing
     @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr48316')
     @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on buildbot
-    def test_nav_arrow_up_empty_pr49845(self):
-        """Tests that navigating with the up arrow doesn't crash."""
+    def test_nav_arrow_up_empty(self):
+        """
+        Tests that navigating with the up arrow doesn't crash and skips
+        empty history entries.
+        """
         self.launch()
 
-        # Create an empty history session by only entering a newline.
+        # Create a real history entry '456' and then follow up with an
+        # empty entry (that shouldn't be saved).
+        self.child.sendline("expr")
+        self.child.expect_exact("terminate with an empty line to evaluate")
+        self.child.send("456\n\n")
+        self.expect_prompt()
+
         self.child.sendline("expr")
         self.child.expect_exact("terminate with an empty line to evaluate")
         self.child.send("\n")
         self.expect_prompt()
 
-        # Send just the up arrow in the expression evaluator. This should bring up the previous empty expression.
+        # The up arrow should recall the actual history entry and not the
+        # the empty entry (as that one shouldn't have been saved).
         self.child.sendline("expr")
         self.child.expect_exact("terminate with an empty line to evaluate")
         self.child.send(self.arrow_up)
-        self.child.expect_exact("1: ")
-        self.child.send("\n")
+        self.child.expect_exact("456")
+        self.child.send("\n\n")
         self.expect_prompt()
 
         self.quit()