[lldb] Don't print "Command Options Usage:" for an alias with no options

[DavidSpickett]

Summary

"shell" is an alias to "platform shell -h --". Previously you would get this
help text:

(lldb) help shell
Run a shell command on the host. Expects 'raw' input (see 'help raw-input'.)

Syntax: shell <shell-command>

Command Options Usage:

'shell' is an abbreviation for 'platform shell -h --'

Since the code doesn't handle the base command having options
but the alias removing them. With these changes you get:

(lldb) help shell
Run a shell command on the host. Expects 'raw' input (see 'help raw-input'.)

Syntax: shell <shell-command>

'shell' is an abbreviation for 'platform shell -h --'

Note that we already handle a non-alias command having no options,
for example "quit":

(lldb) help quit
Quit the LLDB debugger.

Syntax: quit [exit-code]

[JDevlieghere]

LGTM

[jingham]

I was confused by your example of the "quit" command since the previous code output the banner before checking the NumCommandOptions, so that shouldn't have worked. Actually, there's a check in CommandObject::GenerateHelp for a null return from GetOptions before this GenerateOptionUsage gets called, which bypasses the call. Anyway, with this change we'll do this correctly even if we get here in some other way, so that's all good.

I also think this function is pretty confusing to read. First off, only_print_args is a really confusing name for "is_dash_dash_command", which is what that bool really signifies. Also, we do a bunch of checking for both only_print_args and !only_print_args inside the first "if (!only_print_args)" block, but that variable is a const so that's just weird.

So far as I can see, the only thing we do in the only_print_args == true case is this block:

if (cmd && (only_print_args || cmd->WantsRawCommandString()) &&
    arguments_str.GetSize() > 0) {
  if (!only_print_args)
    strm.PutChar('\n');
  strm.Indent(name);
  strm << " " << arguments_str.GetString();
}

plus resetting the indentation.

This function would be much easier to read if we handle IsDashDash commands as an early return, emitting the command name and arguments, and returning. Then we could remove both of the if (!only_print_args) blocks, and the weird checks inside those blocks, and this would make more sense. That little snippet is sufficiently obvious I don't think repeating it would be a big deal.

OTOH, you didn't originally write this code, so if you don't have time to clean it up further, then this LGTM.

[DavidSpickett]

I also think this function is pretty confusing to read.

Enough that I decided not to attempt to unpick it for this bug! I also wondered how the quit command actually worked so thanks for solving that mystery.

I'll go with this as is but if I get a chance I'll try to simplify things like you suggested.

lldb/source/Interpreter/Options.cpp

void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
   StreamString arguments_str;
 
   if (cmd) {
     name = cmd->GetCommandName();
     cmd->GetFormattedCommandArguments(arguments_str);
   } else
     name = "";
 
+  const uint32_t num_options = NumCommandOptions();
+  if (num_options == 0)
+    return;
+
+  if (!only_print_args)
     strm.PutCString("\nCommand Options Usage:\n");
 
   strm.IndentMore(2);
 
   // First, show each usage level set of options, e.g. <cmd> [options-for-
   // level-0]
   //                                                   <cmd>
   //                                                   [options-for-level-1]
   //                                                   etc.
 
-  const uint32_t num_options = NumCommandOptions();
-  if (num_options == 0)
-    return;
-
   uint32_t num_option_sets = GetRequiredOptions().size();
 
   uint32_t i;
 
   if (!only_print_args) {
     for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
       uint32_t opt_set_mask;
 
void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
   if (cmd && (only_print_args || cmd->WantsRawCommandString()) &&
       arguments_str.GetSize() > 0) {
     if (!only_print_args)
       strm.PutChar('\n');
     strm.Indent(name);
     strm << " " << arguments_str.GetString();
   }
 
+  if (!only_print_args) {
     strm.Printf("\n\n");
 
-  if (!only_print_args) {
     // Now print out all the detailed information about the various options:
     // long form, short form and help text:
     //   -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.

lldb/test/API/commands/platform/basic/TestPlatformCommand.py

class PlatformCommandTestCase(TestBase):
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
     @no_debug_info_test
     def test_help_platform(self):
         self.runCmd("help platform")
 
     @no_debug_info_test
-    def test_help_platform(self):
+    def test_help_shell_alias(self):
         self.expect("help shell", substrs=["Run a shell command on the host.",
-                                           "shell <shell-command>"])
-
+                                           "shell <shell-command>",
+                                           "'shell' is an abbreviation"])
+        # "platform shell" has options. The "shell" alias for it does not.
+        self.expect("help shell", substrs=["Command Options:"], matching=False)
 
     @no_debug_info_test
     def test_list(self):
         self.expect("platform list",
                     patterns=['^Available platforms:'])
 
     @no_debug_info_test
     def test_process_list(self):