Fixed bug where completer function of disabled command would still run

3 files changed, 99 insertions, 37 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6764f0ed..f9622e62 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
     strings that crossed multiple lines.
     * Fixed a bug when appending to the clipboard where contents were in reverse order
     * Fixed issue where run_pyscript failed if the script's filename had 2 or more consecutive spaces
+    * Fixed issue where completer function of disabled command would still run
 * Enhancements
     * Greatly simplified using argparse-based tab completion. The new interface is a complete overhaul that breaks
     the previous way of specifying completion and choices functions. See header of [argparse_custom.py](https://github.com/python-cmd2/cmd2/blob/master/cmd2/argparse_custom.py)
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 56243037..753f28ee 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -99,6 +99,9 @@ COMMAND_FUNC_PREFIX = 'do_'
 # All help functions start with this
 HELP_FUNC_PREFIX = 'help_'
 
+# All command completer functions start with this
+COMPLETER_FUNC_PREFIX = 'complete_'
+
 # Sorting keys for strings
 ALPHABETICAL_SORT_KEY = utils.norm_fold
 NATURAL_SORT_KEY = utils.natural_keys
@@ -318,7 +321,7 @@ class EmptyStatement(Exception):
 
 
 # Contains data about a disabled command which is used to restore its original functions when the command is enabled
-DisabledCommand = namedtuple('DisabledCommand', ['command_function', 'help_function'])
+DisabledCommand = namedtuple('DisabledCommand', ['command_function', 'help_function', 'completer_function'])
 
 
 class Cmd(cmd.Cmd):
@@ -1468,7 +1471,7 @@ class Cmd(cmd.Cmd):
         # Check if a command was entered
         elif command in self.get_all_commands():
             # Get the completer function for this command
-            compfunc = getattr(self, 'complete_' + command, None)
+            compfunc = getattr(self, COMPLETER_FUNC_PREFIX + command, None)
 
             if compfunc is None:
                 # There's no completer function, next see if the command uses argparse
@@ -4045,16 +4048,24 @@ class Cmd(cmd.Cmd):
             return
 
         help_func_name = HELP_FUNC_PREFIX + command
+        completer_func_name = COMPLETER_FUNC_PREFIX + command
 
-        # Restore the command and help functions to their original values
+        # Restore the command function to its original value
         dc = self.disabled_commands[command]
         setattr(self, self._cmd_func_name(command), dc.command_function)
 
+        # Restore the help function to its original value
         if dc.help_function is None:
             delattr(self, help_func_name)
         else:
             setattr(self, help_func_name, dc.help_function)
 
+        # Restore the completer function to its original value
+        if dc.completer_function is None:
+            delattr(self, completer_func_name)
+        else:
+            setattr(self, completer_func_name, dc.completer_function)
+
         # Remove the disabled command entry
         del self.disabled_commands[command]
 
@@ -4090,10 +4101,12 @@ class Cmd(cmd.Cmd):
             raise AttributeError("{} does not refer to a command".format(command))
 
         help_func_name = HELP_FUNC_PREFIX + command
+        completer_func_name = COMPLETER_FUNC_PREFIX + command
 
         # Add the disabled command record
         self.disabled_commands[command] = DisabledCommand(command_function=command_function,
-                                                          help_function=getattr(self, help_func_name, None))
+                                                          help_function=getattr(self, help_func_name, None),
+                                                          completer_function=getattr(self, completer_func_name, None))
 
         # Overwrite the command and help functions to print the message
         new_func = functools.partial(self._report_disabled_command_usage,
@@ -4101,6 +4114,9 @@ class Cmd(cmd.Cmd):
         setattr(self, self._cmd_func_name(command), new_func)
         setattr(self, help_func_name, new_func)
 
+        # Set the completer to a function that returns a blank list
+        setattr(self, completer_func_name, lambda *args, **kwargs: [])
+
     def disable_category(self, category: str, message_to_print: str) -> None:
         """Disable an entire category of commands.
 
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 1f63e234..1bd81c50 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -5,11 +5,11 @@ Cmd2 unit/functional testing
 """
 import argparse
 import builtins
-from code import InteractiveConsole
 import io
 import os
 import sys
 import tempfile
+from code import InteractiveConsole
 
 import pytest
 
@@ -21,7 +21,8 @@ except ImportError:
 
 import cmd2
 from cmd2 import ansi, clipboard, constants, utils
-from .conftest import run_cmd, normalize, verify_help_text, HELP_HISTORY, SHORTCUTS_TXT, SHOW_TXT, SHOW_LONG
+from .conftest import run_cmd, normalize, verify_help_text, HELP_HISTORY
+from .conftest import SHORTCUTS_TXT, SHOW_TXT, SHOW_LONG, complete_tester
 
 def CreateOutsimApp():
     c = cmd2.Cmd()
@@ -2100,16 +2101,19 @@ class DisableCommandsApp(cmd2.Cmd):
         super().__init__(*args, **kwargs)
 
     @cmd2.with_category(category_name)
-    def do_has_help_func(self, arg):
-        self.poutput("The real has_help_func")
+    def do_has_helper_funcs(self, arg):
+        self.poutput("The real has_helper_funcs")
+
+    def help_has_helper_funcs(self):
+        self.poutput('Help for has_helper_funcs')
 
-    def help_has_help_func(self):
-        self.poutput('Help for has_help_func')
+    def complete_has_helper_funcs(self, *args):
+        return ['result']
 
     @cmd2.with_category(category_name)
-    def do_has_no_help_func(self, arg):
-        """Help for has_no_help_func"""
-        self.poutput("The real has_no_help_func")
+    def do_has_no_helper_funcs(self, arg):
+        """Help for has_no_helper_funcs"""
+        self.poutput("The real has_no_helper_funcs")
 
 
 @pytest.fixture
@@ -2119,51 +2123,92 @@ def disable_commands_app():
 
 
 def test_disable_and_enable_category(disable_commands_app):
+    ##########################################################################
     # Disable the category
+    ##########################################################################
     message_to_print = 'These commands are currently disabled'
     disable_commands_app.disable_category(disable_commands_app.category_name, message_to_print)
 
     # Make sure all the commands and help on those commands displays the message
-    out, err = run_cmd(disable_commands_app, 'has_help_func')
+    out, err = run_cmd(disable_commands_app, 'has_helper_funcs')
     assert err[0].startswith(message_to_print)
 
-    out, err = run_cmd(disable_commands_app, 'help has_help_func')
+    out, err = run_cmd(disable_commands_app, 'help has_helper_funcs')
     assert err[0].startswith(message_to_print)
 
-    out, err = run_cmd(disable_commands_app, 'has_no_help_func')
+    out, err = run_cmd(disable_commands_app, 'has_no_helper_funcs')
     assert err[0].startswith(message_to_print)
 
-    out, err = run_cmd(disable_commands_app, 'help has_no_help_func')
+    out, err = run_cmd(disable_commands_app, 'help has_no_helper_funcs')
     assert err[0].startswith(message_to_print)
 
+    # Make sure neither function completes
+    text = ''
+    line = 'has_helper_funcs'
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    first_match = complete_tester(text, line, begidx, endidx, disable_commands_app)
+    assert first_match is None
+
+    text = ''
+    line = 'has_no_helper_funcs'
+    endidx = len(line)
+    begidx = endidx - len(text)
+
+    first_match = complete_tester(text, line, begidx, endidx, disable_commands_app)
+    assert first_match is None
+
+    # Make sure both commands are invisible
     visible_commands = disable_commands_app.get_visible_commands()
-    assert 'has_help_func' not in visible_commands
-    assert 'has_no_help_func' not in visible_commands
+    assert 'has_helper_funcs' not in visible_commands
+    assert 'has_no_helper_funcs' not in visible_commands
 
+    ##########################################################################
     # Enable the category
+    ##########################################################################
     disable_commands_app.enable_category(disable_commands_app.category_name)
 
     # Make sure all the commands and help on those commands are restored
-    out, err = run_cmd(disable_commands_app, 'has_help_func')
-    assert out[0] == "The real has_help_func"
+    out, err = run_cmd(disable_commands_app, 'has_helper_funcs')
+    assert out[0] == "The real has_helper_funcs"
+
+    out, err = run_cmd(disable_commands_app, 'help has_helper_funcs')
+    assert out[0] == "Help for has_helper_funcs"
+
+    out, err = run_cmd(disable_commands_app, 'has_no_helper_funcs')
+    assert out[0] == "The real has_no_helper_funcs"
+
+    out, err = run_cmd(disable_commands_app, 'help has_no_helper_funcs')
+    assert out[0] == "Help for has_no_helper_funcs"
+
+    # has_helper_funcs should complete now
+    text = ''
+    line = 'has_helper_funcs'
+    endidx = len(line)
+    begidx = endidx - len(text)
 
-    out, err = run_cmd(disable_commands_app, 'help has_help_func')
-    assert out[0] == "Help for has_help_func"
+    first_match = complete_tester(text, line, begidx, endidx, disable_commands_app)
+    assert first_match is not None and disable_commands_app.completion_matches == ['result ']
 
-    out, err = run_cmd(disable_commands_app, 'has_no_help_func')
-    assert out[0] == "The real has_no_help_func"
+    # has_no_helper_funcs had no completer originally, so there should be no results
+    text = ''
+    line = 'has_no_helper_funcs'
+    endidx = len(line)
+    begidx = endidx - len(text)
 
-    out, err = run_cmd(disable_commands_app, 'help has_no_help_func')
-    assert out[0] == "Help for has_no_help_func"
+    first_match = complete_tester(text, line, begidx, endidx, disable_commands_app)
+    assert first_match is None
 
+    # Make sure both commands are visible
     visible_commands = disable_commands_app.get_visible_commands()
-    assert 'has_help_func' in visible_commands
-    assert 'has_no_help_func' in visible_commands
+    assert 'has_helper_funcs' in visible_commands
+    assert 'has_no_helper_funcs' in visible_commands
 
 def test_enable_enabled_command(disable_commands_app):
     # Test enabling a command that is not disabled
     saved_len = len(disable_commands_app.disabled_commands)
-    disable_commands_app.enable_command('has_help_func')
+    disable_commands_app.enable_command('has_helper_funcs')
 
     # The number of disabled_commands should not have changed
     assert saved_len == len(disable_commands_app.disabled_commands)
@@ -2175,7 +2220,7 @@ def test_disable_fake_command(disable_commands_app):
 def test_disable_command_twice(disable_commands_app):
     saved_len = len(disable_commands_app.disabled_commands)
     message_to_print = 'These commands are currently disabled'
-    disable_commands_app.disable_command('has_help_func', message_to_print)
+    disable_commands_app.disable_command('has_helper_funcs', message_to_print)
 
     # The length of disabled_commands should have increased one
     new_len = len(disable_commands_app.disabled_commands)
@@ -2183,24 +2228,24 @@ def test_disable_command_twice(disable_commands_app):
     saved_len = new_len
 
     # Disable again and the length should not change
-    disable_commands_app.disable_command('has_help_func', message_to_print)
+    disable_commands_app.disable_command('has_helper_funcs', message_to_print)
     new_len = len(disable_commands_app.disabled_commands)
     assert saved_len == new_len
 
 def test_disabled_command_not_in_history(disable_commands_app):
     message_to_print = 'These commands are currently disabled'
-    disable_commands_app.disable_command('has_help_func', message_to_print)
+    disable_commands_app.disable_command('has_helper_funcs', message_to_print)
 
     saved_len = len(disable_commands_app.history)
-    run_cmd(disable_commands_app, 'has_help_func')
+    run_cmd(disable_commands_app, 'has_helper_funcs')
     assert saved_len == len(disable_commands_app.history)
 
 def test_disabled_message_command_name(disable_commands_app):
     message_to_print = '{} is currently disabled'.format(cmd2.cmd2.COMMAND_NAME)
-    disable_commands_app.disable_command('has_help_func', message_to_print)
+    disable_commands_app.disable_command('has_helper_funcs', message_to_print)
 
-    out, err = run_cmd(disable_commands_app, 'has_help_func')
-    assert err[0].startswith('has_help_func is currently disabled')
+    out, err = run_cmd(disable_commands_app, 'has_helper_funcs')
+    assert err[0].startswith('has_helper_funcs is currently disabled')
 
 
 def test_startup_script(request):