Rewrote Cmd2History to only accept completed commands.

Author: kmvanbrunt

cmd2/cmd2.py

@@ -672,7 +672,7 @@ def _(event: Any) -> None:  # pragma: no cover
             "complete_in_thread": True,
             "complete_while_typing": False,
             "completer": Cmd2Completer(self),
-            "history": Cmd2History(self),
+            "history": Cmd2History(item.raw for item in self.history),
             "key_bindings": key_bindings,
             "lexer": Cmd2Lexer(self),
             "rprompt": self.get_rprompt,
@@ -2902,6 +2902,9 @@ def _complete_statement(self, line: str) -> Statement:
         if not statement.command:
             raise EmptyStatement
 
+        # Add the complete command to prompt-toolkit's history.
+        cast(Cmd2History, self.main_session.history).add_command(statement.raw)
+
         return statement
 
     def _input_line_to_statement(self, line: str) -> Statement:
@@ -5002,6 +5005,7 @@ def do_history(self, args: argparse.Namespace) -> bool | None:
 
             # Clear command and prompt-toolkit history
             self.history.clear()
+            cast(Cmd2History, self.main_session.history).clear()
 
             if self.persistent_history_file:
                 try:

cmd2/pt_utils.py

@@ -152,41 +152,46 @@ def get_completions(self, document: Document, _complete_event: object) -> Iterab
 
 
 class Cmd2History(History):
-    """History that bridges cmd2's history storage with prompt_toolkit."""
+    """An in-memory prompt-toolkit History implementation designed for cmd2.
 
-    def __init__(self, cmd_app: 'Cmd') -> None:
-        """Initialize prompt_toolkit based history wrapper class."""
+    This class gives cmd2 total control over what appears in the up-arrow
+    history, preventing multiline fragments from appearing in the navigation.
+    """
+
+    def __init__(self, history_strings: Iterable[str] | None = None) -> None:
+        """Initialize the instance."""
         super().__init__()
-        self.cmd_app = cmd_app
 
-    def load_history_strings(self) -> Iterable[str]:
-        """Yield strings from cmd2's history to prompt_toolkit."""
-        for item in self.cmd_app.history:
-            yield item.statement.raw
-
-    def get_strings(self) -> list[str]:
-        """Get the strings from the history."""
-        # We override this to always get the latest history from cmd2
-        # instead of caching it like the base class does.
-        strings: list[str] = []
-        last_item = None
-        for item in self.cmd_app.history:
-            if item.statement.raw != last_item:
-                strings.append(item.statement.raw)
-                last_item = item.statement.raw
-        return strings
+        if history_strings:
+            # Use add_command() to filter consecutive duplicates
+            # and save history strings from newest to oldest.
+            for string in history_strings:
+                self.add_command(string)
 
-    def store_string(self, string: str) -> None:
-        """prompt_toolkit calls this when a line is accepted.
+        # Mark that self._loaded_strings is loaded.
+        self._loaded = True
 
-        cmd2 handles history addition in its own loop (postcmd).
-        We don't want to double add.
-        However, PromptSession needs to know about it for the *current* session history navigation.
-        If we don't store it here, UP arrow might not work for the just entered command
-        unless cmd2 re-initializes the session or history object.
+    def add_command(self, string: str) -> None:
+        """Manually add a finalized command string to the UI history stack.
 
-        This method is intentionally empty.
+        Ensures consecutive duplicates are not stored.
         """
+        if string and (not self._loaded_strings or self._loaded_strings[0] != string):
+            super().append_string(string)
+
+    def append_string(self, string: str) -> None:
+        """No-op: Blocks prompt-toolkit from storing multiline fragments."""
+
+    def store_string(self, string: str) -> None:
+        """No-op: Persistent history data is stored in cmd_app.history."""
+
+    def load_history_strings(self) -> Iterable[str]:
+        """Yield strings from newest to oldest."""
+        yield from self._loaded_strings
+
+    def clear(self) -> None:
+        """Clear the UI history navigation data."""
+        self._loaded_strings.clear()
 
 
 class Cmd2Lexer(Lexer):

tests/test_pt_utils.py

@@ -20,8 +20,6 @@
 )
 from cmd2 import rich_utils as ru
 from cmd2 import string_utils as su
-from cmd2.history import HistoryItem
-from cmd2.parsing import Statement
 from cmd2.pt_utils import pt_filter_style
 
 from .conftest import with_ansi_style
@@ -34,7 +32,6 @@ def __init__(self) -> None:
         self.complete = Mock(return_value=cmd2.Completions())
 
         self.always_show_hint = False
-        self.history = []
         self.statement_parser = Mock()
         self.statement_parser.terminators = [';']
         self.statement_parser.shortcuts = []
@@ -506,68 +503,84 @@ def test_get_completions_custom_delimiters(self, mock_cmd_app: MockCmd) -> None:
 
 
 class TestCmd2History:
-    def make_history_item(self, text):
-        statement = Mock(spec=Statement)
-        statement.raw = text
-        item = Mock(spec=HistoryItem)
-        item.statement = statement
-        return item
-
-    def test_load_history_strings(self, mock_cmd_app):
-        """Test loading history strings yields all items in forward order."""
-        history = pt_utils.Cmd2History(cast(Any, mock_cmd_app))
-
-        # Set up history items
-        # History in cmd2 is oldest to newest
-        items = [
-            self.make_history_item("cmd1"),
-            self.make_history_item("cmd2"),
-            self.make_history_item("cmd2"),  # Duplicate
-            self.make_history_item("cmd3"),
-        ]
-        mock_cmd_app.history = items
+    def test_load_history_strings(self):
+        """Test loading history strings yields all items newest to oldest."""
 
-        # Expected: cmd1, cmd2, cmd2, cmd3 (raw iteration)
-        result = list(history.load_history_strings())
+        history_strings = ["cmd1", "cmd2", "cmd2", "cmd3", "cmd2"]
+        history = pt_utils.Cmd2History(history_strings)
+        assert history._loaded
 
-        assert result == ["cmd1", "cmd2", "cmd2", "cmd3"]
+        # Consecutive duplicates are removed
+        expected = ["cmd2", "cmd3", "cmd2", "cmd1"]
+        assert list(history.load_history_strings()) == expected
 
-    def test_load_history_strings_empty(self, mock_cmd_app):
+    def test_load_history_strings_empty(self):
         """Test loading history strings with empty history."""
-        history = pt_utils.Cmd2History(cast(Any, mock_cmd_app))
-
-        mock_cmd_app.history = []
-
-        result = list(history.load_history_strings())
+        history = pt_utils.Cmd2History()
+        assert history._loaded
+        assert list(history.load_history_strings()) == []
+
+        history = pt_utils.Cmd2History([])
+        assert history._loaded
+        assert list(history.load_history_strings()) == []
+
+        history = pt_utils.Cmd2History(None)
+        assert history._loaded
+        assert list(history.load_history_strings()) == []
+
+    def test_get_strings(self):
+        history_strings = ["cmd1", "cmd2", "cmd2", "cmd3", "cmd2"]
+        history = pt_utils.Cmd2History(history_strings)
+        assert history._loaded
+
+        # Consecutive duplicates are removed
+        expected = ["cmd1", "cmd2", "cmd3", "cmd2"]
+        assert history.get_strings() == expected
+
+    def test_append_string(self):
+        """Test that append_string() does nothing."""
+        history = pt_utils.Cmd2History()
+        assert history._loaded
+        assert not history._loaded_strings
+
+        history.append_string("new command")
+        assert not history._loaded_strings
+
+    def test_store_string(self):
+        """Test that store_string() does nothing."""
+        history = pt_utils.Cmd2History()
+        assert history._loaded
+        assert not history._loaded_strings
 
-        assert result == []
-
-    def test_get_strings(self, mock_cmd_app):
-        """Test get_strings returns deduped strings and does not cache."""
-        history = pt_utils.Cmd2History(cast(Any, mock_cmd_app))
-
-        items = [
-            self.make_history_item("cmd1"),
-            self.make_history_item("cmd2"),
-            self.make_history_item("cmd2"),  # Duplicate
-            self.make_history_item("cmd3"),
-        ]
-        mock_cmd_app.history = items
-
-        # Expect deduped: cmd1, cmd2, cmd3
-        strings = history.get_strings()
-        assert strings == ["cmd1", "cmd2", "cmd3"]
-
-        # Modify underlying history to prove it does NOT use cache
-        mock_cmd_app.history.append(self.make_history_item("cmd4"))
-        strings2 = history.get_strings()
-        assert strings2 == ["cmd1", "cmd2", "cmd3", "cmd4"]
-
-    def test_store_string(self, mock_cmd_app):
-        """Test store_string does nothing."""
-        history = pt_utils.Cmd2History(cast(Any, mock_cmd_app))
-
-        # Just ensure it doesn't raise error or modify cmd2 history
         history.store_string("new command")
-
-        assert len(mock_cmd_app.history) == 0
+        assert not history._loaded_strings
+
+    def test_add_command(self):
+        """Test that add_command() adds data."""
+        history = pt_utils.Cmd2History()
+        assert history._loaded
+        assert not history._loaded_strings
+
+        history.add_command("new command")
+        assert len(history._loaded_strings) == 1
+        assert history._loaded_strings[0] == "new command"
+
+        # Show that consecutive duplicates are filtered
+        history.add_command("new command")
+        assert len(history._loaded_strings) == 1
+        assert history._loaded_strings[0] == "new command"
+
+        # Show that new items are placed at the front
+        history.add_command("even newer command")
+        assert len(history._loaded_strings) == 2
+        assert history._loaded_strings[0] == "even newer command"
+        assert history._loaded_strings[1] == "new command"
+
+    def test_clear(self):
+        history_strings = ["cmd1", "cmd2"]
+        history = pt_utils.Cmd2History(history_strings)
+        assert history._loaded
+        assert history.get_strings() == history_strings
+
+        history.clear()
+        assert not history.get_strings()