Repository: python-cmd2/cmd2
Pull request #467 (2018-07-12 05:01:17)

History clear
Added --clear argument to history command
Added better error checking when loading readline history file
Improved some error messages
Changed IOError usages to OSError since they were merged in Python 3.3.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7d30d117..ebd83875 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 * Bug Fixes
     * Fixed bug when StatementParser ``__init__()`` was called with ``terminators`` equal to ``None``
     * Fixed bug when ``Cmd.onecmd()`` was called with a raw ``str``
+* Enhancements
+    * Added ``--clear`` flag to ``history`` command that clears both the command and readline history.
 * Deletions
     * The ``CmdResult`` helper class which was *deprecated* in the previous release has now been deleted
         * It has been replaced by the improved ``CommandResult`` class
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 2298f4b9..06295dfd 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -369,18 +369,6 @@ class Cmd(cmd.Cmd):
             except AttributeError:
                 pass
 
-        # If persistent readline history is enabled, then read history from file and register to write to file at exit
-        if persistent_history_file and rl_type != RlType.NONE:
-            persistent_history_file = os.path.expanduser(persistent_history_file)
-            try:
-                readline.read_history_file(persistent_history_file)
-                # default history len is -1 (infinite), which may grow unruly
-                readline.set_history_length(persistent_history_length)
-            except FileNotFoundError:
-                pass
-            import atexit
-            atexit.register(readline.write_history_file, persistent_history_file)
-
         # Call super class constructor
         super().__init__(completekey=completekey, stdin=stdin, stdout=stdout)
 
@@ -448,6 +436,37 @@ class Cmd(cmd.Cmd):
         # If this string is non-empty, then this warning message will print if a broken pipe error occurs while printing
         self.broken_pipe_warning = ''
 
+        # Check if history should persist
+        if persistent_history_file and rl_type != RlType.NONE:
+            persistent_history_file = os.path.expanduser(persistent_history_file)
+            read_err = False
+
+            try:
+                # First try to read any existing history file
+                readline.read_history_file(persistent_history_file)
+            except FileNotFoundError:
+                pass
+            except OSError as ex:
+                self.perror("readline cannot read persistent history file '{}': {}".format(persistent_history_file, ex),
+                            traceback_war=False)
+                read_err = True
+
+            if not read_err:
+                try:
+                    # Make sure readline is able to write the history file. Doing it this way is a more thorough check
+                    # than trying to open the file with write access since readline's underlying function needs to
+                    # create a temporary file in the same directory and may not have permission.
+                    readline.set_history_length(persistent_history_length)
+                    readline.write_history_file(persistent_history_file)
+                except OSError as ex:
+                    self.perror("readline cannot write persistent history file '{}': {}".
+                                format(persistent_history_file, ex), traceback_war=False)
+                else:
+                    # Set history file and register to save our history at exit
+                    import atexit
+                    self.persistent_history_file = persistent_history_file
+                    atexit.register(readline.write_history_file, self.persistent_history_file)
+
         # If a startup script is provided, then add it in the queue to load
         if startup_script is not None:
             startup_script = os.path.expanduser(startup_script)
@@ -610,7 +629,7 @@ class Cmd(cmd.Cmd):
                     try:
                         self.pipe_proc.stdin.write(msg_str.encode('utf-8', 'replace'))
                         self.pipe_proc.stdin.close()
-                    except (IOError, KeyboardInterrupt):
+                    except (OSError, KeyboardInterrupt):
                         pass
 
                     # Less doesn't respect ^C, but catches it for its own UI purposes (aborting search etc. inside less)
@@ -2574,8 +2593,9 @@ Usage:  Usage: unalias [-a] name [name ...]
                 try:
                     with open(filename) as f:
                         interp.runcode(f.read())
-                except IOError as e:
-                    self.perror(e)
+                except OSError as ex:
+                    error_msg = "Error opening script file '{}': {}".format(filename, ex)
+                    self.perror(error_msg, traceback_war=False)
 
             bridge = PyscriptBridge(self)
             self.pystate['run'] = run
@@ -2769,6 +2789,7 @@ Paths or arguments that contain spaces must be enclosed in quotes
     history_parser_group.add_argument('-s', '--script', action='store_true', help='script format; no separation lines')
     history_parser_group.add_argument('-o', '--output-file', metavar='FILE', help='output commands to a script file')
     history_parser_group.add_argument('-t', '--transcript', help='output commands and results to a transcript file')
+    history_parser_group.add_argument('-c', '--clear', action="store_true", help='clears all history')
     _history_arg_help = """empty               all history items
 a                   one history item by number
 a..b, a:b, a:, ..b  items by indices (inclusive)
@@ -2778,7 +2799,18 @@ a..b, a:b, a:, ..b  items by indices (inclusive)
 
     @with_argparser(history_parser)
     def do_history(self, args: argparse.Namespace) -> None:
-        """View, run, edit, and save previously entered commands."""
+        """View, run, edit, save, or clear previously entered commands."""
+
+        if args.clear:
+            # Clear command and readline history
+            self.history.clear()
+
+            if rl_type != RlType.NONE:
+                readline.clear_history()
+                if self.persistent_history_file:
+                    os.remove(self.persistent_history_file)
+            return
+
         # If an argument was supplied, then retrieve partial contents of the history
         cowardly_refuse_to_run = False
         if args.arg:
@@ -2984,25 +3016,30 @@ Script should contain one command per line, just like command would be typed in
         """
         # If arg is None or arg is an empty string this is an error
         if not arglist:
-            self.perror('load command requires a file path:', traceback_war=False)
+            self.perror('load command requires a file path', traceback_war=False)
             return
 
         file_path = arglist[0].strip()
         expanded_path = os.path.abspath(os.path.expanduser(file_path))
 
+        # Make sure the path exists and we can access it
+        if not os.path.exists(expanded_path):
+            self.perror("'{}' does not exist or cannot be accessed".format(expanded_path), traceback_war=False)
+            return
+
         # Make sure expanded_path points to a file
         if not os.path.isfile(expanded_path):
-            self.perror('{} does not exist or is not a file'.format(expanded_path), traceback_war=False)
+            self.perror("'{}' is not a file".format(expanded_path), traceback_war=False)
             return
 
         # Make sure the file is not empty
         if os.path.getsize(expanded_path) == 0:
-            self.perror('{} is empty'.format(expanded_path), traceback_war=False)
+            self.perror("'{}' is empty".format(expanded_path), traceback_war=False)
             return
 
         # Make sure the file is ASCII or UTF-8 encoded text
         if not utils.is_text_file(expanded_path):
-            self.perror('{} is not an ASCII or UTF-8 encoded text file'.format(expanded_path), traceback_war=False)
+            self.perror("'{}' is not an ASCII or UTF-8 encoded text file".format(expanded_path), traceback_war=False)
             return
 
         try:
@@ -3011,8 +3048,8 @@ Script should contain one command per line, just like command would be typed in
             # self._script_dir list when done.
             with open(expanded_path, encoding='utf-8') as target:
                 self.cmdqueue = target.read().splitlines() + ['eos'] + self.cmdqueue
-        except IOError as e:  # pragma: no cover
-            self.perror('Problem accessing script from {}:\n{}'.format(expanded_path, e))
+        except OSError as ex:  # pragma: no cover
+            self.perror("Problem accessing script from '{}': {}".format(expanded_path, ex))
             return
 
         self._script_dir.append(os.path.dirname(expanded_path))
diff --git a/cmd2/utils.py b/cmd2/utils.py
index 1f08b416..d03e7f6f 100644
--- a/cmd2/utils.py
+++ b/cmd2/utils.py
@@ -127,7 +127,7 @@ def is_text_file(file_path: str) -> bool:
             # noinspection PyUnusedLocal
             if sum(1 for line in f) > 0:
                 valid_text_file = True
-    except IOError:  # pragma: no cover
+    except OSError:  # pragma: no cover
         pass
     except UnicodeDecodeError:
         # The file is not ASCII. Check if it is UTF-8.
@@ -137,7 +137,7 @@ def is_text_file(file_path: str) -> bool:
                 # noinspection PyUnusedLocal
                 if sum(1 for line in f) > 0:
                     valid_text_file = True
-        except IOError:  # pragma: no cover
+        except OSError:  # pragma: no cover
             pass
         except UnicodeDecodeError:
             # Not UTF-8


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 90d45bd9..3f3b862e 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -37,7 +37,7 @@ Documented commands (type help <topic>):
 alias               Define or display aliases
 edit                Edit a file in a text editor.
 help                List available commands with "help" or detailed help with "help cmd".
-history             View, run, edit, and save previously entered commands.
+history             View, run, edit, save, or clear previously entered commands.
 load                Runs commands in script file that is encoded as either ASCII or UTF-8 text.
 py                  Invoke python command, shell, or script
 pyscript            Runs a python script file inside the console
@@ -49,9 +49,9 @@ unalias             Unsets aliases
 """
 
 # Help text for the history command
-HELP_HISTORY = """usage: history [-h] [-r | -e | -s | -o FILE | -t TRANSCRIPT] [arg]
+HELP_HISTORY = """usage: history [-h] [-r | -e | -s | -o FILE | -t TRANSCRIPT | -c] [arg]
 
-View, run, edit, and save previously entered commands.
+View, run, edit, save, or clear previously entered commands.
 
 positional arguments:
   arg                   empty               all history items
@@ -69,6 +69,7 @@ optional arguments:
                         output commands to a script file
   -t TRANSCRIPT, --transcript TRANSCRIPT
                         output commands and results to a transcript file
+  -c, --clear           clears all history
 """
 
 # Output from the shortcuts command with default built-in shortcuts
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index b973fdf5..e5dd3baa 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -231,7 +231,7 @@ def test_pyscript_with_nonexist_file(base_app, capsys):
     python_script = 'does_not_exist.py'
     run_cmd(base_app, "pyscript {}".format(python_script))
     out, err = capsys.readouterr()
-    assert err.startswith("EXCEPTION of type 'FileNotFoundError' occurred with message:")
+    assert "Error opening script file" in err
 
 def test_pyscript_with_exception(base_app, capsys, request):
     test_dir = os.path.dirname(request.module.__file__)
@@ -426,7 +426,7 @@ def test_history_run_all_commands(base_app):
     out = run_cmd(base_app, 'history -r')
     # this should generate an error, but we don't currently have a way to
     # capture stderr in these tests. So we assume that if we got nothing on
-    # standard out, that the error occured because if the commaned executed
+    # standard out, that the error occurred because if the command executed
     # then we should have a list of shortcuts in our output
     assert out == []
 
@@ -435,6 +435,23 @@ def test_history_run_one_command(base_app):
     output = run_cmd(base_app, 'history -r 1')
     assert output == expected
 
+def test_history_clear(base_app):
+    # Add commands to history
+    run_cmd(base_app, 'help')
+    run_cmd(base_app, 'alias')
+
+    # Make sure history has items
+    out = run_cmd(base_app, 'history')
+    assert out
+
+    # Clear the history
+    run_cmd(base_app, 'history --clear')
+
+    # Make sure history is empty
+    out = run_cmd(base_app, 'history')
+    assert out == []
+
+
 def test_base_load(base_app, request):
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'script.txt')
@@ -457,8 +474,7 @@ def test_load_with_empty_args(base_app, capsys):
     out, err = capsys.readouterr()
 
     # The load command requires a file path argument, so we should get an error message
-    expected = normalize("""ERROR: load command requires a file path:\n""")
-    assert normalize(str(err)) == expected
+    assert "load command requires a file path" in str(err)
     assert base_app.cmdqueue == []
 
 
@@ -468,10 +484,18 @@ def test_load_with_nonexistent_file(base_app, capsys):
     out, err = capsys.readouterr()
 
     # The load command requires a path to an existing file
-    assert str(err).startswith("ERROR")
-    assert "does not exist or is not a file" in str(err)
+    assert "does not exist" in str(err)
     assert base_app.cmdqueue == []
 
+def test_load_with_directory(base_app, capsys, request):
+    test_dir = os.path.dirname(request.module.__file__)
+
+    # The way the load command works, we can't directly capture its stdout or stderr
+    run_cmd(base_app, 'load {}'.format(test_dir))
+    out, err = capsys.readouterr()
+
+    assert "is not a file" in str(err)
+    assert base_app.cmdqueue == []
 
 def test_load_with_empty_file(base_app, capsys, request):
     test_dir = os.path.dirname(request.module.__file__)
@@ -481,8 +505,7 @@ def test_load_with_empty_file(base_app, capsys, request):
     run_cmd(base_app, 'load {}'.format(filename))
     out, err = capsys.readouterr()
 
-    # The load command requires non-empty scripts files
-    assert str(err).startswith("ERROR")
+    # The load command requires non-empty script files
     assert "is empty" in str(err)
     assert base_app.cmdqueue == []
 
@@ -724,7 +747,7 @@ def test_pipe_to_shell(base_app, capsys):
     out, err = capsys.readouterr()
 
     # Unfortunately with the improved way of piping output to a subprocess, there isn't any good way of getting
-    # access to the output produced by that subprocess within a unit test, but we can verify that no error occured
+    # access to the output produced by that subprocess within a unit test, but we can verify that no error occurred
     assert not err
 
 def test_pipe_to_shell_error(base_app, capsys):
@@ -1225,7 +1248,7 @@ Other
 ================================================================================
 alias               Define or display aliases
 help                List available commands with "help" or detailed help with "help cmd".
-history             View, run, edit, and save previously entered commands.
+history             View, run, edit, save, or clear previously entered commands.
 load                Runs commands in script file that is encoded as either ASCII or UTF-8 text.
 py                  Invoke python command, shell, or script
 pyscript            Runs a python script file inside the console