[Lldb-commits] [lldb] [lldb-dap] Migrating DAP 'initialize' to new typed RequestHandler. (PR #133007)

[Jonas Devlieghere via lldb-commits]

================
@@ -229,136 +230,45 @@ static void EventThreadFunction(DAP &dap) {
   }
 }
 
-// "InitializeRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Initialize request; value of command field is
-//                     'initialize'.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "initialize" ]
-//       },
-//       "arguments": {
-//         "$ref": "#/definitions/InitializeRequestArguments"
-//       }
-//     },
-//     "required": [ "command", "arguments" ]
-//   }]
-// },
-// "InitializeRequestArguments": {
-//   "type": "object",
-//   "description": "Arguments for 'initialize' request.",
-//   "properties": {
-//     "clientID": {
-//       "type": "string",
-//       "description": "The ID of the (frontend) client using this adapter."
-//     },
-//     "adapterID": {
-//       "type": "string",
-//       "description": "The ID of the debug adapter."
-//     },
-//     "locale": {
-//       "type": "string",
-//       "description": "The ISO-639 locale of the (frontend) client using
-//                       this adapter, e.g. en-US or de-CH."
-//     },
-//     "linesStartAt1": {
-//       "type": "boolean",
-//       "description": "If true all line numbers are 1-based (default)."
-//     },
-//     "columnsStartAt1": {
-//       "type": "boolean",
-//       "description": "If true all column numbers are 1-based (default)."
-//     },
-//     "pathFormat": {
-//       "type": "string",
-//       "_enum": [ "path", "uri" ],
-//       "description": "Determines in what format paths are specified. The
-//                       default is 'path', which is the native format."
-//     },
-//     "supportsVariableType": {
-//       "type": "boolean",
-//       "description": "Client supports the optional type attribute for
-//                       variables."
-//     },
-//     "supportsVariablePaging": {
-//       "type": "boolean",
-//       "description": "Client supports the paging of variables."
-//     },
-//     "supportsRunInTerminalRequest": {
-//       "type": "boolean",
-//       "description": "Client supports the runInTerminal request."
-//     }
-//   },
-//   "required": [ "adapterID" ]
-// },
-// "InitializeResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'initialize' request.",
-//     "properties": {
-//       "body": {
-//         "$ref": "#/definitions/Capabilities",
-//         "description": "The capabilities of this debug adapter."
-//       }
-//     }
-//   }]
-// }
-void InitializeRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-  llvm::json::Object body;
-
-  const auto *arguments = request.getObject("arguments");
-  // sourceInitFile option is not from formal DAP specification. It is only
-  // used by unit tests to prevent sourcing .lldbinit files from environment
-  // which may affect the outcome of tests.
-  bool source_init_file =
-      GetBoolean(arguments, "sourceInitFile").value_or(true);
+/// Initialize request; value of command field is 'initialize'.
+llvm::Expected<protocol::InitializeResponseBody> InitializeRequestHandler::Run(
+    const protocol::InitializeRequestArguments &arguments) const {
+  dap.clientFeatures = arguments.supportedFeatures;
 
   // Do not source init files until in/out/err are configured.
   dap.debugger = lldb::SBDebugger::Create(false);
   dap.debugger.SetInputFile(dap.in);
-  auto out_fd = dap.out.GetWriteFileDescriptor();
-  if (llvm::Error err = out_fd.takeError()) {
-    response["success"] = false;
-    EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
+
+  llvm::Expected<int> out_fd = dap.out.GetWriteFileDescriptor();
+  if (!out_fd)
+    return out_fd.takeError();
   dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false));
-  auto err_fd = dap.err.GetWriteFileDescriptor();
-  if (llvm::Error err = err_fd.takeError()) {
-    response["success"] = false;
-    EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
-    dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
+
+  llvm::Expected<int> err_fd = dap.err.GetWriteFileDescriptor();
+  if (!err_fd)
+    return err_fd.takeError();
   dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false));
 
   auto interp = dap.debugger.GetCommandInterpreter();
 
-  if (source_init_file) {
+  // sourceInitFile option is not from formal DAP specification. It is only
+  // used by unit tests to prevent sourcing .lldbinit files from environment
+  // which may affect the outcome of tests.
----------------
JDevlieghere wrote:

I know you're just moving this, but since you're here:
```suggestion
  // The sourceInitFile option is not part of the DAP specification. It is an extension
  // used by the test suite to prevent sourcing `.lldbinit` and changing its behavior .
```

https://github.com/llvm/llvm-project/pull/133007