================ @@ -0,0 +1,118 @@ +//===-- GoToRequestHandler.cpp --------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAP.h" +#include "EventHelper.h" +#include "JSONUtils.h" + +namespace lldb_dap { + +/// Creates an \p StoppedEvent with the reason \a goto +static void SendThreadGotoEvent(DAP &dap, lldb::tid_t thread_id) { + llvm::json::Object event(CreateEventObject("stopped")); + llvm::json::Object body; + body.try_emplace("reason", "goto"); + body.try_emplace("description", "Paused on Jump To Cursor"); + body.try_emplace("threadId", thread_id); + body.try_emplace("preserveFocusHint", false); + body.try_emplace("allThreadsStopped", true); + + event.try_emplace("body", std::move(body)); + dap.SendJSON(llvm::json::Value(std::move(event))); +} + +// "GotoRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "The request sets the location where the debuggee will +// continue to run.\nThis makes it possible to skip the execution of code or +// to execute code again.\nThe code between the current location and the +// goto target is not executed but skipped.\nThe debug adapter first sends +// the response and then a `stopped` event with reason `goto`.\nClients +// should only call this request if the corresponding capability +// `supportsGotoTargetsRequest` is true (because only then goto targets +// exist that can be passed as arguments).", +//. "properties": { +// "command": { +// "type": "string", +// "enum": [ "goto" ] +// }, +// "arguments": { +// "$ref": "#/definitions/GotoArguments" +// } +// }, +// "required": [ "command", "arguments" ] +// }] +// } +// "GotoArguments": { +// "type": "object", +// "description": "Arguments for `goto` request.", +// "properties": { +// "threadId": { +// "type": "integer", +// "description": "Set the goto target for this thread." +// }, +// "targetId": { +// "type": "integer", +// "description": "The location where the debuggee will continue to run." +// } +// }, +// "required": [ "threadId", "targetId" ] +// } +// "GotoResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to `goto` request. This is just an +// acknowledgement, so no body field is required." +// }] +// } +void GoToRequestHandler::operator()(const llvm::json::Object &request) const { + llvm::json::Object response; + FillResponse(request, response); + + auto SendError = [&](auto &&message) { + response["success"] = false; + response["message"] = message; + dap.SendJSON(llvm::json::Value(std::move(response))); + }; ---------------- vogelsgesang wrote:
IMO, we should not factor out this helper, because: 1. We already have a better abstraction. 2. This helper is encouraging to violate the debug adapter protocol. **Better abstraction**: In #129155, @ashgti already introduced the `protocol::Response` class, which also provides the `message` header. In #130090, those messages got integrated with our request handlers. IMO, we should change this PR to use the new mechanism introduced in #130090, instead of using the legacy mechanism. **Violation of the debug adapter protocol specification**: This helper sets the message to an arbitrary string, such as `"Arguments is empty"`. However, this is not the correct usage of DAP. The `message` field of the [Response](https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_Response) should be a machine-readable enum (`cancelled`, `notStopped`, ...). It should not contain the human-readable the error message and will not be displayed to users. Instead, the user-readable message should use the [ErrorResponse](https://microsoft.github.io/debug-adapter-protocol/specification#Base_Protocol_ErrorResponse), i.e. the user-readable message should be in `body. format`, and `body.showUser` should probably be set to true. This is currently done incorrectly by lldb-dap everywhere. This is also, why users currently get no feedback if, e.g., setting a variable failed. The new mechanism introduced in #130090 still does not handle this correctly. But if we consistently adopt the new mechanism, we at least only need to fix a single place to become standard-compliant (That being said, I would not block this PR on using `RequestHandler` instead of `LegacyRequestHandler,` in case @da-viper does not want to invest the time to update his code to the newer `RequestHandler` mechanism) https://github.com/llvm/llvm-project/pull/130503 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
