Add a setting to force overwriting commands in "command script add"

[jingham]

Summary

When I added containers it seemed like it would be easier to accidentally overwrite extant commands, so I required a --overwrite option before "command script add" will overwrite an extant command. That still strikes me as a good idea for users of packages of commands, but when you are developing the commands it's annoying since it means you have to change the "command script add" commands in your package while iteratively adding them during development. So this patch adds a setting to turn off the overwrite requirement. That way you can set this setting while developing your package, then turn it off again when you're acting as a consumer of packages.

[JDevlieghere]

LGTM

[labath]

lldb/source/Commands/CommandObjectCommands.cpp

1650

This doesn't seem right. eLazyBoolCalculate is effectively true, but I don't know if that's what you meant.

lldb/include/lldb/Interpreter/CommandInterpreter.h

public:
 
   bool GetEchoCommands() const;
   void SetEchoCommands(bool enable);
 
   bool GetEchoCommentCommands() const;
   void SetEchoCommentCommands(bool enable);
 
   bool GetRepeatPreviousCommand() const;
   
+  bool GetRequireCommandOverwrite() const;
+
   const CommandObject::CommandMap &GetUserCommands() const {
     return m_user_dict;
   }
 
   const CommandObject::CommandMap &GetUserMultiwordCommands() const {
     return m_user_mw_dict;
   }
 

lldb/source/Commands/CommandObjectCommands.cpp

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         if (!option_arg.empty())
           m_class_name = std::string(option_arg);
         break;
       case 'h':
         if (!option_arg.empty())
           m_short_help = std::string(option_arg);
         break;
       case 'o':
-        m_overwrite = true;
+        m_overwrite_lazy = eLazyBoolYes;
         break;
       case 's':
         m_synchronicity =
             (ScriptedCommandSynchronicity)OptionArgParser::ToOptionEnum(
                 option_arg, GetDefinitions()[option_idx].enum_values, 0, error);
         if (!error.Success())
           error.SetErrorStringWithFormat(
               "unrecognized value for synchronicity '%s'",
               option_arg.str().c_str());
         break;
       default:
         llvm_unreachable("Unimplemented option");
       }
 
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_class_name.clear();
       m_funct_name.clear();
       m_short_help.clear();
-      m_overwrite = false;
+      m_overwrite_lazy = eLazyBoolCalculate;
       m_synchronicity = eScriptedCommandSynchronicitySynchronous;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_script_add_options);
     }
 
     // Instance variables to hold the values for command options.
 
     std::string m_class_name;
     std::string m_funct_name;
     std::string m_short_help;
-    bool m_overwrite = false;
+    LazyBool m_overwrite_lazy;
     ScriptedCommandSynchronicity m_synchronicity =
         eScriptedCommandSynchronicitySynchronous;
   };
 
   void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFileSP());
     if (output_sp && interactive) {
       output_sp->PutCString(g_python_command_instructions);
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override {
     StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
 
     ScriptInterpreter *interpreter = GetDebugger().GetScriptInterpreter();
     if (interpreter) {
-
       StringList lines;
       lines.SplitIntoLines(data);
       if (lines.GetSize() > 0) {
         std::string funct_name_str;
         if (interpreter->GenerateScriptAliasFunction(lines, funct_name_str)) {
           if (funct_name_str.empty()) {
             error_sp->Printf("error: unable to obtain a function name, didn't "
                              "add python command.\n");
if (GetDebugger().GetScriptLanguage() != lldb::eScriptLanguagePython) {
                          "commands is currently Python");
       return false;
     }
 
     if (command.GetArgumentCount() == 0) {
       result.AppendError("'command script add' requires at least one argument");
       return false;
     }
-    // Store the options in case we get multi-line input
-    m_overwrite = m_options.m_overwrite;
+    // Store the options in case we get multi-line input, also figure out the
+    // default if not user supplied:
+    switch (m_options.m_overwrite_lazy) {
+      case eLazyBoolCalculate:
+        m_overwrite = !GetDebugger().GetCommandInterpreter().GetRequireCommandOverwrite();
+        break;
+      case eLazyBoolYes:
+        m_overwrite = true;
+        break;
+      case eLazyBoolNo:
+        m_overwrite = false;
+    }
+    
     Status path_error;
     m_container = GetCommandInterpreter().VerifyUserMultiwordCmdPath(
         command, true, path_error);
 
     if (path_error.Fail()) {
       result.AppendErrorWithFormat("error in command path: %s",
                                    path_error.AsCString());
       return false;
bool DoExecute(Args &command, CommandReturnObject &result) override {
     }
     return result.Succeeded();
   }
 
   CommandOptions m_options;
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = false;
+  bool m_overwrite = eLazyBoolCalculate;
   ScriptedCommandSynchronicity m_synchronicity =
       eScriptedCommandSynchronicitySynchronous;
 };
 
 // CommandObjectCommandsScriptList
 
 class CommandObjectCommandsScriptList : public CommandObjectParsed {
 public:

lldb/source/Interpreter/CommandInterpreter.cpp

 }
 
 bool CommandInterpreter::GetRepeatPreviousCommand() const {
   const uint32_t idx = ePropertyRepeatPreviousCommand;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
 }
 
+bool CommandInterpreter::GetRequireCommandOverwrite() const {
+  const uint32_t idx = ePropertyRequireCommandOverwrite;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
+}
+
 void CommandInterpreter::Initialize() {
   LLDB_SCOPED_TIMER();
 
   CommandReturnObject result(m_debugger.GetUseColor());
 
   LoadCommandDictionary();
 
   // An alias arguments vector to reuse - reset it before use...

lldb/source/Interpreter/InterpreterProperties.td

let Definition = "interpreter" in {
   def EchoCommentCommands: Property<"echo-comment-commands", "Boolean">,
     Global,
     DefaultTrue,
     Desc<"If true, commands will be echoed even if they are pure comment lines.">;
   def RepeatPreviousCommand: Property<"repeat-previous-command", "Boolean">,
     Global,
     DefaultTrue,
     Desc<"If true, LLDB will repeat the previous command if no command was passed to the interpreter. If false, LLDB won't repeat the previous command but only return a new prompt.">;
+  def RequireCommandOverwrite: Property<"require-overwrite", "Boolean">,
+    Global,
+    DefaultTrue,
+    Desc<"If true, require --overwrite in 'command script add' before overwriting existing user commands.">;
 }

lldb/test/API/commands/command/container/TestContainerCommands.py

def container_add(self):
         self.expect("help test-multi test-multi-sub", "Listing subcommands works",
                     substrs=["A test container sub-command", "welcome -- Just a docstring for Welcome"])
         self.expect("help test-multi test-multi-sub welcome", "Subcommand help works",
                     substrs=["Just a docstring for Welcome"])
         # And make sure it actually works:
         self.expect("test-multi test-multi-sub welcome friend", "Test command works",
                     substrs=["Hello friend, welcome to LLDB"])
 
-        # Make sure overwriting works, first the leaf command:
-        # We should not be able to remove extant commands by default:
+        # Make sure overwriting works on the leaf command.  First using the
+        # explicit option so we should not be able to remove extant commands by default:
+
         self.expect("command script add -c welcome.WelcomeCommand2 test-multi test-multi-sub welcome",
                     "overwrite command w/o -o",
                     substrs=["cannot add command: sub-command already exists"], error=True)
         # But we can with the -o option:
         self.runCmd("command script add -c welcome.WelcomeCommand2 -o test-multi test-multi-sub welcome")
         # Make sure we really did overwrite:
         self.expect("test-multi test-multi-sub welcome friend", "Used the new command class",
                     substrs=["Hello friend, welcome again to LLDB"])
-
         self.expect("apropos welcome", "welcome should show up in apropos", substrs=["A docstring for the second Welcome"])
         
+        # Now switch the default and make sure we can now delete w/o the overwrite option:
+        self.runCmd("settings set interpreter.require-overwrite 0")
+        self.runCmd("command script add -c welcome.WelcomeCommand test-multi test-multi-sub welcome")
+        # Make sure we really did overwrite:
+        self.expect("test-multi test-multi-sub welcome friend", "Used the new command class",
+                    substrs=["Hello friend, welcome to LLDB"])
+        
         # Make sure we give good errors when the input is wrong:
         self.expect("command script delete test-mult test-multi-sub welcome", "Delete script command - wrong first path component",
                     substrs=["'test-mult' not found"], error=True)
         
         self.expect("command script delete test-multi test-multi-su welcome", "Delete script command - wrong second path component",
                     substrs=["'test-multi-su' not found"], error=True)
         self.check_command_tree_exists()