Populate readline history from unpickled history

Author: kotfu

CHANGELOG.md

@@ -42,6 +42,8 @@
       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.
+    * HistoryItem class is no longer a subclass of `str`. If you are directly accessing the `.history` attribute
+      of a `cmd2` based app, you will need to update your code to use `.history.get(1).statement.raw` instead.
 * **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

cmd2/cmd2.py

@@ -404,9 +404,8 @@ def __init__(self, completekey: str = 'tab', stdin=None, stdout=None, persistent
 
         # Commands to exclude from the history command
         # initialize history
-        self.persistent_history_file = persistent_history_file
         self.persistent_history_length = persistent_history_length
-        self._initialize_history()
+        self._initialize_history(persistent_history_file)
         self.exclude_from_history = '''history edit eof eos'''.split()
 
         # Command aliases and macros
@@ -3448,7 +3447,7 @@ def do_history(self, args: argparse.Namespace) -> None:
             for hi in history:
                 self.poutput(hi.pr(script=args.script, expanded=args.expanded, verbose=args.verbose))
 
-    def _initialize_history(self):
+    def _initialize_history(self, hist_file):
         """Initialize history using history related attributes
 
         This function can determine whether history is saved in the prior text-based
@@ -3462,43 +3461,46 @@ def _initialize_history(self):
         """
         self.history = History()
         # with no persistent history, nothing else in this method is relevant
-        if not self.persistent_history_file:
+        if not hist_file:
+            self.persistent_history_file = hist_file
             return
 
-        self.persistent_history_file = os.path.expanduser(self.persistent_history_file)
+        hist_file = os.path.expanduser(hist_file)
 
         # first we try and unpickle the history file
         history = History()
         try:
-            with open(self.persistent_history_file, 'rb') as fobj:
+            with open(hist_file, 'rb') as fobj:
                 history = pickle.load(fobj)
         except (FileNotFoundError, KeyError, EOFError):
             pass
         except IsADirectoryError:
             msg = "persistent history file '{}' is a directory"
-            self.perror(msg.format(self.persistent_history_file))
+            self.perror(msg.format(hist_file))
+            return
         except OSError as ex:
             msg = "can not read persistent history file '{}': {}"
-            self.perror(msg.format(self.persistent_history_file, ex), traceback_war=False)
+            self.perror(msg.format(hist_file, ex), traceback_war=False)
+            return
 
         self.history = history
-
-        # 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)
+        self.persistent_history_file = hist_file
+
+        # populate readline history
+        if rl_type != RlType.NONE:
+            last = None
+            for item in history:
+                # readline only adds a single entry for multiple sequential identical commands
+                # so we emulate that behavior here
+                if item.statement.raw != last:
+                    readline.add_history(item.statement.raw)
+                    last = item.statement.raw
 
         # 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(self._persist_history_on_exit)
+        import atexit
+        atexit.register(self._persist_history_on_exit)
 
     def _persist_history_on_exit(self):
         """write history out to the history file"""
@@ -3509,7 +3511,6 @@ def _persist_history_on_exit(self):
         try:
             with open(self.persistent_history_file, 'wb') as fobj:
                 pickle.dump(self.history, fobj)
-
         except OSError as ex:
             msg = "can not write persistent history file '{}': {}"
             self.perror(msg.format(self.persistent_history_file, ex), traceback_war=False)

tests/test_history.py

@@ -458,46 +458,6 @@ def hist_file():
     except FileNotFoundError:
         pass
 
-def test_existing_history_file(hist_file, capsys):
-
-    # Create the history file before making cmd2 app
-    with open(hist_file, 'w'):
-        pass
-
-    # Create a new cmd2 app
-    cmd2.Cmd(persistent_history_file=hist_file)
-    _, err = capsys.readouterr()
-
-    # Make sure there were no errors
-    assert err == ''
-
-    # Unregister the call to write_history_file that cmd2 did
-    ## TODO atexit.unregister(readline.write_history_file)
-
-    # Remove created history file
-    #os.remove(hist_file)
-
-def test_new_history_file(hist_file, capsys):
-
-    # Remove any existing history file
-    try:
-        os.remove(hist_file)
-    except OSError:
-        pass
-
-    # Create a new cmd2 app
-    cmd2.Cmd(persistent_history_file=hist_file)
-    _, err = capsys.readouterr()
-
-    # Make sure there were no errors
-    assert err == ''
-
-    # Unregister the call to write_history_file that cmd2 did
-    ### TODO atexit.unregister(readline.write_history_file)
-
-    # Remove created history file
-    #os.remove(hist_file)
-
 def test_bad_history_file_path(capsys, request):
     # Use a directory path as the history file
     test_dir = os.path.dirname(request.module.__file__)
@@ -531,3 +491,31 @@ def test_history_file_conversion_no_truncate_on_init(hist_file, capsys):
     assert histlist[0]== 'help\n'
     assert histlist[1] == 'alias\n'
     assert histlist[2] == 'alias create s shortcuts\n'
+
+def test_history_populates_readline(hist_file):
+    # - create a cmd2 with persistent history
+    app = cmd2.Cmd(persistent_history_file=hist_file)
+    run_cmd(app, 'help')
+    run_cmd(app, 'shortcuts')
+    run_cmd(app, 'shortcuts')
+    run_cmd(app, 'alias')
+
+    # call the private method which is registered to write history at exit
+    app._persist_history_on_exit()
+    # - create a new cmd2 with persistent history
+    app = cmd2.Cmd(persistent_history_file=hist_file)
+
+    assert len(app.history) == 4
+    assert app.history.get(1).statement.raw == 'help'
+    assert app.history.get(2).statement.raw == 'shortcuts'
+    assert app.history.get(3).statement.raw == 'shortcuts'
+    assert app.history.get(4).statement.raw == 'alias'
+
+    # readline only adds a single entry for multiple sequential identical commands
+    # so we check to make sure that cmd2 populated the readline history
+    # using the same rules
+    from cmd2.rl_utils import readline
+    assert readline.get_current_history_length() == 3
+    assert readline.get_history_item(1) == 'help'
+    assert readline.get_history_item(2) == 'shortcuts'
+    assert readline.get_history_item(3) == 'alias'