Merge pull request #738 from python-cmd2/presentation_changes

Presentation changes

Author: kmvanbrunt

cmd2/argparse_custom.py

@@ -81,15 +81,19 @@ def my_completer_function(text, line, begidx, endidx):
             parser.add_argument('-o', '--options', completer_method=cmd2.Cmd.path_complete)
 
 
-    In all cases in which function/methods are passed you can use functools.partial() to prepopulate
-    values of the underlying function.
+    You can use functools.partial() to prepopulate values of the underlying choices and completer functions/methods.
 
         Example:
             This says to call path_complete with a preset value for its path_filter argument.
             completer_method = functools.partial(path_complete,
                                                  path_filter=lambda path: os.path.isdir(path))
             parser.add_argument('-o', '--options', choices_method=completer_method)
 
+    Of the 5 tab-completion parameters, choices is the only one where argparse validates user input against items
+    in the choices list. This is because the other 4 parameters are meant to tab complete data sets that are viewed
+    as dynamic. Therefore it is up to the user to validate if the user has typed an acceptable value for these
+    arguments.
+
 CompletionItem Class:
     This class was added to help in cases where uninformative data is being tab completed. For instance,
     tab completing ID numbers isn't very helpful to a user without context. Returning a list of CompletionItems

cmd2/cmd2.py

@@ -1724,10 +1724,11 @@ def parseline(self, line: str) -> Tuple[str, str, str]:
         statement = self.statement_parser.parse_command_only(line)
         return statement.command, statement.args, statement.command_and_args
 
-    def onecmd_plus_hooks(self, line: str, py_bridge_call: bool = False) -> bool:
+    def onecmd_plus_hooks(self, line: str, *, add_to_history: bool = True, py_bridge_call: bool = False) -> bool:
         """Top-level function called by cmdloop() to handle parsing a line and running the command and all of its hooks.
 
         :param line: line of text read from input
+        :param add_to_history: If True, then add this command to history. Defaults to True.
         :param py_bridge_call: This should only ever be set to True by PyBridge to signify the beginning
                                of an app() call from Python. It is used to enable/disable the storage of the
                                command's stdout.
@@ -1795,7 +1796,7 @@ def onecmd_plus_hooks(self, line: str, py_bridge_call: bool = False) -> bool:
                     statement = self.precmd(statement)
 
                     # go run the command function
-                    stop = self.onecmd(statement)
+                    stop = self.onecmd(statement, add_to_history=add_to_history)
 
                     # postcommand hooks
                     data = plugin.PostcommandData(stop, statement)
@@ -1852,12 +1853,13 @@ def _run_cmdfinalization_hooks(self, stop: bool, statement: Optional[Statement])
         except Exception as ex:
             self.pexcept(ex)
 
-    def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]]) -> bool:
+    def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]], *, add_to_history: bool = True) -> bool:
         """
         Used when commands are being run in an automated fashion like text scripts or history replays.
         The prompt and command line for each command will be printed if echo is True.
 
         :param cmds: commands to run
+        :param add_to_history: If True, then add these commands to history. Defaults to True.
         :return: True if running of commands should stop
         """
         for line in cmds:
@@ -1867,7 +1869,7 @@ def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]]) -> bool:
             if self.echo:
                 self.poutput('{}{}'.format(self.prompt, line))
 
-            if self.onecmd_plus_hooks(line):
+            if self.onecmd_plus_hooks(line, add_to_history=add_to_history):
                 return True
 
         return False
@@ -2167,13 +2169,15 @@ def _cmd_func_name(self, command: str) -> str:
         target = COMMAND_FUNC_PREFIX + command
         return target if callable(getattr(self, target, None)) else ''
 
-    def onecmd(self, statement: Union[Statement, str]) -> bool:
+    # noinspection PyMethodOverriding
+    def onecmd(self, statement: Union[Statement, str], *, add_to_history: bool = True) -> bool:
         """ This executes the actual do_* method for a command.
 
         If the command provided doesn't exist, then it executes default() instead.
 
         :param statement: intended to be a Statement instance parsed command from the input stream, alternative
                           acceptance of a str is present only for backward compatibility with cmd
+        :param add_to_history: If True, then add this command to history. Defaults to True.
         :return: a flag indicating whether the interpretation of commands should stop
         """
         # For backwards compatibility with cmd, allow a str to be passed in
@@ -2183,8 +2187,9 @@ def onecmd(self, statement: Union[Statement, str]) -> bool:
         func = self.cmd_func(statement.command)
         if func:
             # Check to see if this command should be stored in history
-            if statement.command not in self.exclude_from_history \
-                    and statement.command not in self.disabled_commands:
+            if statement.command not in self.exclude_from_history and \
+                    statement.command not in self.disabled_commands and add_to_history:
+
                 self.history.append(statement)
 
             stop = func(statement)
@@ -2391,7 +2396,7 @@ def _alias_delete(self, args: argparse.Namespace) -> None:
             self.aliases.clear()
             self.poutput("All aliases deleted")
         elif not args.name:
-            self.do_help('alias delete')
+            self.perror("Either --all or alias name(s) must be specified")
         else:
             for cur_name in utils.remove_duplicates(args.name):
                 if cur_name in self.aliases:
@@ -2571,7 +2576,7 @@ def _macro_delete(self, args: argparse.Namespace) -> None:
             self.macros.clear()
             self.poutput("All macros deleted")
         elif not args.name:
-            self.do_help('macro delete')
+            self.perror("Either --all or macro name(s) must be specified")
         else:
             for cur_name in utils.remove_duplicates(args.name):
                 if cur_name in self.macros:
@@ -2640,7 +2645,7 @@ def _macro_list(self, args: argparse.Namespace) -> None:
                            "    macro create show_results print_results -type {1} \"|\" less\n"
                            "\n"
                            "  Because macros do not resolve until after hitting Enter, tab completion\n"
-                           "  will only complete paths while entering a macro.")
+                           "  will only complete paths while typing a macro.")
 
     macro_create_parser = macro_subparsers.add_parser('create', help=macro_create_help,
                                                       description=macro_create_description,

tests/test_cmd2.py

@@ -1617,7 +1617,7 @@ def test_alias_delete_non_existing(base_app):
 
 def test_alias_delete_no_name(base_app):
     out, err = run_cmd(base_app, 'alias delete')
-    assert "Usage: alias delete" in out[0]
+    assert "Either --all or alias name(s)" in err[0]
 
 def test_multiple_aliases(base_app):
     alias1 = 'h1'
@@ -1768,7 +1768,7 @@ def test_macro_delete_non_existing(base_app):
 
 def test_macro_delete_no_name(base_app):
     out, err = run_cmd(base_app, 'macro delete')
-    assert "Usage: macro delete" in out[0]
+    assert "Either --all or macro name(s)" in err[0]
 
 def test_multiple_macros(base_app):
     macro1 = 'h1'
@@ -1853,6 +1853,21 @@ def test_onecmd_raw_str_quit(outsim_app):
     assert stop
     assert out == ''
 
+def test_onecmd_add_to_history(outsim_app):
+    line = "help"
+    saved_hist_len = len(outsim_app.history)
+
+    # Allow command to be added to history
+    outsim_app.onecmd(line, add_to_history=True)
+    new_hist_len = len(outsim_app.history)
+    assert new_hist_len == saved_hist_len + 1
+
+    saved_hist_len = new_hist_len
+
+    # Prevent command from being added to history
+    outsim_app.onecmd(line, add_to_history=False)
+    new_hist_len = len(outsim_app.history)
+    assert new_hist_len == saved_hist_len
 
 def test_get_all_commands(base_app):
     # Verify that the base app has the expected commands