[lldb][lldb-dap] fix repeating commands in repl mode #135008

da-viper · 2025-04-09T13:42:33Z

Fixes #131589
Add a new option to the RunCommands functions to control the echoing of commands

Fixes llvm#131589 Add a new option to the RunCommands* functions to control the echoing of commands

llvmbot · 2025-04-09T13:43:12Z

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

Changes

Fixes #131589
Add a new option to the RunCommands functions to control the echoing of commands

Full diff: https://github.com/llvm/llvm-project/pull/135008.diff

8 Files Affected:

(modified) lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py (+1-1)
(modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-3)
(modified) lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py (+4-7)
(modified) lldb/tools/lldb-dap/DAP.cpp (+4-2)
(modified) lldb/tools/lldb-dap/DAP.h (+2-1)
(modified) lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp (+2-1)
(modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+26-9)
(modified) lldb/tools/lldb-dap/LLDBUtils.h (+14-5)

diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 251d77d79d080..e2f843bd337a6 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -101,7 +101,7 @@ def run_test_evaluate_expressions(
         if context == "repl":
             # In the repl context expressions may be interpreted as lldb
             # commands since no variables have the same name as the command.
-            self.assertEvaluate("list", r"\(lldb\) list\n.*")
+            self.assertEvaluate("list", r".*")
         else:
             self.assertEvaluateFailure("list")  # local variable of a_function
 
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index 64c99019a1c9b..eceba2f8a13cb 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -522,11 +522,9 @@ def test_version(self):
         )
         version_eval_output = version_eval_response["body"]["result"]
 
-        # The first line is the prompt line like "(lldb) version", so we skip it.
-        version_eval_output_without_prompt_line = version_eval_output.splitlines()[1:]
         version_string = self.dap_server.get_initialize_value("$__lldb_version")
         self.assertEqual(
-            version_eval_output_without_prompt_line,
+            version_eval_output.splitlines(),
             version_string.splitlines(),
             "version string does not match",
         )
diff --git a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py
index 7c77fc8541b93..09ca725ee8883 100644
--- a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py
+++ b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py
@@ -28,15 +28,12 @@ def test_completions(self):
 
         self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line])
 
-        self.assertEvaluate(
-            "`command regex user_command s/^$/platform/", r"\(lldb\) command regex"
-        )
-        self.assertEvaluate(
-            "`command alias alias_command platform", r"\(lldb\) command alias"
-        )
+        # the result of the commands should return the empty string.
+        self.assertEvaluate("`command regex user_command s/^$/platform/", r"^$")
+        self.assertEvaluate("`command alias alias_command platform", r"^$")
         self.assertEvaluate(
             "`command alias alias_command_with_arg platform select --sysroot %1 remote-linux",
-            r"\(lldb\) command alias",
+            r"^$",
         )
 
         self.continue_to_next_stop()
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 9361ba968e9c2..03b9dc7135ef7 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -561,10 +561,12 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
 }
 
 bool DAP::RunLLDBCommands(llvm::StringRef prefix,
-                          llvm::ArrayRef<std::string> commands) {
+                          llvm::ArrayRef<std::string> commands,
+                          bool echo_commands) {
   bool required_command_failed = false;
   std::string output =
-      ::RunLLDBCommands(debugger, prefix, commands, required_command_failed);
+      ::RunLLDBCommands(debugger, prefix, commands, required_command_failed,
+                        /*parse_command_directives*/ true, echo_commands);
   SendOutput(OutputType::Console, output);
   return !required_command_failed;
 }
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index fc43d988f3a09..cb3431cc87fd1 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -290,7 +290,8 @@ struct DAP {
   ///   \b false if a fatal error was found while executing these commands,
   ///   according to the rules of \a LLDBUtils::RunLLDBCommands.
   bool RunLLDBCommands(llvm::StringRef prefix,
-                       llvm::ArrayRef<std::string> commands);
+                       llvm::ArrayRef<std::string> commands,
+                       bool echo_commands = true);
 
   llvm::Error RunAttachCommands(llvm::ArrayRef<std::string> attach_commands);
   llvm::Error RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands);
diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
index 8ed09fa2a931a..cd7b367d41d20 100644
--- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
@@ -164,7 +164,8 @@ void EvaluateRequestHandler::operator()(
       dap.focus_tid = frame.GetThread().GetThreadID();
     }
     auto result = RunLLDBCommandsVerbatim(dap.debugger, llvm::StringRef(),
-                                          {std::string(expression)});
+                                          {std::string(expression)},
+                                          /*echo_commands*/ false);
     EmplaceSafeString(body, "result", result);
     body.try_emplace("variablesReference", (int64_t)0);
   } else {
diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp
index a27beff0b030d..393cd7c43e34e 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.cpp
+++ b/lldb/tools/lldb-dap/LLDBUtils.cpp
@@ -17,12 +17,24 @@ namespace lldb_dap {
 
 bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
                      const llvm::ArrayRef<std::string> &commands,
-                     llvm::raw_ostream &strm, bool parse_command_directives) {
+                     llvm::raw_ostream &strm, bool parse_command_directives,
+                     bool echo_commands) {
   if (commands.empty())
     return true;
 
   bool did_print_prefix = false;
 
+  // Get the default prompt from settings.
+  std::string prompt_string = "(lldb) ";
+  if (const lldb::SBStructuredData prompt = debugger.GetSetting("prompt")) {
+    const size_t prompt_length = prompt.GetStringValue(nullptr, 0);
+
+    if (prompt_length != 0) {
+      prompt_string.resize(prompt_length + 1);
+      prompt.GetStringValue(prompt_string.data(), prompt_string.length());
+    }
+  }
+
   lldb::SBCommandInterpreter interp = debugger.GetCommandInterpreter();
   for (llvm::StringRef command : commands) {
     lldb::SBCommandReturnObject result;
@@ -60,7 +72,10 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
         strm << prefix << "\n";
         did_print_prefix = true;
       }
-      strm << "(lldb) " << command << "\n";
+
+      if (echo_commands)
+        strm << prompt_string.c_str() << command << '\n';
+
       auto output_len = result.GetOutputSize();
       if (output_len) {
         const char *output = result.GetOutput();
@@ -81,21 +96,23 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
 std::string RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
                             const llvm::ArrayRef<std::string> &commands,
                             bool &required_command_failed,
-                            bool parse_command_directives) {
+                            bool parse_command_directives, bool echo_commands) {
   required_command_failed = false;
   std::string s;
   llvm::raw_string_ostream strm(s);
-  required_command_failed = !RunLLDBCommands(debugger, prefix, commands, strm,
-                                             parse_command_directives);
+  required_command_failed =
+      !RunLLDBCommands(debugger, prefix, commands, strm,
+                       parse_command_directives, echo_commands);
   return s;
 }
 
-std::string
-RunLLDBCommandsVerbatim(lldb::SBDebugger &debugger, llvm::StringRef prefix,
-                        const llvm::ArrayRef<std::string> &commands) {
+std::string RunLLDBCommandsVerbatim(lldb::SBDebugger &debugger,
+                                    llvm::StringRef prefix,
+                                    const llvm::ArrayRef<std::string> &commands,
+                                    bool echo_commands) {
   bool required_command_failed = false;
   return RunLLDBCommands(debugger, prefix, commands, required_command_failed,
-                         /*parse_command_directives=*/false);
+                         /*parse_command_directives=*/false, echo_commands);
 }
 
 bool ThreadHasStopReason(lldb::SBThread &thread) {
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index 2c57847303cb3..d526ed431fff3 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -51,12 +51,16 @@ namespace lldb_dap {
 ///     If \b false, then command prefixes like \b ! or \b ? are not parsed and
 ///     each command is executed verbatim.
 ///
+/// \param[in] echo_commands
+///     If \b true, the command are echoed to the stream.
+///
 /// \return
 ///     \b true, unless a command prefixed with \b ! fails and parsing of
 ///     command directives is enabled.
 bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
                      const llvm::ArrayRef<std::string> &commands,
-                     llvm::raw_ostream &strm, bool parse_command_directives);
+                     llvm::raw_ostream &strm, bool parse_command_directives,
+                     bool echo_commands);
 
 /// Run a list of LLDB commands in the LLDB command interpreter.
 ///
@@ -81,18 +85,23 @@ bool RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
 ///     If \b false, then command prefixes like \b ! or \b ? are not parsed and
 ///     each command is executed verbatim.
 ///
+/// \param[in] echo_commands
+///     If \b true, the command are echoed to the stream.
+///
 /// \return
 ///     A std::string that contains the prefix and all commands and
 ///     command output.
 std::string RunLLDBCommands(lldb::SBDebugger &debugger, llvm::StringRef prefix,
                             const llvm::ArrayRef<std::string> &commands,
                             bool &required_command_failed,
-                            bool parse_command_directives = true);
+                            bool parse_command_directives = true,
+                            bool echo_commands = true);
 
 /// Similar to the method above, but without parsing command directives.
-std::string
-RunLLDBCommandsVerbatim(lldb::SBDebugger &debugger, llvm::StringRef prefix,
-                        const llvm::ArrayRef<std::string> &commands);
+std::string RunLLDBCommandsVerbatim(lldb::SBDebugger &debugger,
+                                    llvm::StringRef prefix,
+                                    const llvm::ArrayRef<std::string> &commands,
+                                    bool echo_commands);
 
 /// Check if a thread has a stop reason.
 ///

JDevlieghere

With this patch, is there anything that turns on echoing of commands? Do we think that's a useful feature to support and expose to users (my gut says: probably not). In that case, should we just drop the old code and avoid adding the boolean altogether?

lldb/tools/lldb-dap/DAP.h

lldb/tools/lldb-dap/LLDBUtils.cpp

Signed-off-by: Ebuka Ezike <[email protected]>

da-viper · 2025-04-15T12:10:28Z

With this patch, is there anything that turns on echoing of commands?

Yeah, the launch options

InitCommands, PreCommands and ***Commands.

walter-erquinigo

LGTM. Let's see if Jonas has more comments

lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp

lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py

…tion.py Co-authored-by: Walter Erquinigo <[email protected]>

Co-authored-by: Walter Erquinigo <[email protected]>

github-actions · 2025-04-16T19:44:17Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Signed-off-by: Ebuka Ezike <[email protected]>

JDevlieghere

LGTM modulo RunLLDBCommandsVerbatim.

lldb/tools/lldb-dap/LLDBUtils.h

Fixes llvm#131589 Add a new parameter to the RunCommands functions to control the echoing of commands --------- Signed-off-by: Ebuka Ezike <[email protected]> Co-authored-by: Walter Erquinigo <[email protected]>

[lldb][lldb-dap] fix repeating commands in repl mode

296019e

Fixes llvm#131589 Add a new option to the RunCommands* functions to control the echoing of commands

da-viper requested a review from JDevlieghere as a code owner April 9, 2025 13:42

llvmbot added lldb lldb-dap labels Apr 9, 2025

JDevlieghere reviewed Apr 10, 2025

View reviewed changes

lldb/tools/lldb-dap/DAP.h Outdated Show resolved Hide resolved

lldb/tools/lldb-dap/LLDBUtils.cpp Outdated Show resolved Hide resolved

da-viper added 2 commits April 11, 2025 10:11

[lldb][lldb-dap] update review requests

4f97db3

[lldb] set echo commands to false by default

300b722

Signed-off-by: Ebuka Ezike <[email protected]>

walter-erquinigo approved these changes Apr 16, 2025

View reviewed changes

lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp Outdated Show resolved Hide resolved

walter-erquinigo reviewed Apr 16, 2025

View reviewed changes

lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py Outdated Show resolved Hide resolved

da-viper and others added 2 commits April 16, 2025 20:41

Update lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detec…

304f55f

…tion.py Co-authored-by: Walter Erquinigo <[email protected]>

Update lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp

f63a2dd

Co-authored-by: Walter Erquinigo <[email protected]>

[lldb][lldb-dap] fix code formatting

7a467d5

Signed-off-by: Ebuka Ezike <[email protected]>

da-viper requested a review from JDevlieghere April 22, 2025 11:19

JDevlieghere approved these changes Apr 22, 2025

View reviewed changes

lldb/tools/lldb-dap/LLDBUtils.h Outdated Show resolved Hide resolved

[lldb][lldb-dap] Remove function that is only used in one place

cb9f8ab

da-viper merged commit fb01f19 into llvm:main Apr 25, 2025
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][lldb-dap] fix repeating commands in repl mode #135008

[lldb][lldb-dap] fix repeating commands in repl mode #135008

Uh oh!

da-viper commented Apr 9, 2025

Uh oh!

llvmbot commented Apr 9, 2025

Uh oh!

JDevlieghere left a comment

Uh oh!

Uh oh!

Uh oh!

da-viper commented Apr 15, 2025

Uh oh!

walter-erquinigo left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 16, 2025 •

edited

Loading

Uh oh!

JDevlieghere left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[lldb][lldb-dap] fix repeating commands in repl mode #135008

[lldb][lldb-dap] fix repeating commands in repl mode #135008

Uh oh!

Conversation

da-viper commented Apr 9, 2025

Uh oh!

llvmbot commented Apr 9, 2025

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

da-viper commented Apr 15, 2025

Uh oh!

walter-erquinigo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 16, 2025 •

edited

Loading