Initializing history now detects plaintext or pickle format

4 files changed, 130 insertions, 54 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2a43e961..698922ff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,13 +23,25 @@
     * Added support for custom Namespaces in the argparse decorators. See description of `ns_provider` argument
     for more information.
     * Transcript testing now sets the `exit_code` returned from `cmdloop` based on Success/Failure
-* Potentially breaking changes
+    * The history of entered commands previously was saved using the readline persistence mechanism,
+      and only persisted if you had readline installed. Now history is persisted independent of readline; user
+      input from previous invocations of `cmd2` based apps now shows in the `history` command.
+
+* Breaking changes
     * Replaced `unquote_redirection_tokens()` with `unquote_specific_tokens()`. This was to support the fix
       that allows terminators in alias and macro values.
     * Changed `Statement.pipe_to` to a string instead of a list
     * `preserve_quotes` is now a keyword-only argument in the argparse decorators
     * Refactored so that `cmd2.Cmd.cmdloop()` returns the `exit_code` instead of a call to `sys.exit()`
         * It is now applicaiton developer's responsibility to treat the return value from `cmdloop()` accordingly
+      , and is in a binary format,
+      not a text format.
+    * Only valid commands are persistent in history between invocations of `cmd2` based apps. Previously
+      all user input was persistent in history. If readline is installed, the history available with the up and
+      down arrow keys (readline history) may not match that shown in the `history` command, because `history`
+      only tracks valid input, while readline history captures all input.
+    * History is now persisted in a binary format, not plain text format. Previous history files are destroyed
+      on first launch of a `cmd2` based app of version 0.9.13 or higher.
 * **Python 3.4 EOL notice**
     * Python 3.4 reached its [end of life](https://www.python.org/dev/peps/pep-0429/) on March 18, 2019
     * This is the last release of `cmd2` which will support Python 3.4
@@ -38,7 +50,7 @@
 * Bug Fixes
     * Fixed a bug in how redirection and piping worked inside ``py`` or ``pyscript`` commands
     * Fixed bug in `async_alert` where it didn't account for prompts that contained newline characters
-    * Fixed path completion case when CWD is just a slash. Relative path matches were incorrectly prepended with a slash. 
+    * Fixed path completion case when CWD is just a slash. Relative path matches were incorrectly prepended with a slash.
 * Enhancements
     * Added ability to include command name placeholders in the message printed when trying to run a disabled command.
         * See docstring for ``disable_command()`` or ``disable_category()`` for more details.
@@ -60,7 +72,7 @@
         * ``_report_disabled_command_usage()`` - in all cases since this is called when a disabled command is run
     * Removed *** from beginning of error messages printed by `do_help()` and `default()`
     * Significantly refactored ``cmd.Cmd`` class so that all class attributes got converted to instance attributes, also:
-        * Added ``allow_redirection``, ``terminators``, ``multiline_commands``, and ``shortcuts`` as optional arguments 
+        * Added ``allow_redirection``, ``terminators``, ``multiline_commands``, and ``shortcuts`` as optional arguments
         to ``cmd.Cmd.__init__()`
         * A few instance attributes were moved inside ``StatementParser`` and properties were created for accessing them
     * ``self.pipe_proc`` is now called ``self.cur_pipe_proc_reader`` and is a ``ProcReader`` class.
@@ -98,7 +110,7 @@
     ``cmd2`` convention of setting ``self.matches_sorted`` to True before returning the results if you have already
     sorted the ``CompletionItem`` list. Otherwise it will be sorted using ``self.matches_sort_key``.
     * Removed support for bash completion since this feature had slow performance. Also it relied on
-    ``AutoCompleter`` which has since developed a dependency on ``cmd2`` methods. 
+    ``AutoCompleter`` which has since developed a dependency on ``cmd2`` methods.
     * Removed ability to call commands in ``pyscript`` as if they were functions (e.g. ``app.help()``) in favor
     of only supporting one ``pyscript`` interface. This simplifies future maintenance.
     * No longer supporting C-style comments. Hash (#) is the only valid comment marker.
@@ -118,7 +130,7 @@
     * Fixed bug where the ``set`` command was not tab completing from the current ``settable`` dictionary.
 * Enhancements
     * Changed edit command to use do_shell() instead of calling os.system()
-    
+
 ## 0.9.8 (February 06, 2019)
 * Bug Fixes
     * Fixed issue with echoing strings in StdSim. Because they were being sent to a binary buffer, line buffering
@@ -139,9 +151,9 @@
 * Deletions (potentially breaking changes)
     * Deleted ``Cmd.colorize()`` and ``Cmd._colorcodes`` which were deprecated in 0.9.5
     * Replaced ``dir_exe_only`` and  ``dir_only`` flags in ``path_complete`` with optional ``path_filter`` function
-    that is used to filter paths out of completion results. 
+    that is used to filter paths out of completion results.
     * ``perror()`` no longer prepends "ERROR: " to the error message being printed
-    
+
 ## 0.9.6 (October 13, 2018)
 * Bug Fixes
     * Fixed bug introduced in 0.9.5 caused by backing up and restoring `self.prompt` in `pseudo_raw_input`.
@@ -167,8 +179,8 @@
     the argparse object. Also, single-character tokens that happen to be a
     prefix char are not treated as flags by argparse and AutoCompleter now
     matches that behavior.
-    * Fixed bug where AutoCompleter was not distinguishing between a negative number and a flag 
-    * Fixed bug where AutoCompleter did not handle -- the same way argparse does (all args after -- are non-options)  
+    * Fixed bug where AutoCompleter was not distinguishing between a negative number and a flag
+    * Fixed bug where AutoCompleter did not handle -- the same way argparse does (all args after -- are non-options)
 * Enhancements
     * Added ``exit_code`` attribute of ``cmd2.Cmd`` class
         * Enables applications to return a non-zero exit code when exiting from ``cmdloop``
@@ -180,10 +192,10 @@
         * These allow you to provide feedback to the user in an asychronous fashion, meaning alerts can
         display when the user is still entering text at the prompt. See [async_printing.py](https://github.com/python-cmd2/cmd2/blob/master/examples/async_printing.py)
         for an example.
-    * Cross-platform colored output support 
+    * Cross-platform colored output support
         * ``colorama`` gets initialized properly in ``Cmd.__init()``
         * The ``Cmd.colors`` setting is no longer platform dependent and now has three values:
-            * Terminal (default) - output methods do not strip any ANSI escape sequences when output is a terminal, but 
+            * Terminal (default) - output methods do not strip any ANSI escape sequences when output is a terminal, but
             if the output is a pipe or a file the escape sequences are stripped
             * Always - output methods **never** strip ANSI escape sequences, regardless of the output destination
             * Never - output methods strip all ANSI escape sequences
@@ -193,18 +205,18 @@
 * Deprecations
     * Deprecated the built-in ``cmd2`` support for colors including ``Cmd.colorize()`` and ``Cmd._colorcodes``
 * Deletions (potentially breaking changes)
-    * The ``preparse``, ``postparsing_precmd``, and ``postparsing_postcmd`` methods *deprecated* in the previous release 
+    * The ``preparse``, ``postparsing_precmd``, and ``postparsing_postcmd`` methods *deprecated* in the previous release
     have been deleted
         * The new application lifecycle hook system allows for registration of callbacks to be called at various points
         in the lifecycle and is more powerful and flexible than the previous system
     * ``alias`` is now a command with sub-commands to create, list, and delete aliases. Therefore its syntax
       has changed. All current alias commands in startup scripts or transcripts will break with this release.
     * `unalias` was deleted since ``alias delete`` replaced it
-    
+
 ## 0.9.4 (August 21, 2018)
 * Bug Fixes
     * Fixed bug where ``preparse`` was not getting called
-    * Fixed bug in parsing of multiline commands where matching quote is on another line 
+    * Fixed bug in parsing of multiline commands where matching quote is on another line
 * Enhancements
     * Improved implementation of lifecycle hooks to support a plugin
       framework, see ``docs/hooks.rst`` for details.
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 9d36e1b4..14c4227a 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -34,6 +34,7 @@ import cmd
 import glob
 import inspect
 import os
+import pickle
 import re
 import sys
 import threading
@@ -408,7 +409,6 @@ class Cmd(cmd.Cmd):
         self.macros = dict()
 
         self.initial_stdout = sys.stdout
-        self.history = History()
         self.pystate = {}
         self.py_history = []
         self.pyscript_name = 'app'
@@ -463,37 +463,8 @@ 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
-        self.persistent_history_file = ''
-        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)
+        # initialize history
+        self._initialize_history(persistent_history_file, persistent_history_length)
 
         # If a startup script is provided, then add it in the queue to load
         if startup_script is not None:
@@ -3476,6 +3447,53 @@ class Cmd(cmd.Cmd):
             for hi in history:
                 self.poutput(hi.pr(script=args.script, expanded=args.expanded, verbose=args.verbose))
 
+    def _initialize_history(self, histfile, maxlen):
+        """Initialize history with optional persistence and maximum length
+
+        This function can determine whether history is saved in the prior text-based
+        format (one line of input is stored as one line in the file), or the new-as-
+        of-version 0.9.13 pickle based format.
+
+        History created by versions <= 0.9.12 is in readline format, i.e. plain text files.
+
+        Initializing history does not effect history files on disk, versions >= 0.9.13 always
+        write history in the pickle format.
+        """
+        self.history = History()
+        # with no persistent history, nothing else in this method is relevant
+        if not histfile:
+            return
+
+        histfile = os.path.expanduser(histfile)
+
+        # first we try and unpickle the history file
+        history = History()
+        try:
+            with open(histfile, 'rb') as fobj:
+                history = pickle.load(fobj)
+        except (FileNotFoundError, KeyError, EOFError):
+            pass
+        except OSError as ex:
+            msg = "can not read persistent history file '{}': {}"
+            self.perror(msg.format(histfile, ex), traceback_war=False)
+
+        # trim history to length and ensure it's writable
+        history.truncate(maxlen)
+        try:
+            # open with append so it doesn't truncate the file
+            with open(histfile, 'ab') as fobj:
+                self.persistent_history_file = histfile
+        except OSError as ex:
+            msg = "can not write persistent history file '{}': {}"
+            self.perror(msg.format(histfile, ex), traceback_war=False)
+
+        # register a function to write history at save
+        # if the history file is in plain text format from 0.9.12 or lower
+        # this will fail, and the history in the plain text file will be lost
+
+        #import atexit
+        #atexit.register(readline.write_history_file, self.persistent_history_file)
+
     def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcript_file: str) -> None:
         """Generate a transcript file from a given history of commands."""
         import io
diff --git a/cmd2/history.py b/cmd2/history.py
index 7cc36bfc..3b246f95 100644
--- a/cmd2/history.py
+++ b/cmd2/history.py
@@ -224,3 +224,17 @@ class History(list):
             """filter function for doing a regular expression search of history"""
             return finder.search(hi) or finder.search(hi.expanded)
         return [itm for itm in self if isin(itm)]
+
+    def truncate(self, max_length:int) -> None:
+        """Truncate the length of the history, dropping the oldest items if necessary
+
+        :param max_length: the maximum length of the history, if negative, all history
+                           items will be deleted
+        :return: nothing
+        """
+        if max_length <= 0:
+            # remove all history
+            del self[:]
+        elif len(self) > max_length:
+            last_element = len(self) - max_length
+            del self[0:last_element]
diff --git a/tests/test_history.py b/tests/test_history.py
index 105dead9..554281c4 100644
--- a/tests/test_history.py
+++ b/tests/test_history.py
@@ -5,6 +5,7 @@ Test history functions of cmd2
 """
 import tempfile
 import os
+import pickle
 import sys
 
 import pytest
@@ -121,6 +122,19 @@ def test_history_regex_search(hist):
     assert hist.regex_search('/i.*d/') == ['third']
     assert hist.regex_search('s[a-z]+ond') == ['second']
 
+def test_history_max_length_zero(hist):
+    hist.truncate(0)
+    assert len(hist) == 0
+
+def test_history_max_length_negative(hist):
+    hist.truncate(-1)
+    assert len(hist) == 0
+
+def test_history_max_length(hist):
+    hist.truncate(2)
+    assert hist.get(1) == 'third'
+    assert hist.get(2) == 'fourth'
+
 def test_base_history(base_app):
     run_cmd(base_app, 'help')
     run_cmd(base_app, 'shortcuts')
@@ -399,7 +413,7 @@ def test_existing_history_file(hist_file, capsys):
     assert err == ''
 
     # Unregister the call to write_history_file that cmd2 did
-    atexit.unregister(readline.write_history_file)
+    ## TODO atexit.unregister(readline.write_history_file)
 
     # Remove created history file
     os.remove(hist_file)
@@ -422,7 +436,7 @@ def test_new_history_file(hist_file, capsys):
     assert err == ''
 
     # Unregister the call to write_history_file that cmd2 did
-    atexit.unregister(readline.write_history_file)
+    ### TODO atexit.unregister(readline.write_history_file)
 
     # Remove created history file
     os.remove(hist_file)
@@ -435,10 +449,28 @@ def test_bad_history_file_path(capsys, request):
     cmd2.Cmd(persistent_history_file=test_dir)
     _, err = capsys.readouterr()
 
-    if sys.platform == 'win32':
-        # pyreadline masks the read exception. Therefore the bad path error occurs when trying to write the file.
-        assert 'readline cannot write' in err
-    else:
-        # GNU readline raises an exception upon trying to read the directory as a file
-        assert 'readline cannot read' in err
+    assert 'can not write' in err
+
+def test_history_file_conversion_no_truncate_on_init(hist_file, capsys):
+    # test the code that converts a plain text history file to a pickle binary
+    # history file
+
+    # first we need some plain text commands in the history file
+    with open(hist_file, 'w') as hfobj:
+        hfobj.write('help\n')
+        hfobj.write('alias\n')
+        hfobj.write('alias create s shortcuts\n')
+
+    # Create a new cmd2 app
+    cmd2.Cmd(persistent_history_file=hist_file)
+
+    # history should be initialized, but the file on disk should
+    # still be plain text
+    with open(hist_file, 'r') as hfobj:
+        histlist= hfobj.readlines()
 
+    assert len(histlist) == 3
+    # history.get() is overridden to be one based, not zero based
+    assert histlist[0]== 'help\n'
+    assert histlist[1] == 'alias\n'
+    assert histlist[2] == 'alias create s shortcuts\n'