Lots of progress on the CommandAlias refactoring

This cleans things up such CommandAlias essentially can work as its own object; the aliases still live in a separate map, but are now just full-fledged CommandObjectSPs
This patch also cleans up help generation for aliases, allows aliases to vend their own help, and adds a tweak such that "dash-dash aliases", such as po, don't show the list of options for their underlying command, since those can't be provided anyway

I plan to fix up a few more things here, and then add a test case and proclaim victory

llvm-svn: 263499

lldb/include/lldb/Interpreter/CommandAlias.h

@@ -51,9 +51,43 @@ public:
     bool
     WantsRawCommandString() override;
     
+    bool
+    WantsCompletion() override;
+    
+    int
+    HandleCompletion (Args &input,
+                      int &cursor_index,
+                      int &cursor_char_position,
+                      int match_start_point,
+                      int max_return_elements,
+                      bool &word_complete,
+                      StringList &matches) override;
+    
+    int
+    HandleArgumentCompletion (Args &input,
+                              int &cursor_index,
+                              int &cursor_char_position,
+                              OptionElementVector &opt_element_vector,
+                              int match_start_point,
+                              int max_return_elements,
+                              bool &word_complete,
+                              StringList &matches) override;
+    
+    Options*
+    GetOptions() override;
+    
     bool
     IsAlias () override { return true; }
     
+    bool
+    IsDashDashCommand () override;
+    
+    const char*
+    GetHelp () override;
+    
+    const char*
+    GetHelpLong () override;
+    
     bool
     Execute(const char *args_string, CommandReturnObject &result) override;
     
@@ -63,6 +97,7 @@ public:
 private:
     lldb::CommandObjectSP m_underlying_command_sp;
     OptionArgVectorSP m_option_args_sp ;
+    LazyBool m_is_dashdash_alias;
 };
 } // namespace lldb_private
 

lldb/include/lldb/Interpreter/CommandObject.h

@@ -148,7 +148,7 @@ public:
     virtual const char *
     GetHelpLong ();
 
-    const char *
+    virtual const char *
     GetSyntax ();
 
     const char *
@@ -180,6 +180,12 @@ public:
     
     virtual bool
     IsAlias () { return false; }
+    
+    // override this to return true if your command is somehow a "dash-dash"
+    // form of some other command (e.g. po is expr -O --); this is a powerful
+    // hint to the help system that one cannot pass options to this command
+    virtual bool
+    IsDashDashCommand () { return false; }
 
     virtual lldb::CommandObjectSP
     GetSubcommandSP(const char *sub_cmd, StringList *matches = nullptr)

lldb/source/Commands/CommandObjectHelp.cpp

@@ -128,6 +128,8 @@ CommandObjectHelp::DoExecute (Args& command, CommandReturnObject &result)
             {
                 sub_command = command.GetArgumentAtIndex(i);
                 matches.Clear();
+                if (sub_cmd_obj->IsAlias())
+                    sub_cmd_obj = ((CommandAlias*)sub_cmd_obj)->GetUnderlyingCommand().get();
                 if (! sub_cmd_obj->IsMultiwordObject ())
                 {
                     all_okay = false;

lldb/source/Interpreter/CommandAlias.cpp

@@ -85,11 +85,18 @@ CommandAlias::CommandAlias (CommandInterpreter &interpreter,
                   syntax,
                   flags),
 m_underlying_command_sp(),
-m_option_args_sp(new OptionArgVector)
+m_option_args_sp(new OptionArgVector),
+m_is_dashdash_alias(eLazyBoolCalculate)
 {
     if (ProcessAliasOptionsArgs(cmd_sp, options_args, m_option_args_sp))
     {
         m_underlying_command_sp = cmd_sp;
+        for (int i = 0;
+             auto cmd_entry = m_underlying_command_sp->GetArgumentEntryAtIndex(i);
+             i++)
+        {
+            m_arguments.push_back(*cmd_entry);
+        }
         if (!help || !help[0])
         {
             StreamString sstr;
@@ -110,6 +117,64 @@ CommandAlias::WantsRawCommandString()
     return false;
 }
 
+bool
+CommandAlias::WantsCompletion()
+{
+    if (IsValid())
+        return m_underlying_command_sp->WantsCompletion();
+    return false;
+}
+
+int
+CommandAlias::HandleCompletion (Args &input,
+                  int &cursor_index,
+                  int &cursor_char_position,
+                  int match_start_point,
+                  int max_return_elements,
+                  bool &word_complete,
+                  StringList &matches)
+{
+    if (IsValid())
+        return m_underlying_command_sp->HandleCompletion(input,
+                                                         cursor_index,
+                                                         cursor_char_position,
+                                                         match_start_point,
+                                                         max_return_elements,
+                                                         word_complete,
+                                                         matches);
+    return -1;
+}
+
+int
+CommandAlias::HandleArgumentCompletion (Args &input,
+                          int &cursor_index,
+                          int &cursor_char_position,
+                          OptionElementVector &opt_element_vector,
+                          int match_start_point,
+                          int max_return_elements,
+                          bool &word_complete,
+                          StringList &matches)
+{
+    if (IsValid())
+        return m_underlying_command_sp->HandleArgumentCompletion(input,
+                                                                 cursor_index,
+                                                                 cursor_char_position,
+                                                                 opt_element_vector,
+                                                                 match_start_point,
+                                                                 max_return_elements,
+                                                                 word_complete,
+                                                                 matches);
+    return -1;
+}
+
+Options*
+CommandAlias::GetOptions()
+{
+    if (IsValid())
+        return m_underlying_command_sp->GetOptions();
+    return nullptr;
+}
+
 bool
 CommandAlias::Execute(const char *args_string, CommandReturnObject &result)
 {
@@ -149,3 +214,47 @@ CommandAlias::GetAliasExpansion (StreamString &help_string)
     
     help_string.Printf ("'");
 }
+
+bool
+CommandAlias::IsDashDashCommand ()
+{
+    if (m_is_dashdash_alias == eLazyBoolCalculate)
+    {
+        m_is_dashdash_alias = eLazyBoolNo;
+        if (IsValid())
+        {
+            for (const OptionArgPair& opt_arg : *GetOptionArguments())
+            {
+                if (opt_arg.first == "<argument>" &&
+                    opt_arg.second.second == " --")
+                {
+                    m_is_dashdash_alias = eLazyBoolYes;
+                    break;
+                }
+            }
+        }
+    }
+    return (m_is_dashdash_alias == eLazyBoolYes);
+}
+
+// allow CommandAlias objects to provide their own help, but fallback to the info
+// for the underlying command if no customization has been provided
+const char*
+CommandAlias::GetHelp ()
+{
+    if (!m_cmd_help_short.empty())
+        return m_cmd_help_short.c_str();
+    if (IsValid())
+        return m_underlying_command_sp->GetHelp();
+    return nullptr;
+}
+
+const char*
+CommandAlias::GetHelpLong ()
+{
+    if (!m_cmd_help_long.empty())
+        return m_cmd_help_long.c_str();
+    if (IsValid())
+        return m_underlying_command_sp->GetHelpLong();
+    return nullptr;
+}

lldb/source/Interpreter/CommandInterpreter.cpp

@@ -772,7 +772,7 @@ CommandInterpreter::GetCommandSP (const char *cmd_cstr, bool include_aliases, bo
     {
         CommandAliasMap::iterator alias_pos = m_alias_dict.find(cmd);
         if (alias_pos != m_alias_dict.end())
-            command_sp = ((CommandAlias*)alias_pos->second.get())->GetUnderlyingCommand();
+            command_sp = alias_pos->second;
     }
 
     if (HasUserCommands())
@@ -823,7 +823,7 @@ CommandInterpreter::GetCommandSP (const char *cmd_cstr, bool include_aliases, bo
             cmd.assign(matches->GetStringAtIndex (num_cmd_matches));
             CommandAliasMap::iterator alias_pos = m_alias_dict.find(cmd);
             if (alias_pos != m_alias_dict.end())
-                alias_match_sp = ((CommandAlias*)alias_pos->second.get())->GetUnderlyingCommand();
+                alias_match_sp = alias_pos->second;
         }
 
         if (HasUserCommands())
@@ -1351,15 +1351,16 @@ CommandInterpreter::BuildAliasResult (const char *alias_name,
     alias_cmd_obj = GetCommandObject (alias_name);
     StreamString result_str;
     
-    if (alias_cmd_obj)
+    if (alias_cmd_obj && alias_cmd_obj->IsAlias())
     {
+        OptionArgVectorSP option_arg_vector_sp =  ((CommandAlias*)alias_cmd_obj)->GetOptionArguments();
+        alias_cmd_obj = ((CommandAlias*)alias_cmd_obj)->GetUnderlyingCommand().get();
         std::string alias_name_str = alias_name;
         if ((cmd_args.GetArgumentCount() == 0)
             || (alias_name_str.compare (cmd_args.GetArgumentAtIndex(0)) != 0))
             cmd_args.Unshift (alias_name);
             
         result_str.Printf ("%s", alias_cmd_obj->GetCommandName ());
-        OptionArgVectorSP option_arg_vector_sp = GetAlias(alias_name)->GetOptionArguments();
 
         if (option_arg_vector_sp.get())
         {

lldb/source/Interpreter/CommandObject.cpp

@@ -88,12 +88,12 @@ CommandObject::GetSyntax ()
     {
         StreamString syntax_str;
         syntax_str.Printf ("%s", GetCommandName());
-        if (GetOptions() != nullptr)
+        if (!IsDashDashCommand() && GetOptions() != nullptr)
             syntax_str.Printf (" <cmd-options>");
         if (m_arguments.size() > 0)
         {
             syntax_str.Printf (" ");
-            if (WantsRawCommandString() && GetOptions() && GetOptions()->NumCommandOptions())
+            if (!IsDashDashCommand() && WantsRawCommandString() && GetOptions() && GetOptions()->NumCommandOptions())
                 syntax_str.Printf("-- ");
             GetFormattedCommandArguments (syntax_str);
         }

lldb/source/Interpreter/Options.cpp

@@ -14,6 +14,7 @@
 #include <algorithm>
 #include <bitset>
 #include <map>
+#include <set>
 
 // Other libraries and framework includes
 // Project includes
@@ -477,6 +478,7 @@ Options::GenerateOptionUsage
     CommandObject *cmd
 )
 {
+    const bool only_print_args = cmd->IsDashDashCommand();
     const uint32_t screen_width = m_interpreter.GetDebugger().GetTerminalWidth();
 
     const OptionDefinition *opt_defs = GetDefinitions();
@@ -523,105 +525,110 @@ Options::GenerateOptionUsage
         if (cmd)
             cmd->GetFormattedCommandArguments(args_str, opt_set_mask);
 
-        // First go through and print all options that take no arguments as
+        if (!only_print_args)
-        // a single string. If a command has "-a" "-b" and "-c", this will show
-        // up as [-abc]
-
-        std::set<int> options;
-        std::set<int>::const_iterator options_pos, options_end;
-        for (i = 0; i < num_options; ++i)
         {
-            if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
+            // First go through and print all options that take no arguments as
-            {
+            // a single string. If a command has "-a" "-b" and "-c", this will show
-                // Add current option to the end of out_stream.
+            // up as [-abc]
 
-                if (opt_defs[i].required == true && 
+            std::set<int> options;
-                    opt_defs[i].option_has_arg == OptionParser::eNoArgument)
+            std::set<int>::const_iterator options_pos, options_end;
+            for (i = 0; i < num_options; ++i)
+            {
+                if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
                 {
-                    options.insert (opt_defs[i].short_option);
+                    // Add current option to the end of out_stream.
+
+                    if (opt_defs[i].required == true && 
+                        opt_defs[i].option_has_arg == OptionParser::eNoArgument)
+                    {
+                        options.insert (opt_defs[i].short_option);
+                    }
                 }
             }
-        }
 
-        if (options.empty() == false)
+            if (options.empty() == false)
-        {
-            // We have some required options with no arguments
-            strm.PutCString(" -");
-            for (i=0; i<2; ++i)            
-                for (options_pos = options.begin(), options_end = options.end();
-                     options_pos != options_end;
-                     ++options_pos)
-                {
-                    if (i==0 && ::islower (*options_pos))
-                        continue;
-                    if (i==1 && ::isupper (*options_pos))
-                        continue;
-                    strm << (char)*options_pos;
-                }
-        }
-
-        for (i = 0, options.clear(); i < num_options; ++i)
-        {
-            if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
             {
-                // Add current option to the end of out_stream.
+                // We have some required options with no arguments
+                strm.PutCString(" -");
+                for (i=0; i<2; ++i)            
+                    for (options_pos = options.begin(), options_end = options.end();
+                         options_pos != options_end;
+                         ++options_pos)
+                    {
+                        if (i==0 && ::islower (*options_pos))
+                            continue;
+                        if (i==1 && ::isupper (*options_pos))
+                            continue;
+                        strm << (char)*options_pos;
+                    }
+            }
 
-                if (opt_defs[i].required == false &&
+            for (i = 0, options.clear(); i < num_options; ++i)
-                    opt_defs[i].option_has_arg == OptionParser::eNoArgument)
+            {
+                if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
                 {
-                    options.insert (opt_defs[i].short_option);
+                    // Add current option to the end of out_stream.
+
+                    if (opt_defs[i].required == false &&
+                        opt_defs[i].option_has_arg == OptionParser::eNoArgument)
+                    {
+                        options.insert (opt_defs[i].short_option);
+                    }
                 }
             }
-        }
 
-        if (options.empty() == false)
+            if (options.empty() == false)
-        {
-            // We have some required options with no arguments
-            strm.PutCString(" [-");
-            for (i=0; i<2; ++i)            
-                for (options_pos = options.begin(), options_end = options.end();
-                     options_pos != options_end;
-                     ++options_pos)
-                {
-                    if (i==0 && ::islower (*options_pos))
-                        continue;
-                    if (i==1 && ::isupper (*options_pos))
-                        continue;
-                    strm << (char)*options_pos;
-                }
-            strm.PutChar(']');
-        }
-
-        // First go through and print the required options (list them up front).
-        
-        for (i = 0; i < num_options; ++i)
-        {
-            if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
             {
-                if (opt_defs[i].required && opt_defs[i].option_has_arg != OptionParser::eNoArgument)
+                // We have some required options with no arguments
-                    PrintOption (opt_defs[i], eDisplayBestOption, " ", nullptr, true, strm);
+                strm.PutCString(" [-");
+                for (i=0; i<2; ++i)            
+                    for (options_pos = options.begin(), options_end = options.end();
+                         options_pos != options_end;
+                         ++options_pos)
+                    {
+                        if (i==0 && ::islower (*options_pos))
+                            continue;
+                        if (i==1 && ::isupper (*options_pos))
+                            continue;
+                        strm << (char)*options_pos;
+                    }
+                strm.PutChar(']');
             }
-        }
 
-        // Now go through again, and this time only print the optional options.
+            // First go through and print the required options (list them up front).
-
+            
-        for (i = 0; i < num_options; ++i)
+            for (i = 0; i < num_options; ++i)
-        {
-            if (opt_defs[i].usage_mask & opt_set_mask)
             {
-                // Add current option to the end of out_stream.
+                if (opt_defs[i].usage_mask & opt_set_mask && isprint8(opt_defs[i].short_option))
+                {
+                    if (opt_defs[i].required && opt_defs[i].option_has_arg != OptionParser::eNoArgument)
+                        PrintOption (opt_defs[i], eDisplayBestOption, " ", nullptr, true, strm);
+                }
+            }
 
-                if (!opt_defs[i].required && opt_defs[i].option_has_arg != OptionParser::eNoArgument)
+            // Now go through again, and this time only print the optional options.
-                    PrintOption (opt_defs[i], eDisplayBestOption, " ", nullptr, true, strm);
+
+            for (i = 0; i < num_options; ++i)
+            {
+                if (opt_defs[i].usage_mask & opt_set_mask)
+                {
+                    // Add current option to the end of out_stream.
+
+                    if (!opt_defs[i].required && opt_defs[i].option_has_arg != OptionParser::eNoArgument)
+                        PrintOption (opt_defs[i], eDisplayBestOption, " ", nullptr, true, strm);
+                }
             }
         }
         
         if (args_str.GetSize() > 0)
         {
-            if (cmd->WantsRawCommandString())
+            if (cmd->WantsRawCommandString() && !only_print_args)
                 strm.Printf(" --");
             
             strm.Printf (" %s", args_str.GetData());
+            if (only_print_args)
+                break;
         }
     }
     
@@ -636,76 +643,79 @@ Options::GenerateOptionUsage
     
     strm.Printf ("\n\n");
 
-    // Now print out all the detailed information about the various options:  long form, short form and help text:
+    if (!only_print_args)
-    //   -short <argument> ( --long_name <argument> )
-    //   help text
-
-    // This variable is used to keep track of which options' info we've printed out, because some options can be in
-    // more than one usage level, but we only want to print the long form of its information once.
-
-    std::multimap<int, uint32_t> options_seen;
-    strm.IndentMore (5);
-
-    // Put the unique command options in a vector & sort it, so we can output them alphabetically (by short_option)
-    // when writing out detailed help for each option.
-
-    for (i = 0; i < num_options; ++i)
-        options_seen.insert(std::make_pair(opt_defs[i].short_option, i));
-
-    // Go through the unique'd and alphabetically sorted vector of options, find the table entry for each option
-    // and write out the detailed help information for that option.
-
-    bool first_option_printed = false;;
-
-    for (auto pos : options_seen)
     {
-        i = pos.second;
+        // Now print out all the detailed information about the various options:  long form, short form and help text:
-        //Print out the help information for this option.
+        //   -short <argument> ( --long_name <argument> )
+        //   help text
 
-        // Put a newline separation between arguments
+        // This variable is used to keep track of which options' info we've printed out, because some options can be in
-        if (first_option_printed)
+        // more than one usage level, but we only want to print the long form of its information once.
-            strm.EOL();
-        else
-            first_option_printed = true;
-        
-        CommandArgumentType arg_type = opt_defs[i].argument_type;
-        
-        StreamString arg_name_str;
-        arg_name_str.Printf ("<%s>", CommandObject::GetArgumentName (arg_type));
 
-        strm.Indent ();
+        std::multimap<int, uint32_t> options_seen;
-        if (opt_defs[i].short_option && isprint8(opt_defs[i].short_option))
-        {
-            PrintOption (opt_defs[i], eDisplayShortOption, nullptr, nullptr, false, strm);
-            PrintOption (opt_defs[i], eDisplayLongOption, " ( ", " )", false, strm);
-        }
-        else
-        {
-            // Short option is not printable, just print long option
-            PrintOption (opt_defs[i], eDisplayLongOption, nullptr, nullptr, false, strm);
-        }
-        strm.EOL();
-        
         strm.IndentMore (5);
-        
+
-        if (opt_defs[i].usage_text)
+        // Put the unique command options in a vector & sort it, so we can output them alphabetically (by short_option)
-            OutputFormattedUsageText (strm,
+        // when writing out detailed help for each option.
-                                      opt_defs[i],
+
-                                      screen_width);
+        for (i = 0; i < num_options; ++i)
-        if (opt_defs[i].enum_values != nullptr)
+            options_seen.insert(std::make_pair(opt_defs[i].short_option, i));
+
+        // Go through the unique'd and alphabetically sorted vector of options, find the table entry for each option
+        // and write out the detailed help information for that option.
+
+        bool first_option_printed = false;;
+
+        for (auto pos : options_seen)
         {
+            i = pos.second;
+            //Print out the help information for this option.
+
+            // Put a newline separation between arguments
+            if (first_option_printed)
+                strm.EOL();
+            else
+                first_option_printed = true;
+            
+            CommandArgumentType arg_type = opt_defs[i].argument_type;
+            
+            StreamString arg_name_str;
+            arg_name_str.Printf ("<%s>", CommandObject::GetArgumentName (arg_type));
+
             strm.Indent ();
-            strm.Printf("Values: ");
+            if (opt_defs[i].short_option && isprint8(opt_defs[i].short_option))
-            for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr; k++)
             {
-                if (k == 0)
+                PrintOption (opt_defs[i], eDisplayShortOption, nullptr, nullptr, false, strm);
-                    strm.Printf("%s", opt_defs[i].enum_values[k].string_value);
+                PrintOption (opt_defs[i], eDisplayLongOption, " ( ", " )", false, strm);
-                else
+            }
-                    strm.Printf(" | %s", opt_defs[i].enum_values[k].string_value);
+            else
+            {
+                // Short option is not printable, just print long option
+                PrintOption (opt_defs[i], eDisplayLongOption, nullptr, nullptr, false, strm);
             }
             strm.EOL();
+            
+            strm.IndentMore (5);
+            
+            if (opt_defs[i].usage_text)
+                OutputFormattedUsageText (strm,
+                                          opt_defs[i],
+                                          screen_width);
+            if (opt_defs[i].enum_values != nullptr)
+            {
+                strm.Indent ();
+                strm.Printf("Values: ");
+                for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr; k++)
+                {
+                    if (k == 0)
+                        strm.Printf("%s", opt_defs[i].enum_values[k].string_value);
+                    else
+                        strm.Printf(" | %s", opt_defs[i].enum_values[k].string_value);
+                }
+                strm.EOL();
+            }
+            strm.IndentLess (5);
         }
-        strm.IndentLess (5);
     }
 
     // Restore the indent level