Fixed an issue where you couldn't delete a user defined regex, python, or multi-word...
authorGreg Clayton <gclayton@apple.com>
Fri, 9 Jan 2015 19:08:20 +0000 (19:08 +0000)
committerGreg Clayton <gclayton@apple.com>
Fri, 9 Jan 2015 19:08:20 +0000 (19:08 +0000)
This new command will delete user defined regular commands, but not aliases. We still have "command unalias" to remove aliases as they are currently in different buckets. Appropriate error messages are displayed to inform the user when "command unalias" is used on removable user defined commands that points users to the "command delete" command.

Added a test to verify we can remove user defined commands and also verify that "command unalias" fails when used on a user defined command.
<rdar://problem/18248300>

llvm-svn: 225535

lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/include/lldb/Interpreter/CommandObject.h
lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandObjectRegexCommand.cpp
lldb/test/functionalities/command_regex/TestCommandRegex.py

index 0c8edeb..baaa271 100644 (file)
@@ -282,6 +282,10 @@ public:
     AddAlias (const char *alias_name, 
               lldb::CommandObjectSP& command_obj_sp);
 
+    // Remove a command if it is removable (python or regex command)
+    bool
+    RemoveCommand (const char *cmd);
+
     bool
     RemoveAlias (const char *alias_name);
     
index 41c17ef..bace326 100644 (file)
@@ -610,7 +610,7 @@ public:
     
     virtual bool
     Execute (const char *args_string, CommandReturnObject &result);
-    
+
 protected:    
     virtual bool
     DoExecute (const char *command, CommandReturnObject &result) = 0;
index 8855680..44bdf98 100644 (file)
@@ -34,12 +34,16 @@ public:
                                const char *help, 
                                const char *syntax, 
                                uint32_t max_matches,
-                               uint32_t completion_type_mask = 0);
+                               uint32_t completion_type_mask,
+                               bool is_removable);
     
     virtual
     ~CommandObjectRegexCommand ();
 
     bool
+    IsRemovable () const override { return m_is_removable; }
+
+    bool
     AddRegexCommand (const char *re_cstr, const char *command_cstr);
 
     bool
@@ -71,6 +75,7 @@ protected:
     const uint32_t m_max_matches;
     const uint32_t m_completion_type_mask;
     EntryCollection m_entries;
+    bool m_is_removable;
 
 private:
     DISALLOW_COPY_AND_ASSIGN (CommandObjectRegexCommand);
index c0503ae..f98eac0 100644 (file)
@@ -828,8 +828,16 @@ protected:
             {
                 if (m_interpreter.CommandExists (command_name))
                 {
-                    result.AppendErrorWithFormat ("'%s' is a permanent debugger command and cannot be removed.\n",
-                                                  command_name);
+                    if (cmd_obj->IsRemovable())
+                    {
+                        result.AppendErrorWithFormat ("'%s' is not an alias, it is a debugger command which can be removed using the 'command delete' command.\n",
+                                                      command_name);
+                    }
+                    else
+                    {
+                        result.AppendErrorWithFormat ("'%s' is a permanent debugger command and cannot be removed.\n",
+                                                      command_name);
+                    }
                     result.SetStatus (eReturnStatusFailed);
                 }
                 else
@@ -866,6 +874,77 @@ protected:
     }
 };
 
+#pragma mark CommandObjectCommandsDelete
+//-------------------------------------------------------------------------
+// CommandObjectCommandsDelete
+//-------------------------------------------------------------------------
+
+class CommandObjectCommandsDelete : public CommandObjectParsed
+{
+public:
+    CommandObjectCommandsDelete (CommandInterpreter &interpreter) :
+    CommandObjectParsed (interpreter,
+                         "command delete",
+                         "Allow the user to delete user-defined regular expression, python or multi-word commands.",
+                         NULL)
+    {
+        CommandArgumentEntry arg;
+        CommandArgumentData alias_arg;
+
+        // Define the first (and only) variant of this arg.
+        alias_arg.arg_type = eArgTypeCommandName;
+        alias_arg.arg_repetition = eArgRepeatPlain;
+
+        // There is only one variant this argument could be; put it into the argument entry.
+        arg.push_back (alias_arg);
+
+        // Push the data for the first argument into the m_arguments vector.
+        m_arguments.push_back (arg);
+    }
+
+    ~CommandObjectCommandsDelete()
+    {
+    }
+
+protected:
+    bool
+    DoExecute (Args& args, CommandReturnObject &result)
+    {
+        CommandObject::CommandMap::iterator pos;
+
+        if (args.GetArgumentCount() != 0)
+        {
+            const char *command_name = args.GetArgumentAtIndex(0);
+            if (m_interpreter.CommandExists (command_name))
+            {
+                if (m_interpreter.RemoveCommand (command_name))
+                {
+                    result.SetStatus (eReturnStatusSuccessFinishNoResult);
+                }
+                else
+                {
+                    result.AppendErrorWithFormat ("'%s' is a permanent debugger command and cannot be removed.\n",
+                                                  command_name);
+                    result.SetStatus (eReturnStatusFailed);
+                }
+            }
+            else
+            {
+                result.AppendErrorWithFormat ("'%s' is not a known command.\nTry 'help' to see a current list of commands.\n",
+                                              command_name);
+                result.SetStatus (eReturnStatusFailed);
+            }
+        }
+        else
+        {
+            result.AppendErrorWithFormat ("must call '%s' with one or more valid user defined regular expression, python or multi-word command names", GetCommandName ());
+            result.SetStatus (eReturnStatusFailed);
+        }
+
+        return result.Succeeded();
+    }
+};
+
 //-------------------------------------------------------------------------
 // CommandObjectCommandsAddRegex
 //-------------------------------------------------------------------------
@@ -979,7 +1058,9 @@ protected:
                                                                  name, 
                                                                  m_options.GetHelp (),
                                                                  m_options.GetSyntax (),
-                                                                 10));
+                                                                 10,
+                                                                 0,
+                                                                 true));
 
             if (argc == 1)
             {
@@ -1931,11 +2012,11 @@ public:
                             "A set of commands for managing or customizing script commands.",
                             "command script <subcommand> [<subcommand-options>]")
     {
-        LoadSubCommand ("add",  CommandObjectSP (new CommandObjectCommandsScriptAdd (interpreter)));
-        LoadSubCommand ("delete",   CommandObjectSP (new CommandObjectCommandsScriptDelete (interpreter)));
-        LoadSubCommand ("clear", CommandObjectSP (new CommandObjectCommandsScriptClear (interpreter)));
+        LoadSubCommand ("add",    CommandObjectSP (new CommandObjectCommandsScriptAdd (interpreter)));
+        LoadSubCommand ("delete", CommandObjectSP (new CommandObjectCommandsScriptDelete (interpreter)));
+        LoadSubCommand ("clear",  CommandObjectSP (new CommandObjectCommandsScriptClear (interpreter)));
         LoadSubCommand ("list",   CommandObjectSP (new CommandObjectCommandsScriptList (interpreter)));
-        LoadSubCommand ("import",   CommandObjectSP (new CommandObjectCommandsScriptImport (interpreter)));
+        LoadSubCommand ("import", CommandObjectSP (new CommandObjectCommandsScriptImport (interpreter)));
     }
 
     ~CommandObjectMultiwordCommandsScript ()
@@ -1960,9 +2041,10 @@ CommandObjectMultiwordCommands::CommandObjectMultiwordCommands (CommandInterpret
     LoadSubCommand ("source",  CommandObjectSP (new CommandObjectCommandsSource (interpreter)));
     LoadSubCommand ("alias",   CommandObjectSP (new CommandObjectCommandsAlias (interpreter)));
     LoadSubCommand ("unalias", CommandObjectSP (new CommandObjectCommandsUnalias (interpreter)));
+    LoadSubCommand ("delete",  CommandObjectSP (new CommandObjectCommandsDelete (interpreter)));
     LoadSubCommand ("regex",   CommandObjectSP (new CommandObjectCommandsAddRegex (interpreter)));
-    LoadSubCommand ("history",   CommandObjectSP (new CommandObjectCommandsHistory (interpreter)));
-    LoadSubCommand ("script",   CommandObjectSP (new CommandObjectMultiwordCommandsScript (interpreter)));
+    LoadSubCommand ("history", CommandObjectSP (new CommandObjectCommandsHistory (interpreter)));
+    LoadSubCommand ("script",  CommandObjectSP (new CommandObjectMultiwordCommandsScript (interpreter)));
 }
 
 CommandObjectMultiwordCommands::~CommandObjectMultiwordCommands ()
index 2b68f5c..176a1fc 100644 (file)
@@ -442,7 +442,8 @@ CommandInterpreter::LoadCommandDictionary ()
                                                       "_regexp-break [<filename>:<linenum>]\n_regexp-break [<linenum>]\n_regexp-break [<address>]\n_regexp-break <...>",
                                                       2,
                                                       CommandCompletions::eSymbolCompletion |
-                                                      CommandCompletions::eSourceFileCompletion));
+                                                      CommandCompletions::eSourceFileCompletion,
+                                                      false));
 
     if (break_regex_cmd_ap.get())
     {
@@ -469,7 +470,8 @@ CommandInterpreter::LoadCommandDictionary ()
                                                       "_regexp-tbreak [<filename>:<linenum>]\n_regexp-break [<linenum>]\n_regexp-break [<address>]\n_regexp-break <...>",
                                                        2,
                                                        CommandCompletions::eSymbolCompletion |
-                                                       CommandCompletions::eSourceFileCompletion));
+                                                       CommandCompletions::eSourceFileCompletion,
+                                                       false));
 
     if (tbreak_regex_cmd_ap.get())
     {
@@ -500,7 +502,9 @@ CommandInterpreter::LoadCommandDictionary ()
                                                        "_regexp-attach",
                                                        "Attach to a process id if in decimal, otherwise treat the argument as a process name to attach to.",
                                                        "_regexp-attach [<pid>]\n_regexp-attach [<process-name>]",
-                                                       2));
+                                                       2,
+                                                       0,
+                                                       false));
     if (attach_regex_cmd_ap.get())
     {
         if (attach_regex_cmd_ap->AddRegexCommand("^([0-9]+)[[:space:]]*$", "process attach --pid %1") &&
@@ -517,7 +521,10 @@ CommandInterpreter::LoadCommandDictionary ()
     down_regex_cmd_ap(new CommandObjectRegexCommand (*this,
                                                      "_regexp-down",
                                                      "Go down \"n\" frames in the stack (1 frame by default).",
-                                                     "_regexp-down [n]", 2));
+                                                     "_regexp-down [n]",
+                                                     2,
+                                                     0,
+                                                     false));
     if (down_regex_cmd_ap.get())
     {
         if (down_regex_cmd_ap->AddRegexCommand("^$", "frame select -r -1") &&
@@ -532,7 +539,10 @@ CommandInterpreter::LoadCommandDictionary ()
     up_regex_cmd_ap(new CommandObjectRegexCommand (*this,
                                                    "_regexp-up",
                                                    "Go up \"n\" frames in the stack (1 frame by default).",
-                                                   "_regexp-up [n]", 2));
+                                                   "_regexp-up [n]",
+                                                   2,
+                                                   0,
+                                                   false));
     if (up_regex_cmd_ap.get())
     {
         if (up_regex_cmd_ap->AddRegexCommand("^$", "frame select -r 1") &&
@@ -547,7 +557,10 @@ CommandInterpreter::LoadCommandDictionary ()
     display_regex_cmd_ap(new CommandObjectRegexCommand (*this,
                                                         "_regexp-display",
                                                         "Add an expression evaluation stop-hook.",
-                                                        "_regexp-display expression", 2));
+                                                        "_regexp-display expression",
+                                                        2,
+                                                        0,
+                                                        false));
     if (display_regex_cmd_ap.get())
     {
         if (display_regex_cmd_ap->AddRegexCommand("^(.+)$", "target stop-hook add -o \"expr -- %1\""))
@@ -561,7 +574,10 @@ CommandInterpreter::LoadCommandDictionary ()
     undisplay_regex_cmd_ap(new CommandObjectRegexCommand (*this,
                                                           "_regexp-undisplay",
                                                           "Remove an expression evaluation stop-hook.",
-                                                          "_regexp-undisplay stop-hook-number", 2));
+                                                          "_regexp-undisplay stop-hook-number",
+                                                          2,
+                                                          0,
+                                                          false));
     if (undisplay_regex_cmd_ap.get())
     {
         if (undisplay_regex_cmd_ap->AddRegexCommand("^([0-9]+)$", "target stop-hook delete %1"))
@@ -575,7 +591,10 @@ CommandInterpreter::LoadCommandDictionary ()
     connect_gdb_remote_cmd_ap(new CommandObjectRegexCommand (*this,
                                                              "gdb-remote",
                                                              "Connect to a remote GDB server.  If no hostname is provided, localhost is assumed.",
-                                                             "gdb-remote [<hostname>:]<portnum>", 2));
+                                                             "gdb-remote [<hostname>:]<portnum>",
+                                                             2,
+                                                             0,
+                                                             false));
     if (connect_gdb_remote_cmd_ap.get())
     {
         if (connect_gdb_remote_cmd_ap->AddRegexCommand("^([^:]+:[[:digit:]]+)$", "process connect --plugin gdb-remote connect://%1") &&
@@ -590,7 +609,10 @@ CommandInterpreter::LoadCommandDictionary ()
     connect_kdp_remote_cmd_ap(new CommandObjectRegexCommand (*this,
                                                              "kdp-remote",
                                                              "Connect to a remote KDP server.  udp port 41139 is the default port number.",
-                                                             "kdp-remote <hostname>[:<portnum>]", 2));
+                                                             "kdp-remote <hostname>[:<portnum>]",
+                                                             2,
+                                                             0,
+                                                             false));
     if (connect_kdp_remote_cmd_ap.get())
     {
         if (connect_kdp_remote_cmd_ap->AddRegexCommand("^([^:]+:[[:digit:]]+)$", "process connect --plugin kdp-remote udp://%1") &&
@@ -603,9 +625,12 @@ CommandInterpreter::LoadCommandDictionary ()
 
     std::unique_ptr<CommandObjectRegexCommand>
     bt_regex_cmd_ap(new CommandObjectRegexCommand (*this,
-                                                     "_regexp-bt",
-                                                     "Show a backtrace.  An optional argument is accepted; if that argument is a number, it specifies the number of frames to display.  If that argument is 'all', full backtraces of all threads are displayed.",
-                                                     "bt [<digit>|all]", 2));
+                                                   "_regexp-bt",
+                                                   "Show a backtrace.  An optional argument is accepted; if that argument is a number, it specifies the number of frames to display.  If that argument is 'all', full backtraces of all threads are displayed.",
+                                                   "bt [<digit>|all]",
+                                                   2,
+                                                   0,
+                                                   false));
     if (bt_regex_cmd_ap.get())
     {
         // accept but don't document "bt -c <number>" -- before bt was a regex command if you wanted to backtrace
@@ -627,7 +652,8 @@ CommandInterpreter::LoadCommandDictionary ()
                                                      "Implements the GDB 'list' command in all of its forms except FILE:FUNCTION and maps them to the appropriate 'source list' commands.",
                                                      "_regexp-list [<line>]\n_regexp-list [<file>:<line>]\n_regexp-list [<file>:<line>]",
                                                      2,
-                                                     CommandCompletions::eSourceFileCompletion));
+                                                     CommandCompletions::eSourceFileCompletion,
+                                                     false));
     if (list_regex_cmd_ap.get())
     {
         if (list_regex_cmd_ap->AddRegexCommand("^([0-9]+)[[:space:]]*$", "source list --line %1") &&
@@ -647,7 +673,10 @@ CommandInterpreter::LoadCommandDictionary ()
     env_regex_cmd_ap(new CommandObjectRegexCommand (*this,
                                                     "_regexp-env",
                                                     "Implements a shortcut to viewing and setting environment variables.",
-                                                    "_regexp-env\n_regexp-env FOO=BAR", 2));
+                                                    "_regexp-env\n_regexp-env FOO=BAR",
+                                                    2,
+                                                    0,
+                                                    false));
     if (env_regex_cmd_ap.get())
     {
         if (env_regex_cmd_ap->AddRegexCommand("^$", "settings show target.env-vars") &&
@@ -665,7 +694,10 @@ CommandInterpreter::LoadCommandDictionary ()
                                                     "_regexp-jump [<line>]\n"
                                                     "_regexp-jump [<+-lineoffset>]\n"
                                                     "_regexp-jump [<file>:<line>]\n"
-                                                    "_regexp-jump [*<addr>]\n", 2));
+                                                    "_regexp-jump [*<addr>]\n",
+                                                     2,
+                                                     0,
+                                                     false));
     if (jump_regex_cmd_ap.get())
     {
         if (jump_regex_cmd_ap->AddRegexCommand("^\\*(.*)$", "thread jump --addr %1") &&
@@ -1056,6 +1088,22 @@ CommandInterpreter::RemoveAlias (const char *alias_name)
     }
     return false;
 }
+
+bool
+CommandInterpreter::RemoveCommand (const char *cmd)
+{
+    auto pos = m_command_dict.find(cmd);
+    if (pos != m_command_dict.end())
+    {
+        if (pos->second->IsRemovable())
+        {
+            // Only regular expression objects or python commands are removable
+            m_command_dict.erase(pos);
+            return true;
+        }
+    }
+    return false;
+}
 bool
 CommandInterpreter::RemoveUser (const char *alias_name)
 {
index d27320d..efc7c33 100644 (file)
@@ -31,12 +31,14 @@ CommandObjectRegexCommand::CommandObjectRegexCommand
     const char *help,
     const char *syntax,
     uint32_t max_matches,
-    uint32_t completion_type_mask
+    uint32_t completion_type_mask,
+    bool is_removable
 ) :
     CommandObjectRaw (interpreter, name, help, syntax),
     m_max_matches (max_matches),
     m_completion_type_mask (completion_type_mask),
-    m_entries ()
+    m_entries (),
+    m_is_removable (is_removable)
 {
 }
 
index eabbd19..9571bb2 100644 (file)
@@ -37,6 +37,19 @@ class CommandRegexTestCase(TestBase):
         child.sendline('Help__')
         # If we see the familiar 'help' output, the test is done.
         child.expect('The following is a list of built-in, permanent debugger commands:')
+        
+        # Try and incorrectly remove "Help__" using "command unalias" and verify we fail
+        child.sendline('command unalias Help__')
+        child.expect_exact("error: 'Help__' is not an alias, it is a debugger command which can be removed using the 'command delete' command")
+        child.expect_exact(prompt)
+        
+        # Delete the regex command using "command delete"
+        child.sendline('command delete Help__')
+        child.expect_exact(prompt)
+        # Verify the command was removed
+        child.sendline('Help__')
+        child.expect_exact("error: 'Help__' is not a valid command")
+        child.expect_exact(prompt)
 
 if __name__ == '__main__':
     import atexit