Resolve PR feedback

Author: kotfu

CHANGELOG.md

@@ -33,9 +33,7 @@
     * 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.
+      It is now application developer's responsibility to treat the return value from `cmdloop()` accordingly
     * 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`

cmd2/cmd2.py

@@ -3409,9 +3409,9 @@ def do_history(self, args: argparse.Namespace) -> None:
                             traceback_war=False)
             else:
                 for runme in history:
-                    self.pfeedback(runme.statement.raw)
+                    self.pfeedback(runme.raw)
                     if runme:
-                        self.onecmd_plus_hooks(runme.statement.raw)
+                        self.onecmd_plus_hooks(runme.raw)
         elif args.edit:
             import tempfile
             fd, fname = tempfile.mkstemp(suffix='.txt', text=True)
@@ -3420,7 +3420,7 @@ def do_history(self, args: argparse.Namespace) -> None:
                     if command.statement.multiline_command:
                         fobj.write('{}\n'.format(command.expanded.rstrip()))
                     else:
-                        fobj.write('{}\n'.format(command))
+                        fobj.write('{}\n'.format(command.raw))
             try:
                 self.do_edit(fname)
                 self.do_load(fname)
@@ -3433,9 +3433,9 @@ def do_history(self, args: argparse.Namespace) -> None:
                 with open(os.path.expanduser(args.output_file), 'w') as fobj:
                     for item in history:
                         if item.statement.multiline_command:
-                            fobj.write('{}\n'.format(item.statement.expanded_command_line.rstrip()))
+                            fobj.write('{}\n'.format(item.expanded.rstrip()))
                         else:
-                            fobj.write('{}\n'.format(item.statement.raw))
+                            fobj.write('{}\n'.format(item.raw))
                 plural = 's' if len(history) > 1 else ''
                 self.pfeedback('{} command{} saved to {}'.format(len(history), plural, args.output_file))
             except Exception as e:
@@ -3492,9 +3492,9 @@ def _initialize_history(self, hist_file):
             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
+                if item.raw != last:
+                    readline.add_history(item.raw)
+                    last = item.raw
 
         # register a function to write history at save
         # if the history file is in plain text format from 0.9.12 or lower
@@ -3549,7 +3549,7 @@ def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcrip
                 first = True
                 command = ''
                 if isinstance(history_item, HistoryItem):
-                    history_item = history_item.statement.raw
+                    history_item = history_item.raw
                 for line in history_item.splitlines():
                     if first:
                         command += '{}{}\n'.format(self.prompt, line)

cmd2/history.py

@@ -7,29 +7,31 @@
 
 from typing import List, Union
 
+import attr
+
 from . import utils
 from .parsing import Statement
 
-
+@attr.s(frozen=True)
 class HistoryItem():
     """Class used to represent one command in the History list"""
     _listformat = ' {:>4}  {}\n'
     _ex_listformat = ' {:>4}x {}\n'
 
-    # def __new__(cls, statement: Statement):
-    #     """Create a new instance of HistoryItem
+    statement = attr.ib(default=None, validator=attr.validators.instance_of(Statement))
+    idx = attr.ib(default=None, validator=attr.validators.instance_of(int))
 
-    #     We must override __new__ because we are subclassing `str` which is
-    #     immutable and takes a different number of arguments as Statement.
-    #     """
-    #     hi = super().__new__(cls, statement.raw)
-    #     hi.statement = statement
-    #     hi.idx = None
-    #     return hi
+    def __str__(self):
+        """A convenient human readable representation of the history item"""
+        if self.statement:
+            return self.statement.raw
+        else:
+            return ''
 
-    def __init__(self, statement: Statement, idx: int):
-        self.statement = statement
-        self.idx = idx
+    @property
+    def raw(self) -> str:
+        """Return the raw input from the user for this item"""
+        return self.statement.raw
 
     @property
     def expanded(self) -> str:
@@ -43,24 +45,23 @@ def pr(self, script=False, expanded=False, verbose=False) -> str:
 
         :return: pretty print string version of a HistoryItem
         """
-        expanded_command = self.statement.expanded_command_line
         if verbose:
-            ret_str = self._listformat.format(self.idx, self.statement.raw)
-            if self.statement.raw != expanded_command.rstrip():
-                ret_str += self._ex_listformat.format(self.idx, expanded_command)
+            ret_str = self._listformat.format(self.idx, self.raw)
+            if self.raw != self.expanded.rstrip():
+                ret_str += self._ex_listformat.format(self.idx, self.expanded)
         else:
             if script:
                 # display without entry numbers
                 if expanded or self.statement.multiline_command:
-                    ret_str = expanded_command.rstrip()
+                    ret_str = self.expanded.rstrip()
                 else:
-                    ret_str = self.statement.raw
+                    ret_str = self.raw.rstrip()
             else:
                 # display a numbered list
                 if expanded or self.statement.multiline_command:
-                    ret_str = self._listformat.format(self.idx, expanded_command.rstrip())
+                    ret_str = self._listformat.format(self.idx, self.expanded.rstrip())
                 else:
-                    ret_str = self._listformat.format(self.idx, self.statement.raw.rstrip())
+                    ret_str = self._listformat.format(self.idx, self.raw.rstrip())
         return ret_str
 
 
@@ -210,8 +211,8 @@ def str_search(self, search: str) -> List[HistoryItem]:
         def isin(history_item):
             """filter function for string search of history"""
             sloppy = utils.norm_fold(search)
-            inraw = sloppy in utils.norm_fold(history_item.statement.raw)
-            inexpanded = sloppy in utils.norm_fold(history_item.statement.expanded_command_line)
+            inraw = sloppy in utils.norm_fold(history_item.raw)
+            inexpanded = sloppy in utils.norm_fold(history_item.expanded)
             return inraw or inexpanded
         return [item for item in self if isin(item)]
 
@@ -228,7 +229,7 @@ def regex_search(self, regex: str) -> List[HistoryItem]:
 
         def isin(hi):
             """filter function for doing a regular expression search of history"""
-            return finder.search(hi.statement.raw) or finder.search(hi.statement.expanded_command_line)
+            return finder.search(hi.raw) or finder.search(hi.expanded)
         return [itm for itm in self if isin(itm)]
 
     def truncate(self, max_length: int) -> None:

tests/test_history.py

@@ -459,14 +459,11 @@ def hist_file():
         pass
 
 def test_bad_history_file_path(capsys, request):
-    # Use a directory path as the history file
-    test_dir = os.path.dirname(request.module.__file__)
-
-    # Create a new cmd2 app
-    cmd2.Cmd(persistent_history_file=test_dir)
-    _, err = capsys.readouterr()
-
-    assert 'is a directory' in err
+    with tempfile.TemporaryDirectory() as test_dir:
+        # Create a new cmd2 app
+        cmd2.Cmd(persistent_history_file=test_dir)
+        _, err = capsys.readouterr()
+        assert 'is a directory' 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