-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb] Add actionable feedback when overwriting a command fails #76030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Add actionable feedback when overwriting a command fails #76030
Conversation
If adding a user commands fails because a command with the same name already exists, we only say that "force replace is not set" without telling the user _how_ to set it. There are two ways to do so; this commit changes the error message to mention both.
@llvm/pr-subscribers-lldb Author: Felipe de Azevedo Piovezan (felipepiovezan) ChangesIf adding a user commands fails because a command with the same name already exists, we only say that "force replace is not set" without telling the user how to set it. There are two ways to do so; this commit changes the error message to mention both. Full diff: https://github.com/llvm/llvm-project/pull/76030.diff 2 Files Affected:
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index e1275ce711fc17..3d5ecd0612a640 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1160,7 +1160,9 @@ Status CommandInterpreter::AddUserCommand(llvm::StringRef name,
if (UserCommandExists(name)) {
if (!can_replace) {
- result.SetErrorString("user command exists and force replace not set");
+ result.SetErrorString(
+ "user command exists and force replace not set by --overwrite or "
+ "'settings set interpreter.require-overwrite false'");
return result;
}
if (cmd_sp->IsMultiwordObject()) {
diff --git a/lldb/test/API/commands/command/script/TestCommandScript.py b/lldb/test/API/commands/command/script/TestCommandScript.py
index cac11834fa7364..1e1d9b728a965d 100644
--- a/lldb/test/API/commands/command/script/TestCommandScript.py
+++ b/lldb/test/API/commands/command/script/TestCommandScript.py
@@ -161,6 +161,17 @@ def cleanup():
)
self.expect("my_command", substrs=["a.out"])
+ # Test that without --overwrite we are not allowed to redefine the command.
+ self.expect(
+ "command script add my_command --class welcome.TargetnameCommand",
+ substrs=[
+ "cannot add command: user command exists and force replace not set",
+ "--overwrite",
+ "settings set interpreter.require-overwrite false",
+ ],
+ error=True,
+ )
+
self.runCmd("command script clear")
self.expect(
|
Address review comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…nable-feedback [cherry-pick][lldb] Add actionable feedback when overwriting a command fails (llvm#76030)
If adding a user commands fails because a command with the same name already exists, we only say that "force replace is not set" without telling the user how to set it. There are two ways to do so; this commit changes the error message to mention both.