[lldb] Add description to option completions.

[teemperor]

Summary

Right now our argument completions are rather cryptic for command options as they only list the letters:

(lldb) breakpoint set -
Available completions:
	-G
	-C
	-c
	-d
	-i
	-o
	-q
	-t
	-x
[...]

With the new completion API we can easily extend this with the flag description so that it looks like this now:

(lldb) breakpoint set -
Available completions:
	-G -- The breakpoint will auto-continue after running its commands.
	-C -- A command to run when the breakpoint is hit, can be provided more than once, the commands will get run in order left to right.
	-c -- The breakpoint stops only if this condition expression evaluates to true.
	-d -- Disable the breakpoint.
	-i -- Set the number of times this breakpoint is skipped before stopping.
	-o -- The breakpoint is deleted the first time it stop causes a stop.
	-q -- The breakpoint stops only for threads in the queue whose name is given by this argument.
	-t -- The breakpoint stops only for the thread whose TID matches this argument.
	-x -- The breakpoint stops only for the thread whose index matches this argument.

The same happens with --long-options now.

[labath]

Sounds like a good idea.

I noticed that you have three tests, but are changing four places in the code. Looks like you're probably missing one of them. By the looks of things, it's probably the ambiguous long option case (--ambig<TAB>)..

[teemperor]

Good catch, extended the test to also test the ambiguous options.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py

def test_completion_description_commands(self):
             ["platform", "Commands to manage and create platforms."],
             ["plugin", "Commands for managing LLDB plugins."]
         ])
 
         # Just check that this doesn't crash.
         self.check_completion_with_desc("comman", [])
         self.check_completion_with_desc("non-existent-command", [])
 
+    def test_completion_description_command_options(self):
+        """Test descriptions of command options"""
+        # Short options
+        self.check_completion_with_desc("breakpoint set -", [
+            ["-h", "Set the breakpoint on exception catcH."],
+            ["-w", "Set the breakpoint on exception throW."]
+        ])
+
+        # Long options.
+        self.check_completion_with_desc("breakpoint set --", [
+            ["--on-catch", "Set the breakpoint on exception catcH."],
+            ["--on-throw", "Set the breakpoint on exception throW."]
+        ])
+
+        # Ambiguous long options.
+        self.check_completion_with_desc("breakpoint set --on-", [
+            ["--on-catch", "Set the breakpoint on exception catcH."],
+            ["--on-throw", "Set the breakpoint on exception throW."]
+        ])
+
+        # Unknown long option.
+        self.check_completion_with_desc("breakpoint set --Z", [
+        ])
+
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489")
     def test_symbol_name(self):
         self.build()
         self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
         self.complete_from_to('breakpoint set -n Fo',
                               'breakpoint set -n Foo::Bar(int,\\ int)',
                               turn_off_re_match=True)
         # No completion for Qu because the candidate is
         # (anonymous namespace)::Quux().
         self.complete_from_to('breakpoint set -n Qu', '')

lldb/trunk/source/Interpreter/Options.cpp

if (opt_pos == request.GetCursorIndex()) {
         // options
         // within the option group they belong to.
         std::string opt_str = "-a";
 
         for (auto &def : opt_defs) {
           if (!def.short_option)
             continue;
           opt_str[1] = def.short_option;
-          request.AddCompletion(opt_str);
+          request.AddCompletion(opt_str, def.usage_text);
         }
 
         return true;
       } else if (opt_defs_index == OptionArgElement::eBareDoubleDash) {
         std::string full_name("--");
         for (auto &def : opt_defs) {
           if (!def.short_option)
             continue;
 
           full_name.erase(full_name.begin() + 2, full_name.end());
           full_name.append(def.long_option);
-          request.AddCompletion(full_name);
+          request.AddCompletion(full_name, def.usage_text);
         }
         return true;
       } else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) {
         // We recognized it, if it an incomplete long option, complete it
         // anyway (getopt_long_only is happy with shortest unique string, but
         // it's still a nice thing to do.)  Otherwise return The string so the
         // upper level code will know this is a full match and add the " ".
-        llvm::StringRef long_option = opt_defs[opt_defs_index].long_option;
+        const OptionDefinition &opt = opt_defs[opt_defs_index];
+        llvm::StringRef long_option = opt.long_option;
         if (cur_opt_str.startswith("--") && cur_opt_str != long_option) {
-          request.AddCompletion("--" + long_option.str());
+          request.AddCompletion("--" + long_option.str(), opt.usage_text);
           return true;
         } else
           request.AddCompletion(request.GetCursorArgument());
         return true;
       } else {
         // FIXME - not handling wrong options yet:
         // Check to see if they are writing a long option & complete it.
         // I think we will only get in here if the long option table has two
         // elements
         // that are not unique up to this point.  getopt_long_only does
         // shortest unique match for long options already.
         if (cur_opt_str.consume_front("--")) {
           for (auto &def : opt_defs) {
             llvm::StringRef long_option(def.long_option);
             if (long_option.startswith(cur_opt_str))
-              request.AddCompletion("--" + long_option.str());
+              request.AddCompletion("--" + long_option.str(), def.usage_text);
           }
         }
         return true;
       }
 
     } else if (opt_arg_pos == request.GetCursorIndex()) {
       // Okay the cursor is on the completion of an argument. See if it has a
       // completion, otherwise return no matches.