[Lldb-commits] [PATCH] D64853: Fix CommandInterpreter for _regex-break with options

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:5
+
+from __future__ import print_function
+
----------------
I don't think you need this.


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:23
+        self.build()
+        self.regexp_break_command_line()
+
----------------
Why not inline the test? Now you have two doc-strings, which are slightly different. 


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:58
+        TestBase.setUp(self)
+        self.runCmd("settings set use-color false")
+        # Find the line number to break inside main().
----------------
This is no longer necessary 


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:107
+
+        break_results = lldbutil.run_break_set_command(
+            self, "b %s:%d" % (self.source, self.line))
----------------
`# Check breakpoint with relative file path.`


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:130
+        # The stop reason of the thread should be breakpoint.
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stopped',
----------------
Since this is repeated in every function, I'd factor this out into a method. 


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:171
+        break_results = lldbutil.run_break_set_command(
+            self, "b %s" %
+            function_name)
----------------
We discussed a few scenarios yesterday with single and double quotes. Any reason you didn't include those?


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/TestRegexpBreakCommand.py:243
+    def regexp_break_command_regex(self):
+        """Test the super consie "b" command with a regular expression, which is analias for _regexp-break."""
+        exe = self.getBuildArtifact("a.out")
----------------
s/consie/concise/


================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/regex_break/main.c:1
-//===-- main.c --------------------------------------------------*- C++ -*-===//
+//===-- main.c ----------------------------------------------------*- C -*-===//
 //
----------------
Just remove the header


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64853/new/

https://reviews.llvm.org/D64853