[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: kastiglione, clayborg.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

At present, if you run a regex command (e.g. the `b` command) you will get two history entries, one for the actual command the user typed, and one for the resolved command:

  (lldb) b foo.c:12
  Breakpoint 1: no locations (pending).
  Breakpoint set in dummy target, will get copied into future targets.
  (lldb) history
     0: b foo.c:12
     1: breakpoint set --file 'foo.c' --line 12
     2: history

That seems wrong in principle, there shouldn't be double entry like this.  But it also has some subtle side effects that are even less desirable.  For instance, if you write a Python based command that calls "HandleCommand" of a regex command, then the resolved command gets inserted into the history list AFTER the python command the user actually ran.  That in turn means that up-arrow doesn't actually rerun the command the user typed, but instead runs the resolved regex command.

Since regex command resolution isn't stateful - the same input string will always resolve to the same command - there's no actual reason to do this double entry, just storing the command the user typed will suffice.  And there's already a way to see resolved commands, so you don't need it for this purpose either.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126789

Files:
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/test/API/functionalities/history/TestHistoryRecall.py


Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===================================================================
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
 """
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
 """
 
 
@@ -20,9 +20,25 @@
 
     def test_history_recall(self):
         """Test the !N and !-N functionality of the command interpreter."""
-        self.sample_test()
+        self.do_bang_N_test()
 
-    def sample_test(self):
+    def test_regex_history(self):
+        """Test the regex commands don't add two elements to the history"""
+        self.do_regex_history_test()
+
+    def do_regex_history_test(self):
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        command = "_regexp-break foo.c:12"
+        self.runCmd(command, msg="Run the regex break command", inHistory = True)
+        interp.HandleCommand("session history", result, True)
+        self.assertTrue(result.Succeeded(), "session history ran successfully")
+        results = result.GetOutput()
+        self.assertIn(command, results, "Recorded the actual command")
+        self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+                         "Didn't record the resolved command")
+        
+    def do_bang_N_test(self):
         interp = self.dbg.GetCommandInterpreter()
         result = lldb.SBCommandReturnObject()
         interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===================================================================
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -75,7 +75,7 @@
       // should have set up the context appropriately, we shouldn't have to
       // redo that.
       return m_interpreter.HandleCommand(new_command->c_str(),
-                                         eLazyBoolCalculate, result);
+                                         eLazyBoolNo, result);
     }
   }
   result.SetStatus(eReturnStatusFailed);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126789.433422.patch
Type: text/x-patch
Size: 2242 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220601/9dcee07c/attachment-0001.bin>