Index: lldb/trunk/include/lldb/API/SBCommandInterpreter.h =================================================================== --- lldb/trunk/include/lldb/API/SBCommandInterpreter.h +++ lldb/trunk/include/lldb/API/SBCommandInterpreter.h @@ -162,6 +162,20 @@ int match_start_point, int max_return_elements, lldb::SBStringList &matches); + // Same as HandleCompletion, but also fills out `descriptions` with + // descriptions for each match. + int HandleCompletionWithDescriptions( + const char *current_line, const char *cursor, const char *last_char, + int match_start_point, int max_return_elements, + lldb::SBStringList &matches, lldb::SBStringList &descriptions); + + int HandleCompletionWithDescriptions(const char *current_line, + uint32_t cursor_pos, + int match_start_point, + int max_return_elements, + lldb::SBStringList &matches, + lldb::SBStringList &descriptions); + bool WasInterrupted() const; // Catch commands before they execute by registering a callback that will get Index: lldb/trunk/include/lldb/Core/IOHandler.h =================================================================== --- lldb/trunk/include/lldb/Core/IOHandler.h +++ lldb/trunk/include/lldb/Core/IOHandler.h @@ -205,7 +205,7 @@ virtual int IOHandlerComplete(IOHandler &io_handler, const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches); + StringList &matches, StringList &descriptions); virtual const char *IOHandlerGetFixIndentationCharacters() { return nullptr; } @@ -430,7 +430,8 @@ static int AutoCompleteCallback(const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches, void *baton); + StringList &matches, StringList &descriptions, + void *baton); #endif protected: @@ -464,7 +465,7 @@ int IOHandlerComplete(IOHandler &io_handler, const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches) override; + StringList &matches, StringList &descriptions) override; void IOHandlerInputComplete(IOHandler &io_handler, std::string &data) override; Index: lldb/trunk/include/lldb/Expression/REPL.h =================================================================== --- lldb/trunk/include/lldb/Expression/REPL.h +++ lldb/trunk/include/lldb/Expression/REPL.h @@ -117,7 +117,7 @@ int IOHandlerComplete(IOHandler &io_handler, const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches) override; + StringList &matches, StringList &descriptions) override; protected: static int CalculateActualIndentation(const StringList &lines); Index: lldb/trunk/include/lldb/Host/Editline.h =================================================================== --- lldb/trunk/include/lldb/Host/Editline.h +++ lldb/trunk/include/lldb/Host/Editline.h @@ -101,7 +101,8 @@ typedef int (*CompleteCallbackType)(const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches, void *baton); + StringList &matches, + StringList &descriptions, void *baton); /// Status used to decide when and how to start editing another line in /// multi-line sessions Index: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h =================================================================== --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h +++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h @@ -206,7 +206,8 @@ bool include_aliases) const; CommandObject *GetCommandObject(llvm::StringRef cmd, - StringList *matches = nullptr) const; + StringList *matches = nullptr, + StringList *descriptions = nullptr) const; bool CommandExists(llvm::StringRef cmd) const; @@ -310,7 +311,8 @@ // FIXME: Only max_return_elements == -1 is supported at present. int HandleCompletion(const char *current_line, const char *cursor, const char *last_char, int match_start_point, - int max_return_elements, StringList &matches); + int max_return_elements, StringList &matches, + StringList &descriptions); // This version just returns matches, and doesn't compute the substring. It // is here so the Help command can call it for the first argument. It uses @@ -319,7 +321,8 @@ int GetCommandNamesMatchingPartialString(const char *cmd_cstr, bool include_aliases, - StringList &matches); + StringList &matches, + StringList &descriptions); void GetHelp(CommandReturnObject &result, uint32_t types = eCommandTypesAllThem); @@ -520,7 +523,8 @@ lldb::CommandObjectSP GetCommandSP(llvm::StringRef cmd, bool include_aliases = true, bool exact = true, - StringList *matches = nullptr) const; + StringList *matches = nullptr, + StringList *descriptions = nullptr) const; private: Status PreprocessCommand(std::string &command); Index: lldb/trunk/include/lldb/Interpreter/CommandObject.h =================================================================== --- lldb/trunk/include/lldb/Interpreter/CommandObject.h +++ lldb/trunk/include/lldb/Interpreter/CommandObject.h @@ -37,8 +37,9 @@ // number added. template -int AddNamesMatchingPartialString(const std::map &in_map, - llvm::StringRef cmd_str, StringList &matches) { +int AddNamesMatchingPartialString( + const std::map &in_map, llvm::StringRef cmd_str, + StringList &matches, StringList *descriptions = nullptr) { int number_added = 0; const bool add_all = cmd_str.empty(); @@ -47,6 +48,8 @@ if (add_all || (iter->first.find(cmd_str, 0) == 0)) { ++number_added; matches.AppendString(iter->first.c_str()); + if (descriptions) + descriptions->AppendString(iter->second->GetHelp()); } } Index: lldb/trunk/include/lldb/Utility/CompletionRequest.h =================================================================== --- lldb/trunk/include/lldb/Utility/CompletionRequest.h +++ lldb/trunk/include/lldb/Utility/CompletionRequest.h @@ -11,11 +11,50 @@ #define LLDB_UTILITY_COMPLETIONREQUEST_H #include "lldb/Utility/Args.h" +#include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/StringList.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" namespace lldb_private { +class CompletionResult { + //---------------------------------------------------------- + /// A single completion and all associated data. + //---------------------------------------------------------- + struct Completion { + Completion(llvm::StringRef completion, llvm::StringRef description) + : m_completion(completion.str()), m_descripton(description.str()) {} + + std::string m_completion; + std::string m_descripton; + + /// Generates a string that uniquely identifies this completion result. + std::string GetUniqueKey() const; + }; + std::vector m_results; + + /// List of added completions so far. Used to filter out duplicates. + llvm::StringSet<> m_added_values; + +public: + void AddResult(llvm::StringRef completion, llvm::StringRef description); + + //---------------------------------------------------------- + /// Adds all collected completion matches to the given list. + /// The list will be cleared before the results are added. The number of + /// results here is guaranteed to be equal to GetNumberOfResults(). + //---------------------------------------------------------- + void GetMatches(StringList &matches) const; + + //---------------------------------------------------------- + /// Adds all collected completion descriptions to the given list. + /// The list will be cleared before the results are added. The number of + /// results here is guaranteed to be equal to GetNumberOfResults(). + //---------------------------------------------------------- + void GetDescriptions(StringList &descriptions) const; + + std::size_t GetNumberOfResults() const { return m_results.size(); } +}; //---------------------------------------------------------------------- /// @class CompletionRequest CompletionRequest.h @@ -46,13 +85,13 @@ /// completion from match_start_point, and return match_return_elements /// elements. /// - /// @param [out] matches - /// A list of matches that will be filled by the different completion - /// handlers. + /// @param [out] result + /// The CompletionResult that will be filled with the results after this + /// request has been handled. //---------------------------------------------------------- CompletionRequest(llvm::StringRef command_line, unsigned raw_cursor_pos, int match_start_point, int max_return_elements, - StringList &matches); + CompletionResult &result); llvm::StringRef GetRawLine() const { return m_command; } @@ -84,10 +123,11 @@ /// afterwards. /// /// @param match The suggested completion. - void AddCompletion(llvm::StringRef completion) { - // Add the completion if we haven't seen the same value before. - if (m_match_set.insert(completion).second) - m_matches->AppendString(completion); + /// @param match An optional description of the completion string. The + /// description will be displayed to the user alongside the completion. + void AddCompletion(llvm::StringRef completion, + llvm::StringRef description = "") { + m_result.AddResult(completion, description); } /// Adds multiple possible completion strings. @@ -100,7 +140,25 @@ AddCompletion(completions.GetStringAtIndex(i)); } - std::size_t GetNumberOfMatches() const { return m_matches->GetSize(); } + /// Adds multiple possible completion strings alongside their descriptions. + /// + /// The number of completions and descriptions must be identical. + /// + /// \param completions The list of completions. + /// \param completions The list of descriptions. + /// + /// @see AddCompletion + void AddCompletions(const StringList &completions, + const StringList &descriptions) { + lldbassert(completions.GetSize() == descriptions.GetSize()); + for (std::size_t i = 0; i < completions.GetSize(); ++i) + AddCompletion(completions.GetStringAtIndex(i), + descriptions.GetStringAtIndex(i)); + } + + std::size_t GetNumberOfMatches() const { + return m_result.GetNumberOfResults(); + } llvm::StringRef GetCursorArgument() const { return GetParsedLine().GetArgumentAtIndex(GetCursorIndex()); @@ -134,14 +192,11 @@ /// after the completion.) \bfalse otherwise. bool m_word_complete = false; - // Note: This list is kept private. This is by design to prevent that any - // completion depends on any already computed completion from another backend. - // Note: We don't own the list. It's owned by the creator of the - // CompletionRequest object. - StringList *m_matches; - - /// List of added completions so far. Used to filter out duplicates. - llvm::StringSet<> m_match_set; + /// The result this request is supposed to fill out. + /// We keep this object private to ensure that no backend can in any way + /// depend on already calculated completions (which would make debugging and + /// testing them much more complicated). + CompletionResult &m_result; }; } // namespace lldb_private Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py =================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py @@ -274,6 +274,22 @@ self.complete_from_to("watchpoint set variable foo --watch w", "watchpoint set variable foo --watch write") self.complete_from_to('watchpoint set variable foo -w read_', 'watchpoint set variable foo -w read_write') + def test_completion_description_commands(self): + """Test descriptions of top-level command completions""" + self.check_completion_with_desc("", [ + ["command", "Commands for managing custom LLDB commands."], + ["bugreport", "Commands for creating domain-specific bug reports."] + ]) + + self.check_completion_with_desc("pl", [ + ["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", []) + @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489") def test_symbol_name(self): self.build() Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py =================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py +++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py @@ -2145,6 +2145,35 @@ return match_object + def check_completion_with_desc(self, str_input, match_desc_pairs): + interp = self.dbg.GetCommandInterpreter() + match_strings = lldb.SBStringList() + description_strings = lldb.SBStringList() + num_matches = interp.HandleCompletionWithDescriptions(str_input, len(str_input), 0, -1, match_strings, description_strings) + self.assertEqual(len(description_strings), len(match_strings)) + + missing_pairs = [] + for pair in match_desc_pairs: + found_pair = False + for i in range(num_matches + 1): + match_candidate = match_strings.GetStringAtIndex(i) + description_candidate = description_strings.GetStringAtIndex(i) + if match_candidate == pair[0] and description_candidate == pair[1]: + found_pair = True + break + if not found_pair: + missing_pairs.append(pair) + + if len(missing_pairs): + error_msg = "Missing pairs:\n" + for pair in missing_pairs: + error_msg += " [" + pair[0] + ":" + pair[1] + "]\n" + error_msg += "Got the following " + str(num_matches) + " completions back:\n" + for i in range(num_matches + 1): + match_candidate = match_strings.GetStringAtIndex(i) + description_candidate = description_strings.GetStringAtIndex(i) + error_msg += "[" + match_candidate + ":" + description_candidate + "]\n" + self.assertEqual(0, len(missing_pairs), error_msg) def complete_exactly(self, str_input, patterns): self.complete_from_to(str_input, patterns, True) Index: lldb/trunk/scripts/interface/SBCommandInterpreter.i =================================================================== --- lldb/trunk/scripts/interface/SBCommandInterpreter.i +++ lldb/trunk/scripts/interface/SBCommandInterpreter.i @@ -219,6 +219,13 @@ int max_return_elements, lldb::SBStringList &matches); + int + HandleCompletionWithDescriptions (const char *current_line, + uint32_t cursor_pos, + int match_start_point, + int max_return_elements, + lldb::SBStringList &matches, + lldb::SBStringList &descriptions); bool IsActive (); Index: lldb/trunk/source/API/SBCommandInterpreter.cpp =================================================================== --- lldb/trunk/source/API/SBCommandInterpreter.cpp +++ lldb/trunk/source/API/SBCommandInterpreter.cpp @@ -269,6 +269,16 @@ int SBCommandInterpreter::HandleCompletion( const char *current_line, const char *cursor, const char *last_char, int match_start_point, int max_return_elements, SBStringList &matches) { + SBStringList dummy_descriptions; + return HandleCompletionWithDescriptions( + current_line, cursor, last_char, match_start_point, max_return_elements, + matches, dummy_descriptions); +} + +int SBCommandInterpreter::HandleCompletionWithDescriptions( + const char *current_line, const char *cursor, const char *last_char, + int match_start_point, int max_return_elements, SBStringList &matches, + SBStringList &descriptions) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API)); int num_completions = 0; @@ -296,13 +306,15 @@ match_start_point, max_return_elements); if (IsValid()) { - lldb_private::StringList lldb_matches; + lldb_private::StringList lldb_matches, lldb_descriptions; num_completions = m_opaque_ptr->HandleCompletion( current_line, cursor, last_char, match_start_point, max_return_elements, - lldb_matches); + lldb_matches, lldb_descriptions); - SBStringList temp_list(&lldb_matches); - matches.AppendList(temp_list); + SBStringList temp_matches_list(&lldb_matches); + matches.AppendList(temp_matches_list); + SBStringList temp_descriptions_list(&lldb_descriptions); + descriptions.AppendList(temp_descriptions_list); } if (log) log->Printf( @@ -312,6 +324,17 @@ return num_completions; } +int SBCommandInterpreter::HandleCompletionWithDescriptions( + const char *current_line, uint32_t cursor_pos, int match_start_point, + int max_return_elements, SBStringList &matches, + SBStringList &descriptions) { + const char *cursor = current_line + cursor_pos; + const char *last_char = current_line + strlen(current_line); + return HandleCompletionWithDescriptions( + current_line, cursor, last_char, match_start_point, max_return_elements, + matches, descriptions); +} + int SBCommandInterpreter::HandleCompletion(const char *current_line, uint32_t cursor_pos, int match_start_point, Index: lldb/trunk/source/Commands/CommandObjectMultiword.cpp =================================================================== --- lldb/trunk/source/Commands/CommandObjectMultiword.cpp +++ lldb/trunk/source/Commands/CommandObjectMultiword.cpp @@ -193,9 +193,10 @@ auto arg0 = request.GetParsedLine()[0].ref; if (request.GetCursorIndex() == 0) { - StringList new_matches; - AddNamesMatchingPartialString(m_subcommand_dict, arg0, new_matches); - request.AddCompletions(new_matches); + StringList new_matches, descriptions; + AddNamesMatchingPartialString(m_subcommand_dict, arg0, new_matches, + &descriptions); + request.AddCompletions(new_matches, descriptions); if (new_matches.GetSize() == 1 && new_matches.GetStringAtIndex(0) != nullptr && Index: lldb/trunk/source/Core/IOHandler.cpp =================================================================== --- lldb/trunk/source/Core/IOHandler.cpp +++ lldb/trunk/source/Core/IOHandler.cpp @@ -172,12 +172,10 @@ IOHandlerConfirm::~IOHandlerConfirm() = default; -int IOHandlerConfirm::IOHandlerComplete(IOHandler &io_handler, - const char *current_line, - const char *cursor, - const char *last_char, - int skip_first_n_matches, - int max_matches, StringList &matches) { +int IOHandlerConfirm::IOHandlerComplete( + IOHandler &io_handler, const char *current_line, const char *cursor, + const char *last_char, int skip_first_n_matches, int max_matches, + StringList &matches, StringList &descriptions) { if (current_line == cursor) { if (m_default_response) { matches.AppendString("y"); @@ -223,12 +221,10 @@ } } -int IOHandlerDelegate::IOHandlerComplete(IOHandler &io_handler, - const char *current_line, - const char *cursor, - const char *last_char, - int skip_first_n_matches, - int max_matches, StringList &matches) { +int IOHandlerDelegate::IOHandlerComplete( + IOHandler &io_handler, const char *current_line, const char *cursor, + const char *last_char, int skip_first_n_matches, int max_matches, + StringList &matches, StringList &descriptions) { switch (m_completion) { case Completion::None: break; @@ -236,14 +232,16 @@ case Completion::LLDBCommand: return io_handler.GetDebugger().GetCommandInterpreter().HandleCompletion( current_line, cursor, last_char, skip_first_n_matches, max_matches, - matches); - + matches, descriptions); case Completion::Expression: { + CompletionResult result; CompletionRequest request(current_line, current_line - cursor, - skip_first_n_matches, max_matches, matches); + skip_first_n_matches, max_matches, result); CommandCompletions::InvokeCommonCompletionCallbacks( io_handler.GetDebugger().GetCommandInterpreter(), CommandCompletions::eVariablePathCompletion, request, nullptr); + result.GetMatches(matches); + result.GetDescriptions(descriptions); size_t num_matches = request.GetNumberOfMatches(); if (num_matches > 0) { @@ -432,17 +430,15 @@ *editline_reader, lines, cursor_position); } -int IOHandlerEditline::AutoCompleteCallback(const char *current_line, - const char *cursor, - const char *last_char, - int skip_first_n_matches, - int max_matches, - StringList &matches, void *baton) { +int IOHandlerEditline::AutoCompleteCallback( + const char *current_line, const char *cursor, const char *last_char, + int skip_first_n_matches, int max_matches, StringList &matches, + StringList &descriptions, void *baton) { IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton; if (editline_reader) return editline_reader->m_delegate.IOHandlerComplete( *editline_reader, current_line, cursor, last_char, skip_first_n_matches, - max_matches, matches); + max_matches, matches, descriptions); return 0; } #endif Index: lldb/trunk/source/Expression/REPL.cpp =================================================================== --- lldb/trunk/source/Expression/REPL.cpp +++ lldb/trunk/source/Expression/REPL.cpp @@ -453,7 +453,7 @@ int REPL::IOHandlerComplete(IOHandler &io_handler, const char *current_line, const char *cursor, const char *last_char, int skip_first_n_matches, int max_matches, - StringList &matches) { + StringList &matches, StringList &descriptions) { matches.Clear(); llvm::StringRef line(current_line, cursor - current_line); @@ -466,7 +466,7 @@ const char *lldb_current_line = line.substr(1).data(); return debugger.GetCommandInterpreter().HandleCompletion( lldb_current_line, cursor, last_char, skip_first_n_matches, max_matches, - matches); + matches, descriptions); } // Strip spaces from the line and see if we had only spaces Index: lldb/trunk/source/Host/common/Editline.cpp =================================================================== --- lldb/trunk/source/Host/common/Editline.cpp +++ lldb/trunk/source/Host/common/Editline.cpp @@ -853,19 +853,45 @@ return CC_NEWLINE; } +//------------------------------------------------------------------------------ +/// Prints completions and their descriptions to the given file. Only the +/// completions in the interval [start, end) are printed. +//------------------------------------------------------------------------------ +static void PrintCompletion(FILE *output_file, size_t start, size_t end, + StringList &completions, StringList &descriptions) { + // This is an 'int' because of printf. + int max_len = 0; + + for (size_t i = start; i < end; i++) { + const char *completion_str = completions.GetStringAtIndex(i); + max_len = std::max((int)strlen(completion_str), max_len); + } + + for (size_t i = start; i < end; i++) { + const char *completion_str = completions.GetStringAtIndex(i); + const char *description_str = descriptions.GetStringAtIndex(i); + + fprintf(output_file, "\n\t%-*s", max_len, completion_str); + + // Print the description if we got one. + if (strlen(description_str)) + fprintf(output_file, " -- %s", description_str); + } +} + unsigned char Editline::TabCommand(int ch) { if (m_completion_callback == nullptr) return CC_ERROR; const LineInfo *line_info = el_line(m_editline); - StringList completions; + StringList completions, descriptions; int page_size = 40; const int num_completions = m_completion_callback( line_info->buffer, line_info->cursor, line_info->lastchar, 0, // Don't skip any matches (start at match zero) -1, // Get all the matches - completions, m_completion_callback_baton); + completions, descriptions, m_completion_callback_baton); if (num_completions == 0) return CC_ERROR; @@ -893,10 +919,8 @@ int num_elements = num_completions + 1; fprintf(m_output_file, "\n" ANSI_CLEAR_BELOW "Available completions:"); if (num_completions < page_size) { - for (int i = 1; i < num_elements; i++) { - completion_str = completions.GetStringAtIndex(i); - fprintf(m_output_file, "\n\t%s", completion_str); - } + PrintCompletion(m_output_file, 1, num_elements, completions, + descriptions); fprintf(m_output_file, "\n"); } else { int cur_pos = 1; @@ -906,10 +930,10 @@ int endpoint = cur_pos + page_size; if (endpoint > num_elements) endpoint = num_elements; - for (; cur_pos < endpoint; cur_pos++) { - completion_str = completions.GetStringAtIndex(cur_pos); - fprintf(m_output_file, "\n\t%s", completion_str); - } + + PrintCompletion(m_output_file, cur_pos, endpoint, completions, + descriptions); + cur_pos = endpoint; if (cur_pos >= num_elements) { fprintf(m_output_file, "\n"); Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp =================================================================== --- lldb/trunk/source/Interpreter/CommandInterpreter.cpp +++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp @@ -794,20 +794,23 @@ } int CommandInterpreter::GetCommandNamesMatchingPartialString( - const char *cmd_str, bool include_aliases, StringList &matches) { - AddNamesMatchingPartialString(m_command_dict, cmd_str, matches); + const char *cmd_str, bool include_aliases, StringList &matches, + StringList &descriptions) { + AddNamesMatchingPartialString(m_command_dict, cmd_str, matches, + &descriptions); if (include_aliases) { - AddNamesMatchingPartialString(m_alias_dict, cmd_str, matches); + AddNamesMatchingPartialString(m_alias_dict, cmd_str, matches, + &descriptions); } return matches.GetSize(); } -CommandObjectSP CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, - bool include_aliases, - bool exact, - StringList *matches) const { +CommandObjectSP +CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases, + bool exact, StringList *matches, + StringList *descriptions) const { CommandObjectSP command_sp; std::string cmd = cmd_str; @@ -848,8 +851,8 @@ // empty CommandObjectSP and the list of matches. if (HasCommands()) { - num_cmd_matches = - AddNamesMatchingPartialString(m_command_dict, cmd_str, *matches); + num_cmd_matches = AddNamesMatchingPartialString(m_command_dict, cmd_str, + *matches, descriptions); } if (num_cmd_matches == 1) { @@ -860,8 +863,8 @@ } if (include_aliases && HasAliases()) { - num_alias_matches = - AddNamesMatchingPartialString(m_alias_dict, cmd_str, *matches); + num_alias_matches = AddNamesMatchingPartialString(m_alias_dict, cmd_str, + *matches, descriptions); } if (num_alias_matches == 1) { @@ -872,8 +875,8 @@ } if (HasUserCommands()) { - num_user_matches = - AddNamesMatchingPartialString(m_user_dict, cmd_str, *matches); + num_user_matches = AddNamesMatchingPartialString(m_user_dict, cmd_str, + *matches, descriptions); } if (num_user_matches == 1) { @@ -898,6 +901,8 @@ } } else if (matches && command_sp) { matches->AppendString(cmd_str); + if (descriptions) + descriptions->AppendString(command_sp->GetHelp()); } return command_sp; @@ -997,10 +1002,12 @@ return ret_val; } -CommandObject *CommandInterpreter::GetCommandObject(llvm::StringRef cmd_str, - StringList *matches) const { +CommandObject * +CommandInterpreter::GetCommandObject(llvm::StringRef cmd_str, + StringList *matches, + StringList *descriptions) const { CommandObject *command_obj = - GetCommandSP(cmd_str, false, true, matches).get(); + GetCommandSP(cmd_str, false, true, matches, descriptions).get(); // If we didn't find an exact match to the command string in the commands, // look in the aliases. @@ -1008,7 +1015,7 @@ if (command_obj) return command_obj; - command_obj = GetCommandSP(cmd_str, true, true, matches).get(); + command_obj = GetCommandSP(cmd_str, true, true, matches, descriptions).get(); if (command_obj) return command_obj; @@ -1023,10 +1030,12 @@ if (command_obj) { if (matches) matches->AppendString(command_obj->GetCommandName()); + if (descriptions) + descriptions->AppendString(command_obj->GetHelp()); return command_obj; } - return GetCommandSP(cmd_str, true, false, matches).get(); + return GetCommandSP(cmd_str, true, false, matches, descriptions).get(); } bool CommandInterpreter::CommandExists(llvm::StringRef cmd) const { @@ -1712,16 +1721,17 @@ if (request.GetCursorIndex() == -1) { // We got nothing on the command line, so return the list of commands bool include_aliases = true; - StringList new_matches; - num_command_matches = - GetCommandNamesMatchingPartialString("", include_aliases, new_matches); - request.AddCompletions(new_matches); + StringList new_matches, descriptions; + num_command_matches = GetCommandNamesMatchingPartialString( + "", include_aliases, new_matches, descriptions); + request.AddCompletions(new_matches, descriptions); } else if (request.GetCursorIndex() == 0) { // The cursor is in the first argument, so just do a lookup in the // dictionary. - StringList new_matches; - CommandObject *cmd_obj = GetCommandObject( - request.GetParsedLine().GetArgumentAtIndex(0), &new_matches); + StringList new_matches, new_descriptions; + CommandObject *cmd_obj = + GetCommandObject(request.GetParsedLine().GetArgumentAtIndex(0), + &new_matches, &new_descriptions); if (num_command_matches == 1 && cmd_obj && cmd_obj->IsMultiwordObject() && new_matches.GetStringAtIndex(0) != nullptr && @@ -1733,12 +1743,13 @@ look_for_subcommand = true; num_command_matches = 0; new_matches.DeleteStringAtIndex(0); + new_descriptions.DeleteStringAtIndex(0); request.GetParsedLine().AppendArgument(llvm::StringRef()); request.SetCursorIndex(request.GetCursorIndex() + 1); request.SetCursorCharPosition(0); } } - request.AddCompletions(new_matches); + request.AddCompletions(new_matches, new_descriptions); num_command_matches = request.GetNumberOfMatches(); } @@ -1762,12 +1773,13 @@ int CommandInterpreter::HandleCompletion( const char *current_line, const char *cursor, const char *last_char, - int match_start_point, int max_return_elements, StringList &matches) { + int match_start_point, int max_return_elements, StringList &matches, + StringList &descriptions) { llvm::StringRef command_line(current_line, last_char - current_line); + CompletionResult result; CompletionRequest request(command_line, cursor - current_line, - match_start_point, max_return_elements, matches); - + match_start_point, max_return_elements, result); // Don't complete comments, and if the line we are completing is just the // history repeat character, substitute the appropriate history line. const char *first_arg = request.GetParsedLine().GetArgumentAtIndex(0); @@ -1777,6 +1789,7 @@ else if (first_arg[0] == CommandHistory::g_repeat_char) { if (auto hist_str = m_command_history.FindString(first_arg)) { matches.InsertStringAtIndex(0, *hist_str); + descriptions.InsertStringAtIndex(0, "Previous command history event"); return -2; } else return 0; @@ -1787,6 +1800,8 @@ lldbassert(max_return_elements == -1); int num_command_matches = HandleCompletionMatches(request); + result.GetMatches(matches); + result.GetDescriptions(descriptions); if (num_command_matches <= 0) return num_command_matches; @@ -1794,6 +1809,7 @@ if (request.GetParsedLine().GetArgumentCount() == 0) { // If we got an empty string, insert nothing. matches.InsertStringAtIndex(0, ""); + descriptions.InsertStringAtIndex(0, ""); } else { // Now figure out if there is a common substring, and if so put that in // element 0, otherwise put an empty string in element 0. @@ -1815,6 +1831,7 @@ common_prefix.push_back(' '); } matches.InsertStringAtIndex(0, common_prefix.c_str()); + descriptions.InsertStringAtIndex(0, ""); } return num_command_matches; } Index: lldb/trunk/source/Utility/CompletionRequest.cpp =================================================================== --- lldb/trunk/source/Utility/CompletionRequest.cpp +++ lldb/trunk/source/Utility/CompletionRequest.cpp @@ -16,11 +16,10 @@ unsigned raw_cursor_pos, int match_start_point, int max_return_elements, - StringList &matches) + CompletionResult &result) : m_command(command_line), m_raw_cursor_pos(raw_cursor_pos), m_match_start_point(match_start_point), - m_max_return_elements(max_return_elements), m_matches(&matches) { - matches.Clear(); + m_max_return_elements(max_return_elements), m_result(result) { // We parse the argument up to the cursor, so the last argument in // parsed_line is the one containing the cursor, and the cursor is after the @@ -36,8 +35,6 @@ m_cursor_char_position = strlen(m_partial_parsed_line.GetArgumentAtIndex(m_cursor_index)); - matches.Clear(); - const char *cursor = command_line.data() + raw_cursor_pos; if (raw_cursor_pos > 0 && cursor[-1] == ' ') { // We are just after a space. If we are in an argument, then we will @@ -58,3 +55,39 @@ } } } + +std::string CompletionResult::Completion::GetUniqueKey() const { + + // We build a unique key for this pair of completion:description. We + // prefix the key with the length of the completion string. This prevents + // that we could get any collisions from completions pairs such as these: + // "foo:", "bar" would be "foo:bar", but will now be: "4foo:bar" + // "foo", ":bar" would be "foo:bar", but will now be: "3foo:bar" + + std::string result; + result.append(std::to_string(m_completion.size())); + result.append(m_completion); + result.append(m_descripton); + return result; +} + +void CompletionResult::AddResult(llvm::StringRef completion, + llvm::StringRef description) { + Completion r(completion, description); + + // Add the completion if we haven't seen the same value before. + if (m_added_values.insert(r.GetUniqueKey()).second) + m_results.push_back(r); +} + +void CompletionResult::GetMatches(StringList &matches) const { + matches.Clear(); + for (const Completion &completion : m_results) + matches.AppendString(completion.m_completion); +} + +void CompletionResult::GetDescriptions(StringList &descriptions) const { + descriptions.Clear(); + for (const Completion &completion : m_results) + descriptions.AppendString(completion.m_descripton); +} Index: lldb/trunk/unittests/Utility/CompletionRequestTest.cpp =================================================================== --- lldb/trunk/unittests/Utility/CompletionRequestTest.cpp +++ lldb/trunk/unittests/Utility/CompletionRequestTest.cpp @@ -20,9 +20,11 @@ const int match_start = 2345; const int match_max_return = 12345; StringList matches; + CompletionResult result; CompletionRequest request(command, cursor_pos, match_start, match_max_return, - matches); + result); + result.GetMatches(matches); EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str()); EXPECT_EQ(request.GetRawCursorPos(), cursor_pos); @@ -41,29 +43,39 @@ const unsigned cursor_pos = 3; StringList matches; - CompletionRequest request(command, cursor_pos, 0, 0, matches); + CompletionResult result; + CompletionRequest request(command, cursor_pos, 0, 0, result); + result.GetMatches(matches); EXPECT_EQ(0U, request.GetNumberOfMatches()); // Add foo twice request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(1U, request.GetNumberOfMatches()); EXPECT_EQ(1U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(1U, request.GetNumberOfMatches()); EXPECT_EQ(1U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); // Add bar twice request.AddCompletion("bar"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); request.AddCompletion("bar"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); @@ -71,6 +83,8 @@ // Add foo again. request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); @@ -78,6 +92,8 @@ // Add something with an existing prefix request.AddCompletion("foobar"); + result.GetMatches(matches); + EXPECT_EQ(3U, request.GetNumberOfMatches()); EXPECT_EQ(3U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); @@ -85,12 +101,89 @@ EXPECT_STREQ("foobar", matches.GetStringAtIndex(2)); } +TEST(CompletionRequest, DuplicateFilteringWithComments) { + std::string command = "a bad c"; + const unsigned cursor_pos = 3; + StringList matches, descriptions; + + CompletionResult result; + CompletionRequest request(command, cursor_pos, 0, 0, result); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(0U, request.GetNumberOfMatches()); + + // Add foo twice with same comment + request.AddCompletion("foo", "comment"); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(1U, request.GetNumberOfMatches()); + EXPECT_EQ(1U, matches.GetSize()); + EXPECT_EQ(1U, descriptions.GetSize()); + EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(0)); + + request.AddCompletion("foo", "comment"); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(1U, request.GetNumberOfMatches()); + EXPECT_EQ(1U, matches.GetSize()); + EXPECT_EQ(1U, descriptions.GetSize()); + EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(0)); + + // Add bar twice with different comments + request.AddCompletion("bar", "comment"); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(2U, request.GetNumberOfMatches()); + EXPECT_EQ(2U, matches.GetSize()); + EXPECT_EQ(2U, descriptions.GetSize()); + EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); + EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); + + request.AddCompletion("bar", "another comment"); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(3U, request.GetNumberOfMatches()); + EXPECT_EQ(3U, matches.GetSize()); + EXPECT_EQ(3U, descriptions.GetSize()); + EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(0)); + EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(1)); + EXPECT_STREQ("bar", matches.GetStringAtIndex(2)); + EXPECT_STREQ("another comment", descriptions.GetStringAtIndex(2)); + + // Add foo again with no comment + request.AddCompletion("foo"); + result.GetMatches(matches); + result.GetDescriptions(descriptions); + + EXPECT_EQ(4U, request.GetNumberOfMatches()); + EXPECT_EQ(4U, matches.GetSize()); + EXPECT_EQ(4U, descriptions.GetSize()); + EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(0)); + EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); + EXPECT_STREQ("comment", descriptions.GetStringAtIndex(1)); + EXPECT_STREQ("bar", matches.GetStringAtIndex(2)); + EXPECT_STREQ("another comment", descriptions.GetStringAtIndex(2)); + EXPECT_STREQ("foo", matches.GetStringAtIndex(3)); + EXPECT_STREQ("", descriptions.GetStringAtIndex(3)); +} + TEST(CompletionRequest, TestCompletionOwnership) { std::string command = "a bad c"; const unsigned cursor_pos = 3; StringList matches; - CompletionRequest request(command, cursor_pos, 0, 0, matches); + CompletionResult result; + CompletionRequest request(command, cursor_pos, 0, 0, result); std::string Temporary = "bar"; request.AddCompletion(Temporary); @@ -98,6 +191,7 @@ // shouldn't influence anything. Temporary[0] = 'f'; + result.GetMatches(matches); EXPECT_EQ(1U, request.GetNumberOfMatches()); EXPECT_STREQ("bar", matches.GetStringAtIndex(0)); }