Merge pull request #467 from python-cmd2/history

History clear

5 files changed, 100 insertions, 37 deletions

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
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