From cba3dd190b7d8cdcc5a2a0dbf25a0aa2d754df90 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 31 Mar 2018 00:49:42 -0400 Subject: [PATCH 1/3] Removed unnecessary sorting and duplicate removal from the completers since all results from these functions are returned to complete() which already does these things. These changes also provide better examples of what is required to write a completer and what isn't. --- cmd2.py | 92 +++++++++++++++------------------------- tests/test_completion.py | 28 +++++++----- 2 files changed, 53 insertions(+), 67 deletions(-) diff --git a/cmd2.py b/cmd2.py index dbcbae147..093abc01b 100755 --- a/cmd2.py +++ b/cmd2.py @@ -1401,17 +1401,9 @@ def basic_complete(text, line, begidx, endidx, match_against): :param begidx: int - the beginning index of the prefix text :param endidx: int - the ending index of the prefix text :param match_against: Collection - the list being matched against - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ - # Make sure we were given a Collection with items to match against - if not isinstance(match_against, Collection) or len(match_against) == 0: - return [] - - # Perform matching and eliminate duplicates - matches = [cur_match for cur_match in set(match_against) if cur_match.startswith(text)] - - matches.sort() - return matches + return [cur_match for cur_match in match_against if cur_match.startswith(text)] def delimiter_complete(self, text, line, begidx, endidx, match_against, delimiter): """ @@ -1444,7 +1436,7 @@ def delimiter_complete(self, text, line, begidx, endidx, match_against, delimite :param endidx: int - the ending index of the prefix text :param match_against: Collection - the list being matched against :param delimiter: str - what delimits each portion of the matches (ex: paths are delimited by a slash) - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ matches = self.basic_complete(text, line, begidx, endidx, match_against) @@ -1486,7 +1478,7 @@ def flag_based_complete(self, text, line, begidx, endidx, flag_dict, all_else=No 2. function that performs tab completion (ex: path_complete) :param all_else: Collection or function - an optional parameter for tab completing any token that isn't preceded by a flag in flag_dict - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ # Get all tokens through the one being completed tokens, _ = self.tokens_for_completion(line, begidx, endidx) @@ -1502,14 +1494,13 @@ def flag_based_complete(self, text, line, begidx, endidx, flag_dict, all_else=No if flag in flag_dict: match_against = flag_dict[flag] - # Perform tab completion using a Collection. These matches are already sorted. + # Perform tab completion using a Collection if isinstance(match_against, Collection): completions_matches = self.basic_complete(text, line, begidx, endidx, match_against) # Perform tab completion using a function elif callable(match_against): completions_matches = match_against(text, line, begidx, endidx) - completions_matches.sort() return completions_matches @@ -1528,7 +1519,7 @@ def index_based_complete(self, text, line, begidx, endidx, index_dict, all_else= 2. function that performs tab completion (ex: path_complete) :param all_else: Collection or function - an optional parameter for tab completing any token that isn't at an index in index_dict - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ # Get all tokens through the one being completed tokens, _ = self.tokens_for_completion(line, begidx, endidx) @@ -1546,14 +1537,13 @@ def index_based_complete(self, text, line, begidx, endidx, index_dict, all_else= else: match_against = all_else - # Perform tab completion using a Collection. These matches are already sorted. + # Perform tab completion using a Collection if isinstance(match_against, Collection): matches = self.basic_complete(text, line, begidx, endidx, match_against) # Perform tab completion using a function elif callable(match_against): matches = match_against(text, line, begidx, endidx) - matches.sort() return matches @@ -1567,7 +1557,7 @@ def path_complete(self, text, line, begidx, endidx, dir_exe_only=False, dir_only :param endidx: int - the ending index of the prefix text :param dir_exe_only: bool - only return directories and executables, not non-executable files :param dir_only: bool - only return directories - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ # Determine if a trailing separator should be appended to directory completions add_trailing_sep_if_dir = False @@ -1655,7 +1645,6 @@ def path_complete(self, text, line, begidx, endidx, dir_exe_only=False, dir_only if tilde_expanded: matches = [cur_path.replace(user_path, '~', 1) for cur_path in matches] - matches.sort() return matches @staticmethod @@ -1663,7 +1652,7 @@ def get_exes_in_path(starts_with): """ Returns names of executables in a user's path :param starts_with: str - what the exes should start with. leave blank for all exes in path. - :return: List[str] - a sorted list of matching exe names + :return: List[str] - a list of matching exe names """ # Purposely don't match any executable containing wildcards wildcards = ['*', '?'] @@ -1685,9 +1674,7 @@ def get_exes_in_path(starts_with): for match in matches: exes_set.add(os.path.basename(match)) - exes_list = list(exes_set) - exes_list.sort() - return exes_list + return list(exes_set) def shell_cmd_complete(self, text, line, begidx, endidx, complete_blank=False): """Performs completion of executables either in a user's path or a given path @@ -1698,7 +1685,7 @@ def shell_cmd_complete(self, text, line, begidx, endidx, complete_blank=False): :param complete_blank: bool - If True, then a blank will complete all shell commands in a user's path If False, then no completion is performed Defaults to False to match Bash shell behavior - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ # Don't tab complete anything if no shell command has been started if not complete_blank and len(text) == 0: @@ -1724,7 +1711,7 @@ def _redirect_complete(self, text, line, begidx, endidx, compfunc): :param endidx: int - the ending index of the prefix text :param compfunc: Callable - the completer function for the current command this will be called if we aren't completing for redirection - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ if self.allow_redirection: @@ -1794,11 +1781,6 @@ def _display_matches_gnu_readline(self, substitution, matches, longest_match_len else: matches_to_display = matches - # Eliminate duplicates and sort - matches_to_display_set = set(matches_to_display) - matches_to_display = list(matches_to_display_set) - matches_to_display.sort() - # We will use readline's display function (rl_display_match_list()), so we # need to encode our string as bytes to place in a C array. if six.PY3: @@ -1845,11 +1827,6 @@ def _display_matches_pyreadline(self, matches): else: matches_to_display = matches - # Eliminate duplicates and sort - matches_to_display_set = set(matches_to_display) - matches_to_display = list(matches_to_display_set) - matches_to_display.sort() - # Display the matches orig_pyreadline_display(matches_to_display) @@ -2121,9 +2098,9 @@ def complete(self, text, state): else: # Complete token against aliases and command names - alias_names = set(self.aliases.keys()) - visible_commands = set(self.get_visible_commands()) - strs_to_match = list(alias_names | visible_commands) + alias_names = list(self.aliases.keys()) + visible_commands = self.get_visible_commands() + strs_to_match = alias_names + visible_commands self.completion_matches = self.basic_complete(text, line, begidx, endidx, strs_to_match) # Eliminate duplicates and sort @@ -2131,6 +2108,10 @@ def complete(self, text, state): self.completion_matches = list(matches_set) self.completion_matches.sort() + display_matches_set = set(self.display_matches) + self.display_matches = list(display_matches_set) + self.display_matches.sort() + # Handle single result if len(self.completion_matches) == 1: str_to_append = '' @@ -2152,19 +2133,14 @@ def complete(self, text, state): def get_all_commands(self): """ - Returns a sorted list of all commands - Any duplicates have been removed as well + Returns a list of all commands """ - commands = [cur_name[3:] for cur_name in set(self.get_names()) if cur_name.startswith('do_')] - commands.sort() - return commands + return [cur_name[3:] for cur_name in self.get_names() if cur_name.startswith('do_')] def get_visible_commands(self): """ - Returns a sorted list of commands that have not been hidden - Any duplicates have been removed as well + Returns a list of commands that have not been hidden """ - # This list is already sorted and has no duplicates commands = self.get_all_commands() # Remove the hidden commands @@ -2175,13 +2151,13 @@ def get_visible_commands(self): return commands def get_help_topics(self): - """ Returns a sorted list of help topics with all duplicates removed """ - return [name[5:] for name in set(self.get_names()) if name.startswith('help_')] + """ Returns a list of help topics """ + return [name[5:] for name in self.get_names() if name.startswith('help_')] def complete_help(self, text, line, begidx, endidx): """ Override of parent class method to handle tab completing subcommands and not showing hidden commands - Returns a sorted list of possible tab completions + Returns a list of possible tab completions """ # The command is the token at index 1 in the command line @@ -2204,9 +2180,9 @@ def complete_help(self, text, line, begidx, endidx): if index == cmd_index: # Complete token against topics and visible commands - topics = set(self.get_help_topics()) - visible_commands = set(self.get_visible_commands()) - strs_to_match = list(topics | visible_commands) + topics = self.get_help_topics() + visible_commands = self.get_visible_commands() + strs_to_match = topics + visible_commands matches = self.basic_complete(text, line, begidx, endidx, strs_to_match) # Check if we are completing a subcommand @@ -2871,12 +2847,14 @@ def _help_menu(self): """ # Get a sorted list of help topics help_topics = self.get_help_topics() - - cmds_doc = [] - cmds_undoc = [] + help_topics.sort() # Get a sorted list of visible command names visible_commands = self.get_visible_commands() + visible_commands.sort() + + cmds_doc = [] + cmds_undoc = [] for command in visible_commands: if command in help_topics: @@ -3067,7 +3045,7 @@ def complete_shell(self, text, line, begidx, endidx): :param line: str - the current input line with leading whitespace removed :param begidx: int - the beginning index of the prefix text :param endidx: int - the ending index of the prefix text - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ index_dict = {1: self.shell_cmd_complete} return self.index_based_complete(text, line, begidx, endidx, index_dict, self.path_complete) @@ -3102,7 +3080,7 @@ def cmd_with_subs_completer(self, text, line, begidx, endidx): :param line: str - the current input line with leading whitespace removed :param begidx: int - the beginning index of the prefix text :param endidx: int - the ending index of the prefix text - :return: List[str] - a sorted list of possible tab completions + :return: List[str] - a list of possible tab completions """ # The command is the token at index 0 in the command line cmd_index = 0 diff --git a/tests/test_completion.py b/tests/test_completion.py index e779e44ba..69404aa73 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -73,7 +73,7 @@ def complete_tester(text, line, begidx, endidx, app): :param app: the cmd2 app that will run completions :return: The first matched string or None if there are no matches Matches are stored in app.completion_matches - These matches have been sorted by complete() + These matches also have been sorted by complete() """ def get_line(): return line @@ -116,7 +116,7 @@ def test_complete_empty_arg(cmd2_app): endidx = len(line) begidx = endidx - len(text) - expected = cmd2_app.complete_help(text, line, begidx, endidx) + expected = sorted(cmd2_app.complete_help(text, line, begidx, endidx)) first_match = complete_tester(text, line, begidx, endidx, cmd2_app) assert first_match is not None and \ @@ -167,7 +167,8 @@ def test_cmd2_help_completion_multiple(cmd2_app): endidx = len(line) begidx = endidx - len(text) - assert cmd2_app.complete_help(text, line, begidx, endidx) == ['help', 'history'] + matches = sorted(cmd2_app.complete_help(text, line, begidx, endidx)) + assert matches == ['help', 'history'] def test_cmd2_help_completion_nomatch(cmd2_app): text = 'fakecommand' @@ -269,8 +270,9 @@ def test_path_completion_multiple(cmd2_app, request): endidx = len(line) begidx = endidx - len(text) + matches = sorted(cmd2_app.path_complete(text, line, begidx, endidx)) expected = [text + 'cript.py', text + 'cript.txt', text + 'cripts' + os.path.sep] - assert expected == cmd2_app.path_complete(text, line, begidx, endidx) + assert matches == expected def test_path_completion_nomatch(cmd2_app, request): test_dir = os.path.dirname(request.module.__file__) @@ -410,7 +412,8 @@ def test_basic_completion_multiple(cmd2_app): endidx = len(line) begidx = endidx - len(text) - assert cmd2_app.basic_complete(text, line, begidx, endidx, food_item_strs) == sorted(food_item_strs) + matches = sorted(cmd2_app.basic_complete(text, line, begidx, endidx, food_item_strs)) + assert matches == sorted(food_item_strs) def test_basic_completion_nomatch(cmd2_app): text = 'q' @@ -449,7 +452,8 @@ def test_flag_based_completion_multiple(cmd2_app): endidx = len(line) begidx = endidx - len(text) - assert cmd2_app.flag_based_complete(text, line, begidx, endidx, flag_dict) == sorted(food_item_strs) + matches = sorted(cmd2_app.flag_based_complete(text, line, begidx, endidx, flag_dict)) + assert matches == sorted(food_item_strs) def test_flag_based_completion_nomatch(cmd2_app): text = 'q' @@ -499,7 +503,8 @@ def test_index_based_completion_multiple(cmd2_app): endidx = len(line) begidx = endidx - len(text) - assert cmd2_app.index_based_complete(text, line, begidx, endidx, index_dict) == sorted(sport_item_strs) + matches = sorted(cmd2_app.index_based_complete(text, line, begidx, endidx, index_dict)) + assert matches == sorted(sport_item_strs) def test_index_based_completion_nomatch(cmd2_app): text = 'q' @@ -744,7 +749,8 @@ def test_cmd2_help_subcommand_completion_multiple(sc_app): endidx = len(line) begidx = endidx - len(text) - assert sc_app.complete_help(text, line, begidx, endidx) == ['bar', 'foo', 'sport'] + matches = sorted(sc_app.complete_help(text, line, begidx, endidx)) + assert matches == ['bar', 'foo', 'sport'] def test_cmd2_help_subcommand_completion_nomatch(sc_app): @@ -899,7 +905,8 @@ def test_cmd2_help_submenu_completion_multiple(sb_app): endidx = len(line) begidx = endidx - len(text) - assert sb_app.complete_help(text, line, begidx, endidx) == ['py', 'pyscript'] + matches = sorted(sb_app.complete_help(text, line, begidx, endidx)) + assert matches == ['py', 'pyscript'] def test_cmd2_help_submenu_completion_nomatch(sb_app): @@ -916,4 +923,5 @@ def test_cmd2_help_submenu_completion_subcommands(sb_app): endidx = len(line) begidx = endidx - len(text) - assert sb_app.complete_help(text, line, begidx, endidx) == ['py', 'pyscript'] + matches = sorted(sb_app.complete_help(text, line, begidx, endidx)) + assert matches == ['py', 'pyscript'] From fd546ec1a4e0927cfc09b1b778456f26054e629f Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 31 Mar 2018 02:58:30 -0400 Subject: [PATCH 2/3] Sorting matches earlier --- cmd2.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/cmd2.py b/cmd2.py index 093abc01b..d2111dff9 100755 --- a/cmd2.py +++ b/cmd2.py @@ -2065,6 +2065,13 @@ def complete(self, text, state): if len(self.completion_matches) > 0: + # Eliminate duplicates + matches_set = set(self.completion_matches) + self.completion_matches = list(matches_set) + + display_matches_set = set(self.display_matches) + self.display_matches = list(display_matches_set) + # Get the token being completed as it appears on the command line raw_completion_token = raw_tokens[-1] @@ -2103,15 +2110,6 @@ def complete(self, text, state): strs_to_match = alias_names + visible_commands self.completion_matches = self.basic_complete(text, line, begidx, endidx, strs_to_match) - # Eliminate duplicates and sort - matches_set = set(self.completion_matches) - self.completion_matches = list(matches_set) - self.completion_matches.sort() - - display_matches_set = set(self.display_matches) - self.display_matches = list(display_matches_set) - self.display_matches.sort() - # Handle single result if len(self.completion_matches) == 1: str_to_append = '' @@ -2126,6 +2124,11 @@ def complete(self, text, state): self.completion_matches[0] += str_to_append + # Otherwise sort matches + elif len(self.completion_matches) > 0: + self.completion_matches.sort() + self.display_matches.sort() + try: return self.completion_matches[state] except IndexError: From a6ed2bcf69744975d4f54c8ae3eee8aec322fb20 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 31 Mar 2018 03:09:39 -0400 Subject: [PATCH 3/3] Forgot that topic and command names can overlap --- cmd2.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd2.py b/cmd2.py index d2111dff9..4453ea152 100755 --- a/cmd2.py +++ b/cmd2.py @@ -2183,9 +2183,9 @@ def complete_help(self, text, line, begidx, endidx): if index == cmd_index: # Complete token against topics and visible commands - topics = self.get_help_topics() - visible_commands = self.get_visible_commands() - strs_to_match = topics + visible_commands + topics = set(self.get_help_topics()) + visible_commands = set(self.get_visible_commands()) + strs_to_match = list(topics | visible_commands) matches = self.basic_complete(text, line, begidx, endidx, strs_to_match) # Check if we are completing a subcommand