Merge pull request #993 from python-cmd2/completion_hints

Completion hint enhancements

8 files changed, 131 insertions, 55 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fe9c1ca6..3ac4ead0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+## 1.4.0 (TBD, 2020)
+* Enhancements
+    * Added user-settable option called `always_show_hint`. If True, then tab completion hints will always
+    display even when tab completion suggestions print. Arguments whose help or hint text is suppressed will
+    not display hints even when this setting is True.
+* Bug Fixes
+    * Fixed issue where flag names weren't always sorted correctly in argparse tab completion
+
 ## 1.3.9 (September 03, 2020)
 * Breaking Changes
     * `CommandSet.on_unregister()` is now called as first step in unregistering a `CommandSet` and not
diff --git a/README.md b/README.md
index ca03cc47..8bb8b930 100755
--- a/README.md
+++ b/README.md
@@ -322,6 +322,7 @@ example/transcript_regex.txt:
 # regexes on prompts just make the trailing space obvious
 (Cmd) set
 allow_style: '/(Terminal|Always|Never)/'
+always_show_hint: False
 debug: False
 echo: False
 editor: /.*?/
diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py
index 0efaebe9..04e18d4a 100644
--- a/cmd2/argparse_completer.py
+++ b/cmd2/argparse_completer.py
@@ -35,6 +35,22 @@ DEFAULT_DESCRIPTIVE_HEADER = 'Description'
 ARG_TOKENS = 'arg_tokens'
 
 
+# noinspection PyProtectedMember
+def _build_hint(parser: argparse.ArgumentParser, arg_action: argparse.Action) -> str:
+    """Build tab completion hint for a given argument"""
+    # Check if hinting is disabled for this argument
+    suppress_hint = getattr(arg_action, ATTR_SUPPRESS_TAB_HINT, False)
+    if suppress_hint or arg_action.help == argparse.SUPPRESS:
+        return ''
+    else:
+        # Use the parser's help formatter to display just this action's help text
+        formatter = parser._get_formatter()
+        formatter.start_section("Hint")
+        formatter.add_argument(arg_action)
+        formatter.end_section()
+        return formatter.format_help()
+
+
 def _single_prefix_char(token: str, parser: argparse.ArgumentParser) -> bool:
     """Returns if a token is just a single flag prefix character"""
     return len(token) == 1 and token[0] in parser.prefix_chars
@@ -115,7 +131,6 @@ class _UnfinishedFlagError(CompletionError):
         super().__init__(error)
 
 
-# noinspection PyProtectedMember
 class _NoResultsError(CompletionError):
     def __init__(self, parser: argparse.ArgumentParser, arg_action: argparse.Action) -> None:
         """
@@ -124,19 +139,8 @@ class _NoResultsError(CompletionError):
         :param parser: ArgumentParser instance which owns the action being tab completed
         :param arg_action: action being tab completed
         """
-        # Check if hinting is disabled
-        suppress_hint = getattr(arg_action, ATTR_SUPPRESS_TAB_HINT, False)
-        if suppress_hint or arg_action.help == argparse.SUPPRESS:
-            hint_str = ''
-        else:
-            # Use the parser's help formatter to print just this action's help text
-            formatter = parser._get_formatter()
-            formatter.start_section("Hint")
-            formatter.add_argument(arg_action)
-            formatter.end_section()
-            hint_str = formatter.format_help()
         # Set apply_style to False because we don't want hints to look like errors
-        super().__init__(hint_str, apply_style=False)
+        super().__init__(_build_hint(parser, arg_action), apply_style=False)
 
 
 # noinspection PyProtectedMember
@@ -411,6 +415,7 @@ class ArgparseCompleter:
 
             # If we have results, then return them
             if completion_results:
+                self._cmd2_app.completion_hint = _build_hint(self._parser, flag_arg_state.action)
                 return completion_results
 
             # Otherwise, print a hint if the flag isn't finished or text isn't possibly the start of a flag
@@ -432,6 +437,7 @@ class ArgparseCompleter:
 
             # If we have results, then return them
             if completion_results:
+                self._cmd2_app.completion_hint = _build_hint(self._parser, pos_arg_state.action)
                 return completion_results
 
             # Otherwise, print a hint if text isn't possibly the start of a flag
@@ -566,12 +572,21 @@ class ArgparseCompleter:
         """
         # Check if the arg provides choices to the user
         if arg_state.action.choices is not None:
-            arg_choices = arg_state.action.choices
+            arg_choices = list(arg_state.action.choices)
+            if not arg_choices:
+                return []
+
+            # If these choices are numbers, then sort them now
+            if all(isinstance(x, numbers.Number) for x in arg_choices):
+                arg_choices.sort()
+                self._cmd2_app.matches_sorted = True
+
+            # Since choices can be various types, convert them all to strings
+            arg_choices = [str(x) for x in arg_choices]
         else:
             arg_choices = getattr(arg_state.action, ATTR_CHOICES_CALLABLE, None)
-
-        if arg_choices is None:
-            return []
+            if arg_choices is None:
+                return []
 
         # If we are going to call a completer/choices function, then set up the common arguments
         args = []
@@ -612,19 +627,6 @@ class ArgparseCompleter:
             if isinstance(arg_choices, ChoicesCallable) and not arg_choices.is_completer:
                 arg_choices = arg_choices.to_call(*args, **kwargs)
 
-            # Since arg_choices can be any iterable type, convert to a list
-            arg_choices = list(arg_choices)
-
-            # If these choices are numbers, and have not yet been sorted, then sort them now
-            if not self._cmd2_app.matches_sorted and all(isinstance(x, numbers.Number) for x in arg_choices):
-                arg_choices.sort()
-                self._cmd2_app.matches_sorted = True
-
-            # Since choices can be various types like int, we must convert them to strings
-            for index, choice in enumerate(arg_choices):
-                if not isinstance(choice, str):
-                    arg_choices[index] = str(choice)
-
             # Filter out arguments we already used
             used_values = consumed_arg_values.get(arg_state.action.dest, [])
             arg_choices = [choice for choice in arg_choices if choice not in used_values]
@@ -632,4 +634,10 @@ class ArgparseCompleter:
             # Do tab completion on the choices
             results = basic_complete(text, line, begidx, endidx, arg_choices)
 
+        if not results:
+            # Reset the value for matches_sorted. This is because completion of flag names
+            # may still be attempted after we return and they haven't been sorted yet.
+            self._cmd2_app.matches_sorted = False
+            return []
+
         return self._format_completions(arg_state, results)
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index f3a2d88d..cde3892b 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -208,6 +208,7 @@ class Cmd(cmd.Cmd):
         self.allow_redirection = allow_redirection  # Security setting to prevent redirection of stdout
 
         # Attributes which ARE dynamically settable via the set command at runtime
+        self.always_show_hint = False
         self.debug = False
         self.echo = False
         self.editor = Cmd.DEFAULT_EDITOR
@@ -375,17 +376,21 @@ class Cmd(cmd.Cmd):
         # will be added if there is an unmatched opening quote
         self.allow_closing_quote = True
 
-        # An optional header that prints above the tab completion suggestions
+        # An optional hint which prints above tab completion suggestions
+        self.completion_hint = ''
+
+        # Header which prints above CompletionItem tables
         self.completion_header = ''
 
         # Used by complete() for readline tab completion
         self.completion_matches = []
 
-        # Use this list if you are completing strings that contain a common delimiter and you only want to
-        # display the final portion of the matches as the tab completion suggestions. The full matches
-        # still must be returned from your completer function. For an example, look at path_complete()
-        # which uses this to show only the basename of paths as the suggestions. delimiter_complete() also
-        # populates this list.
+        # Use this list if you need to display tab completion suggestions that are different than the actual text
+        # of the matches. For instance, if you are completing strings that contain a common delimiter and you only
+        # want to display the final portion of the matches as the tab completion suggestions. The full matches
+        # still must be returned from your completer function. For an example, look at path_complete() which
+        # uses this to show only the basename of paths as the suggestions. delimiter_complete() also populates
+        # this list.
         self.display_matches = []
 
         # Used by functions like path_complete() and delimiter_complete() to properly
@@ -788,6 +793,8 @@ class Cmd(cmd.Cmd):
                                                         ansi.STYLE_NEVER),
                                    choices=[ansi.STYLE_TERMINAL, ansi.STYLE_ALWAYS, ansi.STYLE_NEVER]))
 
+        self.add_settable(Settable('always_show_hint', bool,
+                                   'Display tab completion hint even when completion suggestions print'))
         self.add_settable(Settable('debug', bool, "Show full traceback on exception"))
         self.add_settable(Settable('echo', bool, "Echo command issued into output"))
         self.add_settable(Settable('editor', str, "Program used by 'edit'"))
@@ -984,6 +991,7 @@ class Cmd(cmd.Cmd):
         """
         self.allow_appended_space = True
         self.allow_closing_quote = True
+        self.completion_hint = ''
         self.completion_header = ''
         self.completion_matches = []
         self.display_matches = []
@@ -1479,6 +1487,22 @@ class Cmd(cmd.Cmd):
 
         return [cur_match + padding for cur_match in matches_to_display], len(padding)
 
+    def _build_completion_metadata_string(self) -> str:  # pragma: no cover
+        """Build completion metadata string which can contain a hint and CompletionItem table header"""
+        metadata = ''
+
+        # Add hint if one exists and we are supposed to display it
+        if self.always_show_hint and self.completion_hint:
+            metadata += '\n' + self.completion_hint
+
+        # Add table header if one exists
+        if self.completion_header:
+            if not metadata:
+                metadata += '\n'
+            metadata += '\n' + self.completion_header
+
+        return metadata
+
     def _display_matches_gnu_readline(self, substitution: str, matches: List[str],
                                       longest_match_length: int) -> None:  # pragma: no cover
         """Prints a match list using GNU readline's rl_display_match_list()
@@ -1523,9 +1547,8 @@ class Cmd(cmd.Cmd):
             strings_array[1:-1] = encoded_matches
             strings_array[-1] = None
 
-            # Print the header if one exists
-            if self.completion_header:
-                sys.stdout.write('\n\n' + self.completion_header)
+            # Print any metadata like a hint or table header
+            sys.stdout.write(self._build_completion_metadata_string())
 
             # Call readline's display function
             # rl_display_match_list(strings_array, number of completion matches, longest match length)
@@ -1551,10 +1574,8 @@ class Cmd(cmd.Cmd):
             # Add padding for visual appeal
             matches_to_display, _ = self._pad_matches_to_display(matches_to_display)
 
-            # Print the header if one exists
-            if self.completion_header:
-                # noinspection PyUnresolvedReferences
-                readline.rl.mode.console.write('\n\n' + self.completion_header)
+            # Print any metadata like a hint or table header
+            readline.rl.mode.console.write(sys.stdout.write(self._build_completion_metadata_string()))
 
             # Display matches using actual display function. This also redraws the prompt and line.
             orig_pyreadline_display(matches_to_display)
@@ -3317,7 +3338,7 @@ class Cmd(cmd.Cmd):
     # Create the parser for the set command
     set_parser = DEFAULT_ARGUMENT_PARSER(parents=[set_parser_parent])
     set_parser.add_argument('value', nargs=argparse.OPTIONAL, help='new value for settable',
-                            completer_method=complete_set_value)
+                            completer_method=complete_set_value, suppress_tab_hint=True)
 
     # Preserve quotes so users can pass in quoted empty strings and flags (e.g. -h) as the value
     @with_argparser(set_parser, preserve_quotes=True)
diff --git a/examples/transcripts/transcript_regex.txt b/examples/transcripts/transcript_regex.txt
index a3a91236..3065aae5 100644
--- a/examples/transcripts/transcript_regex.txt
+++ b/examples/transcripts/transcript_regex.txt
@@ -4,6 +4,7 @@
 # regexes on prompts just make the trailing space obvious
 (Cmd) set
 allow_style: '/(Terminal|Always|Never)/'
+always_show_hint: False
 debug: False
 echo: False
 editor: /.*?/
diff --git a/tests/conftest.py b/tests/conftest.py
index 5b1a6f05..73080b5c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -93,6 +93,7 @@ SHORTCUTS_TXT = """Shortcuts for other commands:
 
 # Output from the show command with default settings
 SHOW_TXT = """allow_style: 'Terminal'
+always_show_hint: False
 debug: False
 echo: False
 editor: 'vim'
@@ -104,6 +105,7 @@ timing: False
 
 SHOW_LONG = """
 allow_style: 'Terminal'   # Allow ANSI text style sequences in output (valid values: Terminal, Always, Never)
+always_show_hint: False   # Display tab completion hint even when completion suggestions print
 debug: False              # Show full traceback on exception
 echo: False               # Echo command issued into output
 editor: 'vim'             # Program used by 'edit'
diff --git a/tests/test_argparse_completer.py b/tests/test_argparse_completer.py
index 151923ea..cc9ffb54 100644
--- a/tests/test_argparse_completer.py
+++ b/tests/test_argparse_completer.py
@@ -4,6 +4,7 @@
 Unit/functional testing for argparse completer in cmd2
 """
 import argparse
+import numbers
 from typing import List
 
 import pytest
@@ -11,11 +12,11 @@ import pytest
 import cmd2
 from cmd2 import Cmd2ArgumentParser, CompletionItem, with_argparser
 from cmd2.utils import CompletionError, StdSim, basic_complete
-
 from .conftest import complete_tester, run_cmd
 
 # Lists used in our tests (there is a mix of sorted and unsorted on purpose)
-static_int_choices_list = [-1, 1, -2, 2, 0, -12]
+non_negative_int_choices = [1, 2, 3, 0, 22]
+int_choices = [-1, 1, -2, 2, 0, -12]
 static_choices_list = ['static', 'choices', 'stop', 'here']
 choices_from_function = ['choices', 'function', 'chatty', 'smith']
 choices_from_method = ['choices', 'method', 'most', 'improved']
@@ -55,7 +56,7 @@ def completer_takes_arg_tokens(text: str, line: str, begidx: int, endidx: int,
 
 
 # noinspection PyMethodMayBeStatic,PyUnusedLocal,PyProtectedMember
-class AutoCompleteTester(cmd2.Cmd):
+class ArgparseCompleterTester(cmd2.Cmd):
     """Cmd2 app that exercises ArgparseCompleter class"""
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -141,7 +142,7 @@ class AutoCompleteTester(cmd2.Cmd):
                                 choices_method=completion_item_method, metavar=TUPLE_METAVAR,
                                 nargs=argparse.ONE_OR_MORE)
     choices_parser.add_argument('-i', '--int', type=int, help='a flag with an int type',
-                                choices=static_int_choices_list)
+                                choices=int_choices)
 
     # Positional args for choices command
     choices_parser.add_argument("list_pos", help="a positional populated with a choices list",
@@ -150,6 +151,10 @@ class AutoCompleteTester(cmd2.Cmd):
                                 choices_function=choices_function)
     choices_parser.add_argument("method_pos", help="a positional populated with a choices method",
                                 choices_method=choices_method)
+    choices_parser.add_argument('non_negative_int', type=int, help='a positional with non-negative int choices',
+                                choices=non_negative_int_choices)
+    choices_parser.add_argument('empty_choices', help='a positional with empty choices',
+                                choices=[])
 
     @with_argparser(choices_parser)
     def do_choices(self, args: argparse.Namespace) -> None:
@@ -282,7 +287,7 @@ class AutoCompleteTester(cmd2.Cmd):
 
 @pytest.fixture
 def ac_app():
-    app = AutoCompleteTester()
+    app = ArgparseCompleterTester()
     app.stdout = StdSim(app.stdout)
     return app
 
@@ -415,14 +420,12 @@ def test_autcomp_flag_completion(ac_app, command_and_args, text, completions):
     ('--function', 'ch', ['choices', 'chatty']),
     ('-m', '', choices_from_method),
     ('--method', 'm', ['method', 'most']),
-    ('-i', '', static_int_choices_list),
+    ('-i', '', int_choices),
     ('--int', '1', ['1 ']),
     ('--int', '-', [-1, -2, -12]),
     ('--int', '-1', [-1, -12])
 ])
 def test_autocomp_flag_choices_completion(ac_app, flag, text, completions):
-    import numbers
-
     line = 'choices {} {}'.format(flag, text)
     endidx = len(line)
     begidx = endidx - len(text)
@@ -434,7 +437,7 @@ def test_autocomp_flag_choices_completion(ac_app, flag, text, completions):
         assert first_match is None
 
     # Numbers will be sorted in ascending order and then converted to strings by ArgparseCompleter
-    if all(isinstance(x, numbers.Number) for x in completions):
+    if completions and all(isinstance(x, numbers.Number) for x in completions):
         completions.sort()
         completions = [str(x) for x in completions]
     else:
@@ -449,7 +452,10 @@ def test_autocomp_flag_choices_completion(ac_app, flag, text, completions):
     (2, '', choices_from_function),
     (2, 'ch', ['choices', 'chatty']),
     (3, '', choices_from_method),
-    (3, 'm', ['method', 'most'])
+    (3, 'm', ['method', 'most']),
+    (4, '', non_negative_int_choices),
+    (4, '2', [2, 22]),
+    (5, '', []),
 ])
 def test_autocomp_positional_choices_completion(ac_app, pos, text, completions):
     # Generate line were preceding positionals are already filled
@@ -463,7 +469,35 @@ def test_autocomp_positional_choices_completion(ac_app, pos, text, completions):
     else:
         assert first_match is None
 
-    assert ac_app.completion_matches == sorted(completions, key=ac_app.default_sort_key)
+    # Numbers will be sorted in ascending order and then converted to strings by ArgparseCompleter
+    if completions and all(isinstance(x, numbers.Number) for x in completions):
+        completions.sort()
+        completions = [str(x) for x in completions]
+    else:
+        completions.sort(key=ac_app.default_sort_key)
+
+    assert ac_app.completion_matches == completions
+
+
+def test_flag_sorting(ac_app):
+    # This test exercises the case where a positional arg has non-negative integers for its choices.
+    # ArgparseCompleter will sort these numerically before converting them to strings. As a result,
+    # cmd2.matches_sorted gets set to True. If no completion matches are returned and the entered
+    # text looks like the beginning of a flag (e.g -), then ArgparseCompleter will try to complete
+    # flag names next. Before it does this, cmd2.matches_sorted is reset to make sure the flag names
+    # get sorted correctly.
+    option_strings = []
+    for action in ac_app.choices_parser._actions:
+        option_strings.extend(action.option_strings)
+    option_strings.sort(key=ac_app.default_sort_key)
+
+    text = '-'
+    line = 'choices arg1 arg2 arg3 {}'.format(text)
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    first_match = complete_tester(text, line, begidx, endidx, ac_app)
+    assert first_match is not None and ac_app.completion_matches == option_strings
 
 
 @pytest.mark.parametrize('flag, text, completions', [
diff --git a/tests/transcripts/regex_set.txt b/tests/transcripts/regex_set.txt
index 5004adc5..623df8ed 100644
--- a/tests/transcripts/regex_set.txt
+++ b/tests/transcripts/regex_set.txt
@@ -5,6 +5,7 @@
 
 (Cmd) set
 allow_style: /'(Terminal|Always|Never)'/
+always_show_hint: False
 debug: False
 echo: False
 editor: /'.*'/