From 09cd46a63b1fb68e3c5c03273cbc90825b09d027 Mon Sep 17 00:00:00 2001 From: walter erquinigo Date: Mon, 16 Oct 2023 15:08:20 -0400 Subject: [PATCH] [lldb-vscode] Allow specifying a custom escape character for LLDB commands We've been using the backtick as our escape character, however that leads to a weird experience on VS Code, because on most hosts, as soon as you type the backtick on VS Code, the IDE will introduce another backtick. As changing the default escape character might be out of question because other plugins might rely on it, we can instead introduce an option to change this variable upon lldb-vscode initialization. FWIW, my users will be using : instead ot the backtick. --- .../test/tools/lldb-dap/dap_server.py | 9 +++- .../test/tools/lldb-dap/lldbdap_testcase.py | 4 ++ .../tools/lldb-dap/console/TestDAP_console.py | 44 +++++++++++++++++-- lldb/tools/lldb-dap/DAP.cpp | 11 +++-- lldb/tools/lldb-dap/DAP.h | 1 + lldb/tools/lldb-dap/JSONUtils.cpp | 10 +++-- lldb/tools/lldb-dap/JSONUtils.h | 11 +++-- lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++ lldb/tools/lldb-dap/package.json | 10 +++++ 9 files changed, 88 insertions(+), 16 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 8c645e0fdca72..d1fb478bc8bb9 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -731,6 +731,7 @@ def request_launch( postRunCommands=None, enableAutoVariableSummaries=False, enableSyntheticChildDebugging=False, + commandEscapePrefix="`", ): args_dict = {"program": program} if args: @@ -774,6 +775,7 @@ def request_launch( args_dict["postRunCommands"] = postRunCommands args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging + args_dict["commandEscapePrefix"] = commandEscapePrefix command_dict = {"command": "launch", "type": "request", "arguments": args_dict} response = self.send_recv(command_dict) @@ -1015,7 +1017,12 @@ def terminate(self): class DebugAdaptorServer(DebugCommunication): def __init__( - self, executable=None, port=None, init_commands=[], log_file=None, env=None + self, + executable=None, + port=None, + init_commands=[], + log_file=None, + env=None, ): self.process = None if executable is not None: diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 3094b66af4792..aa89ffe24c3e0 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -351,6 +351,7 @@ def launch( postRunCommands=None, enableAutoVariableSummaries=False, enableSyntheticChildDebugging=False, + commandEscapePrefix="`", ): """Sending launch request to dap""" @@ -389,6 +390,7 @@ def cleanup(): postRunCommands=postRunCommands, enableAutoVariableSummaries=enableAutoVariableSummaries, enableSyntheticChildDebugging=enableSyntheticChildDebugging, + commandEscapePrefix=commandEscapePrefix, ) if expectFailure: @@ -425,6 +427,7 @@ def build_and_launch( lldbDAPEnv=None, enableAutoVariableSummaries=False, enableSyntheticChildDebugging=False, + commandEscapePrefix="`", ): """Build the default Makefile target, create the DAP debug adaptor, and launch the process. @@ -455,4 +458,5 @@ def build_and_launch( postRunCommands=postRunCommands, enableAutoVariableSummaries=enableAutoVariableSummaries, enableSyntheticChildDebugging=enableSyntheticChildDebugging, + commandEscapePrefix=commandEscapePrefix, ) diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py index 47c706f2ca721..ffa0dc943e069 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -3,16 +3,18 @@ """ import dap_server +import lldbdap_testcase +from lldbsuite.test import lldbutil from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -from lldbsuite.test import lldbutil -import lldbdap_testcase class TestDAP_console(lldbdap_testcase.DAPTestCaseBase): - def check_lldb_command(self, lldb_command, contains_string, assert_msg): + def check_lldb_command( + self, lldb_command, contains_string, assert_msg, command_escape_prefix="`" + ): response = self.dap_server.request_evaluate( - "`%s" % (lldb_command), context="repl" + f"{command_escape_prefix}{lldb_command}", context="repl" ) output = response["body"]["result"] self.assertIn( @@ -68,3 +70,37 @@ def test_scopes_variables_setVariable_evaluate(self): # currently selected frame. self.check_lldb_command("frame select", "frame #1", "frame 1 is selected") + + @skipIfWindows + @skipIfRemote + def test_custom_escape_prefix(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program, commandEscapePrefix="::") + source = "main.cpp" + breakpoint1_line = line_number(source, "// breakpoint 1") + breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) + self.continue_to_breakpoints(breakpoint_ids) + + self.check_lldb_command( + "help", + "For more information on any command", + "Help can be invoked", + command_escape_prefix="::", + ) + + @skipIfWindows + @skipIfRemote + def test_empty_escape_prefix(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program, commandEscapePrefix="") + source = "main.cpp" + breakpoint1_line = line_number(source, "// breakpoint 1") + breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) + self.continue_to_breakpoints(breakpoint_ids) + + self.check_lldb_command( + "help", + "For more information on any command", + "Help can be invoked", + command_escape_prefix="", + ) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index af20e6460685e..6fa0514bfe32b 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -382,9 +382,10 @@ llvm::json::Value DAP::CreateTopLevelScopes() { ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame &frame, std::string &text) { - // Include ` as an escape hatch. - if (!text.empty() && text[0] == '`') { - text = text.substr(1); + // Include the escape hatch prefix. + if (!text.empty() && + llvm::StringRef(text).starts_with(g_dap.command_escape_prefix)) { + text = text.substr(g_dap.command_escape_prefix.size()); return ExpressionContext::Command; } @@ -416,7 +417,9 @@ ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame &frame, if (!auto_repl_mode_collision_warning) { llvm::errs() << "Variable expression '" << text << "' is hiding an lldb command, prefix an expression " - "with ` to ensure it runs as a lldb command.\n"; + "with '" + << g_dap.command_escape_prefix + << "' to ensure it runs as a lldb command.\n"; auto_repl_mode_collision_warning = true; } return ExpressionContext::Variable; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index e13d91a9df5d2..b00c103c33b7a 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -188,6 +188,7 @@ struct DAP { ReplModeRequestHandler repl_mode_request_handler; ReplMode repl_mode; bool auto_repl_mode_collision_warning; + std::string command_escape_prefix = "`"; DAP(); ~DAP(); diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 0daa8c11c1fa6..46528a2a28d4d 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -46,15 +46,17 @@ llvm::StringRef GetAsString(const llvm::json::Value &value) { } // Gets a string from a JSON object using the key, or returns an empty string. -llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key) { +llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key, + llvm::StringRef defaultValue) { if (std::optional value = obj.getString(key)) return *value; - return llvm::StringRef(); + return defaultValue; } -llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key) { +llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key, + llvm::StringRef defaultValue) { if (obj == nullptr) - return llvm::StringRef(); + return defaultValue; return GetString(*obj, key); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index ade8ed6822691..d829e650b8f31 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -52,12 +52,17 @@ llvm::StringRef GetAsString(const llvm::json::Value &value); /// \param[in] key /// The key to use when extracting the value /// +/// \param[in] defaultValue +/// The default value to return if the key is not present +/// /// \return /// A llvm::StringRef that contains the string value for the -/// specified \a key, or an empty string if there is no key that +/// specified \a key, or the default value if there is no key that /// matches or if the value is not a string. -llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key); -llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key); +llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key, + llvm::StringRef defaultValue = {}); +llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key, + llvm::StringRef defaultValue = {}); /// Extract the unsigned integer value for the specified key from /// the specified object. diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index a218e85664b59..b5d06a977a449 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -651,6 +651,8 @@ void request_attach(const llvm::json::Object &request) { GetBoolean(arguments, "enableAutoVariableSummaries", false); g_dap.enable_synthetic_child_debugging = GetBoolean(arguments, "enableSyntheticChildDebugging", false); + g_dap.command_escape_prefix = + GetString(arguments, "commandEscapePrefix", "`"); // This is a hack for loading DWARF in .o files on Mac where the .o files // in the debug map of the main executable have relative paths which require @@ -1801,6 +1803,8 @@ void request_launch(const llvm::json::Object &request) { GetBoolean(arguments, "enableAutoVariableSummaries", false); g_dap.enable_synthetic_child_debugging = GetBoolean(arguments, "enableSyntheticChildDebugging", false); + g_dap.command_escape_prefix = + GetString(arguments, "commandEscapePrefix", "`"); // This is a hack for loading DWARF in .o files on Mac where the .o files // in the debug map of the main executable have relative paths which require diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 79954cde2bbc8..a0ae7ac834939 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -250,6 +250,11 @@ "type": "boolean", "description": "If a variable is displayed using a synthetic children, also display the actual contents of the variable at the end under a [raw] entry. This is useful when creating sythetic child plug-ins as it lets you see the actual contents of the variable.", "default": false + }, + "commandEscapePrefix": { + "type": "string", + "description": "The escape prefix to use for executing regular LLDB commands in the Debug Console, instead of printing variables. Defaults to a back-tick (`). If it's an empty string, then all expression in the Debug Console are treated as regular LLDB commands.", + "default": "`" } } }, @@ -339,6 +344,11 @@ "type": "boolean", "description": "If a variable is displayed using a synthetic children, also display the actual contents of the variable at the end under a [raw] entry. This is useful when creating sythetic child plug-ins as it lets you see the actual contents of the variable.", "default": false + }, + "commandEscapePrefix": { + "type": "string", + "description": "The escape prefix character to use for executing regular LLDB commands in the Debug Console, instead of printing variables. Defaults to a back-tick (`). If empty, then all expression in the Debug Console are treated as regular LLDB commands.", + "default": "`" } } }