Repository: python-cmd2/cmd2
Pull request #993 (2020-09-04 05:33:32)

Completion hint enhancements
* 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

codecov[bot]: # [Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=h1) Report
> Merging [#993](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=desc) into [master](https://codecov.io/gh/python-cmd2/cmd2/commit/1054dda76b87d9f7f77311d8d804c1017b668996?el=desc) will **decrease** coverage by `0.16%`.
> The diff coverage is `78.37%`.

[![Impacted file tree graph](https://codecov.io/gh/python-cmd2/cmd2/pull/993/graphs/tree.svg?width=650&height=150&src=pr&token=eks2PUwa6A)](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
- Coverage   97.84%   97.67%   -0.17%     
==========================================
  Files          22       22              
  Lines        4540     4567      +27     
==========================================
+ Hits         4442     4461      +19     
- Misses         98      106       +8     
```


| [Impacted Files](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cmd2/cmd2.py](https://codecov.io/gh/python-cmd2/cmd2/pull/993/diff?src=pr&el=tree#diff-Y21kMi9jbWQyLnB5) | `96.85% <38.46%> (-0.37%)` | :arrow_down: |
| [cmd2/argparse\_completer.py](https://codecov.io/gh/python-cmd2/cmd2/pull/993/diff?src=pr&el=tree#diff-Y21kMi9hcmdwYXJzZV9jb21wbGV0ZXIucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=footer). Last update [1054dda...2d1c2bf](https://codecov.io/gh/python-cmd2/cmd2/pull/993?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tleonhardt: From the description, this sounds like a great enhancement!  I look forward to reviewing it later when I have some time.

Patch:
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: /.*?/


Test patch:
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: /'.*'/