Refactor HistoryItem to not subclass str

Author: kotfu

cmd2/cmd2.py

@@ -3410,9 +3410,9 @@ def do_history(self, args: argparse.Namespace) -> None:
                             traceback_war=False)
             else:
                 for runme in history:
-                    self.pfeedback(runme)
+                    self.pfeedback(runme.statement.raw)
                     if runme:
-                        self.onecmd_plus_hooks(runme)
+                        self.onecmd_plus_hooks(runme.statement.raw)
         elif args.edit:
             import tempfile
             fd, fname = tempfile.mkstemp(suffix='.txt', text=True)
@@ -3432,11 +3432,11 @@ def do_history(self, args: argparse.Namespace) -> None:
         elif args.output_file:
             try:
                 with open(os.path.expanduser(args.output_file), 'w') as fobj:
-                    for command in history:
-                        if command.statement.multiline_command:
-                            fobj.write('{}\n'.format(command.expanded.rstrip()))
+                    for item in history:
+                        if item.statement.multiline_command:
+                            fobj.write('{}\n'.format(item.statement.expanded_command_line.rstrip()))
                         else:
-                            fobj.write('{}\n'.format(command))
+                            fobj.write('{}\n'.format(item.statement.raw))
                 plural = 's' if len(history) > 1 else ''
                 self.pfeedback('{} command{} saved to {}'.format(len(history), plural, args.output_file))
             except Exception as e:
@@ -3547,6 +3547,8 @@ def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcrip
                 # the command from the output
                 first = True
                 command = ''
+                if isinstance(history_item, HistoryItem):
+                    history_item = history_item.statement.raw
                 for line in history_item.splitlines():
                     if first:
                         command += '{}{}\n'.format(self.prompt, line)

cmd2/history.py

@@ -11,21 +11,25 @@
 from .parsing import Statement
 
 
-class HistoryItem(str):
+class HistoryItem():
     """Class used to represent one command in the History list"""
-    listformat = ' {:>4}  {}\n'
-    ex_listformat = ' {:>4}x {}\n'
+    _listformat = ' {:>4}  {}\n'
+    _ex_listformat = ' {:>4}x {}\n'
 
-    def __new__(cls, statement: Statement):
-        """Create a new instance of HistoryItem
+    # def __new__(cls, statement: Statement):
+    #     """Create a new instance of HistoryItem
 
-        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
+    #     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 __init__(self, statement: Statement, idx: int):
+        self.statement = statement
+        self.idx = idx
 
     @property
     def expanded(self) -> str:
@@ -39,23 +43,24 @@ 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, str(self).rstrip())
-            if self != self.expanded:
-                ret_str += self.ex_listformat.format(self.idx, self.expanded.rstrip())
+            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)
         else:
             if script:
                 # display without entry numbers
                 if expanded or self.statement.multiline_command:
-                    ret_str = self.expanded.rstrip()
+                    ret_str = expanded_command.rstrip()
                 else:
-                    ret_str = str(self)
+                    ret_str = self.statement.raw
             else:
                 # display a numbered list
                 if expanded or self.statement.multiline_command:
-                    ret_str = self.listformat.format(self.idx, self.expanded.rstrip())
+                    ret_str = self._listformat.format(self.idx, expanded_command.rstrip())
                 else:
-                    ret_str = self.listformat.format(self.idx, str(self).rstrip())
+                    ret_str = self._listformat.format(self.idx, self.statement.raw.rstrip())
         return ret_str
 
 
@@ -85,9 +90,8 @@ def append(self, new: Statement) -> None:
 
         :param new: command line to convert to HistoryItem and add to the end of the History list
         """
-        new = HistoryItem(new)
-        list.append(self, new)
-        new.idx = len(self)
+        history_item = HistoryItem(new, len(self)+1)
+        list.append(self, history_item)
 
     def get(self, index: Union[int, str]) -> HistoryItem:
         """Get item from the History list using 1-based indexing.
@@ -206,7 +210,7 @@ def str_search(self, search: str) -> List[HistoryItem]:
         def isin(history_item):
             """filter function for string search of history"""
             sloppy = utils.norm_fold(search)
-            return sloppy in utils.norm_fold(history_item) or sloppy in utils.norm_fold(history_item.expanded)
+            return sloppy in utils.norm_fold(history_item.statement.raw) or sloppy in utils.norm_fold(history_item.statement.expanded_command_line)
         return [item for item in self if isin(item)]
 
     def regex_search(self, regex: str) -> List[HistoryItem]:
@@ -222,7 +226,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) or finder.search(hi.expanded)
+            return finder.search(hi.statement.raw) or finder.search(hi.statement.expanded_command_line)
         return [itm for itm in self if isin(itm)]
 
     def truncate(self, max_length:int) -> None:

tests/test_history.py

@@ -55,44 +55,93 @@ def test_exclude_from_history(base_app, monkeypatch):
 def hist():
     from cmd2.parsing import Statement
     from cmd2.cmd2 import History, HistoryItem
-    h = History([HistoryItem(Statement('', raw='first')),
-                 HistoryItem(Statement('', raw='second')),
-                 HistoryItem(Statement('', raw='third')),
-                 HistoryItem(Statement('', raw='fourth'))])
+    h = History([HistoryItem(Statement('', raw='first'), 1),
+                 HistoryItem(Statement('', raw='second'), 2),
+                 HistoryItem(Statement('', raw='third'), 3),
+                 HistoryItem(Statement('', raw='fourth'),4)])
     return h
 
 def test_history_class_span(hist):
     for tryit in ['*', ':', '-', 'all', 'ALL']:
         assert hist.span(tryit) == hist
 
-    assert hist.span('3') == ['third']
-    assert hist.span('-1') == ['fourth']
-
-    assert hist.span('2..') == ['second', 'third', 'fourth']
-    assert hist.span('2:') == ['second', 'third', 'fourth']
-
-    assert hist.span('-2..') == ['third', 'fourth']
-    assert hist.span('-2:') == ['third', 'fourth']
-
-    assert hist.span('1..3') == ['first', 'second', 'third']
-    assert hist.span('1:3') == ['first', 'second', 'third']
-    assert hist.span('2:-1') == ['second', 'third', 'fourth']
-    assert hist.span('-3:4') == ['second', 'third','fourth']
-    assert hist.span('-4:-2') == ['first', 'second', 'third']
-
-    assert hist.span(':-2') == ['first', 'second', 'third']
-    assert hist.span('..-2') == ['first', 'second', 'third']
+    assert hist.span('3')[0].statement.raw == 'third'
+    assert hist.span('-1')[0].statement.raw == 'fourth'
+
+    span = hist.span('2..')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'second'
+    assert span[1].statement.raw == 'third'
+    assert span[2].statement.raw == 'fourth'
+
+    span = hist.span('2:')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'second'
+    assert span[1].statement.raw == 'third'
+    assert span[2].statement.raw == 'fourth'
+
+    span = hist.span('-2..')
+    assert len(span) == 2
+    assert span[0].statement.raw == 'third'
+    assert span[1].statement.raw == 'fourth'
+
+    span = hist.span('-2:')
+    assert len(span) == 2
+    assert span[0].statement.raw == 'third'
+    assert span[1].statement.raw == 'fourth'
+
+    span = hist.span('1..3')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'first'
+    assert span[1].statement.raw == 'second'
+    assert span[2].statement.raw == 'third'
+
+    span = hist.span('1:3')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'first'
+    assert span[1].statement.raw == 'second'
+    assert span[2].statement.raw == 'third'
+
+    span = hist.span('2:-1')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'second'
+    assert span[1].statement.raw == 'third'
+    assert span[2].statement.raw == 'fourth'
+
+    span = hist.span('-3:4')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'second'
+    assert span[1].statement.raw == 'third'
+    assert span[2].statement.raw == 'fourth'
+
+    span = hist.span('-4:-2')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'first'
+    assert span[1].statement.raw == 'second'
+    assert span[2].statement.raw == 'third'
+
+    span = hist.span(':-2')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'first'
+    assert span[1].statement.raw == 'second'
+    assert span[2].statement.raw == 'third'
+
+    span = hist.span('..-2')
+    assert len(span) == 3
+    assert span[0].statement.raw == 'first'
+    assert span[1].statement.raw == 'second'
+    assert span[2].statement.raw == 'third'
 
     value_errors = ['fred', 'fred:joe', 'a..b', '2 ..', '1 : 3', '1:0', '0:3']
     for tryit in value_errors:
         with pytest.raises(ValueError):
             hist.span(tryit)
 
 def test_history_class_get(hist):
-    assert hist.get('1') == 'first'
-    assert hist.get(3) == 'third'
+    assert hist.get('1').statement.raw == 'first'
+    assert hist.get(3).statement.raw == 'third'
     assert hist.get('-2') == hist[-2]
-    assert hist.get(-1) == 'fourth'
+    assert hist.get(-1).statement.raw == 'fourth'
 
     with pytest.raises(IndexError):
         hist.get(0)
@@ -115,12 +164,23 @@ def test_history_class_get(hist):
         hist.get(None)
 
 def test_history_str_search(hist):
-    assert hist.str_search('ir') == ['first', 'third']
-    assert hist.str_search('rth') == ['fourth']
+    items = hist.str_search('ir')
+    assert len(items) == 2
+    assert items[0].statement.raw == 'first'
+    assert items[1].statement.raw == 'third'
+
+    items = hist.str_search('rth')
+    assert len(items) == 1
+    assert items[0].statement.raw == 'fourth'
 
 def test_history_regex_search(hist):
-    assert hist.regex_search('/i.*d/') == ['third']
-    assert hist.regex_search('s[a-z]+ond') == ['second']
+    items = hist.regex_search('/i.*d/')
+    assert len(items) == 1
+    assert items[0].statement.raw == 'third'
+
+    items = hist.regex_search('s[a-z]+ond')
+    assert len(items) == 1
+    assert items[0].statement.raw == 'second'
 
 def test_history_max_length_zero(hist):
     hist.truncate(0)
@@ -132,8 +192,9 @@ def test_history_max_length_negative(hist):
 
 def test_history_max_length(hist):
     hist.truncate(2)
-    assert hist.get(1) == 'third'
-    assert hist.get(2) == 'fourth'
+    assert len(hist) == 2
+    assert hist.get(1).statement.raw == 'third'
+    assert hist.get(2).statement.raw == 'fourth'
 
 def test_base_history(base_app):
     run_cmd(base_app, 'help')