[lldb-dap] Add external terminal support (#146950)

This patch deprecates the `runInTerminal` option in favour of `console`
which allow the user the specify an integrated or external option.

Author: DrSergei

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -908,7 +908,7 @@ def request_launch(
         disableASLR=False,
         disableSTDIO=False,
         shellExpandArguments=False,
-        runInTerminal=False,
+        console: Optional[str] = None,
         enableAutoVariableSummaries=False,
         displayExtendedBacktrace=False,
         enableSyntheticChildDebugging=False,
@@ -958,8 +958,8 @@ def request_launch(
             args_dict["launchCommands"] = launchCommands
         if sourceMap:
             args_dict["sourceMap"] = sourceMap
-        if runInTerminal:
-            args_dict["runInTerminal"] = runInTerminal
+        if console:
+            args_dict["console"] = console
         if postRunCommands:
             args_dict["postRunCommands"] = postRunCommands
         if customFrameFormat:

lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

@@ -44,22 +44,39 @@ def test_failing_launch_program(self):
             "'{0}' does not exist".format(program), response["body"]["error"]["format"]
         )
 
-    def test_failing_launch_commands_and_run_in_terminal(self):
+    def test_failing_launch_commands_and_console(self):
         """
-        Tests launching with an invalid program.
+        Tests launching with launch commands in an integrated terminal.
         """
         program = self.getBuildArtifact("a.out")
         self.create_debug_adapter()
         response = self.launch(
-            program, launchCommands=["a b c"], runInTerminal=True, expectFailure=True
+            program,
+            launchCommands=["a b c"],
+            console="integratedTerminal",
+            expectFailure=True,
         )
         self.assertFalse(response["success"])
         self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"]))
         self.assertEqual(
-            "'launchCommands' and 'runInTerminal' are mutually exclusive",
+            "'launchCommands' and non-internal 'console' are mutually exclusive",
             self.get_dict_value(response, ["body", "error", "format"]),
         )
 
+    def test_failing_console(self):
+        """
+        Tests launching in console with an invalid terminal type.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.create_debug_adapter()
+        response = self.launch(program, console="invalid", expectFailure=True)
+        self.assertFalse(response["success"])
+        self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"]))
+        self.assertRegex(
+            response["body"]["error"]["format"],
+            r"unexpected value, expected 'internalConsole\', 'integratedTerminal\' or 'externalTerminal\' at arguments.console",
+        )
+
     @skipIfWindows
     def test_termination(self):
         """

lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py renamed to lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py

@@ -10,7 +10,7 @@
 
 
 @skipIfBuildType(["debug"])
-class TestDAP_restart_runInTerminal(lldbdap_testcase.DAPTestCaseBase):
+class TestDAP_restart_console(lldbdap_testcase.DAPTestCaseBase):
     def verify_stopped_on_entry(self, stopped_events: List[Dict[str, Any]]):
         seen_stopped_event = 0
         for stopped_event in stopped_events:
@@ -44,7 +44,7 @@ def test_basic_functionality(self):
         line_B = line_number("main.c", "// breakpoint B")
 
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, runInTerminal=True)
+        self.build_and_launch(program, console="integratedTerminal")
         [bp_A, bp_B] = self.set_source_breakpoints("main.c", [line_A, line_B])
 
         # Verify we hit A, then B.
@@ -86,10 +86,10 @@ def test_basic_functionality(self):
     @skipIf(oslist=["linux"], archs=["arm$"])  # Always times out on buildbot
     def test_stopOnEntry(self):
         """
-        Check that stopOnEntry works correctly when using runInTerminal.
+        Check that stopOnEntry works correctly when using console.
         """
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, runInTerminal=True, stopOnEntry=True)
+        self.build_and_launch(program, console="integratedTerminal", stopOnEntry=True)
         [bp_main] = self.set_function_breakpoints(["main"])
 
         self.dap_server.request_continue()  # sends configuration done

lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py

@@ -37,7 +37,7 @@ def test_runInTerminal(self):
         program = self.getBuildArtifact("a.out")
         source = "main.c"
         self.build_and_launch(
-            program, runInTerminal=True, args=["foobar"], env=["FOO=bar"]
+            program, console="integratedTerminal", args=["foobar"], env=["FOO=bar"]
         )
 
         self.assertEqual(
@@ -83,7 +83,7 @@ def test_runInTerminalWithObjectEnv(self):
             launch the inferior with the correct environment variables using an object.
         """
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, runInTerminal=True, env={"FOO": "BAR"})
+        self.build_and_launch(program, console="integratedTerminal", env={"FOO": "BAR"})
 
         self.assertEqual(
             len(self.dap_server.reverse_requests),
@@ -105,7 +105,7 @@ def test_runInTerminalInvalidTarget(self):
         self.build_and_create_debug_adapter()
         response = self.launch(
             "INVALIDPROGRAM",
-            runInTerminal=True,
+            console="integratedTerminal",
             args=["foobar"],
             env=["FOO=bar"],
             expectFailure=True,

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

@@ -23,9 +23,10 @@ namespace lldb_dap {
 /// Launch request; value of command field is 'launch'.
 Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
   // Validate that we have a well formed launch request.
-  if (!arguments.launchCommands.empty() && arguments.runInTerminal)
+  if (!arguments.launchCommands.empty() &&
+      arguments.console != protocol::eConsoleInternal)
     return make_error<DAPError>(
-        "'launchCommands' and 'runInTerminal' are mutually exclusive");
+        "'launchCommands' and non-internal 'console' are mutually exclusive");
 
   dap.SetConfiguration(arguments.configuration, /*is_attach=*/false);
   dap.last_launch_request = arguments;

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

@@ -80,7 +80,8 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
 
   llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
       arguments.configuration.program, arguments.args, arguments.env,
-      arguments.cwd, comm_file.m_path, debugger_pid);
+      arguments.cwd, comm_file.m_path, debugger_pid,
+      arguments.console == protocol::eConsoleExternalTerminal);
   dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
                                                     std::move(reverse_request));
 
@@ -192,7 +193,7 @@ llvm::Error BaseRequestHandler::LaunchProcess(
     // about process state changes during the launch.
     ScopeSyncMode scope_sync_mode(dap.debugger);
 
-    if (arguments.runInTerminal) {
+    if (arguments.console != protocol::eConsoleInternal) {
       if (llvm::Error err = RunInTerminal(dap, arguments))
         return err;
     } else if (launchCommands.empty()) {

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -854,12 +854,16 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) {
 llvm::json::Object CreateRunInTerminalReverseRequest(
     llvm::StringRef program, const std::vector<std::string> &args,
     const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
-    llvm::StringRef comm_file, lldb::pid_t debugger_pid) {
+    llvm::StringRef comm_file, lldb::pid_t debugger_pid, bool external) {
   llvm::json::Object run_in_terminal_args;
-  // This indicates the IDE to open an embedded terminal, instead of opening
-  // the terminal in a new window.
-  run_in_terminal_args.try_emplace("kind", "integrated");
-
+  if (external) {
+    // This indicates the IDE to open an external terminal window.
+    run_in_terminal_args.try_emplace("kind", "external");
+  } else {
+    // This indicates the IDE to open an embedded terminal, instead of opening
+    // the terminal in a new window.
+    run_in_terminal_args.try_emplace("kind", "integrated");
+  }
   // The program path must be the first entry in the "args" field
   std::vector<std::string> req_args = {DAP::debug_adapter_path.str(),
                                        "--comm-file", comm_file.str()};

lldb/tools/lldb-dap/JSONUtils.h

@@ -388,13 +388,17 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
 ///     launcher uses it on Linux tell the kernel that it should allow the
 ///     debugger process to attach.
 ///
+/// \param[in] external
+///     If set to true, the program will run in an external terminal window
+///     instead of IDE's integrated terminal.
+///
 /// \return
 ///     A "runInTerminal" JSON object that follows the specification outlined by
 ///     Microsoft.
 llvm::json::Object CreateRunInTerminalReverseRequest(
     llvm::StringRef program, const std::vector<std::string> &args,
     const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
-    llvm::StringRef comm_file, lldb::pid_t debugger_pid);
+    llvm::StringRef comm_file, lldb::pid_t debugger_pid, bool external);
 
 /// Create a "Terminated" JSON object that contains statistics
 ///

lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

@@ -262,6 +262,34 @@ json::Value toJSON(const BreakpointLocationsResponseBody &BLRB) {
   return json::Object{{"breakpoints", BLRB.breakpoints}};
 }
 
+bool fromJSON(const json::Value &Params, Console &C, json::Path P) {
+  auto oldFormatConsole = Params.getAsBoolean();
+  if (oldFormatConsole) {
+    C = *oldFormatConsole ? eConsoleIntegratedTerminal : eConsoleInternal;
+    return true;
+  }
+  auto newFormatConsole = Params.getAsString();
+  if (!newFormatConsole) {
+    P.report("expected a string");
+    return false;
+  }
+
+  std::optional<Console> console =
+      StringSwitch<std::optional<Console>>(*newFormatConsole)
+          .Case("internalConsole", eConsoleInternal)
+          .Case("integratedTerminal", eConsoleIntegratedTerminal)
+          .Case("externalTerminal", eConsoleExternalTerminal)
+          .Default(std::nullopt);
+  if (!console) {
+    P.report("unexpected value, expected 'internalConsole', "
+             "'integratedTerminal' or 'externalTerminal'");
+    return false;
+  }
+
+  C = *console;
+  return true;
+}
+
 bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
               json::Path P) {
   json::ObjectMapper O(Params, P);
@@ -273,9 +301,8 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
          O.mapOptional("disableASLR", LRA.disableASLR) &&
          O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
          O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
-
-         O.mapOptional("runInTerminal", LRA.runInTerminal) &&
-         parseEnv(Params, LRA.env, P);
+         O.mapOptional("runInTerminal", LRA.console) &&
+         O.mapOptional("console", LRA.console) && parseEnv(Params, LRA.env, P);
 }
 
 bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

@@ -242,6 +242,12 @@ struct Configuration {
   std::string platformName;
 };
 
+enum Console : unsigned {
+  eConsoleInternal,
+  eConsoleIntegratedTerminal,
+  eConsoleExternalTerminal
+};
+
 /// lldb-dap specific launch arguments.
 struct LaunchRequestArguments {
   /// Common lldb-dap configuration values for launching/attaching operations.
@@ -290,9 +296,9 @@ struct LaunchRequestArguments {
   /// Set whether to shell expand arguments to the process when launching.
   bool shellExpandArguments = false;
 
-  /// Launch the program inside an integrated terminal in the IDE. Useful for
-  /// debugging interactive command line programs.
-  bool runInTerminal = false;
+  /// Specify where to launch the program: internal console, integrated
+  /// terminal or external terminal.
+  Console console = eConsoleInternal;
 
   /// @}
 };