diff --git a/CHANGELOG.md b/CHANGELOG.md index 21fa3a869..eab98958b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,27 @@ +## 4.4.0 (TBD) + +- Breaking Changes + - `History.get()` and `History.span()` now raise `HistoryIndexError`, an `IndexError` subclass, + for invalid indices and bounded ranges selecting no history items. Unbounded ranges (`:` and + `..`) can still return an empty dictionary, and negative starts still clamp to the beginning + of history. Malformed ranges, including zero endpoints, raise `HistoryFormatError`, a + `ValueError` subclass. The `history` command reports these selection errors without a + traceback hint, and index zero includes the reminder that the first command is command 1. + - End-only ranges now start at command 1 in the full history, including persisted commands, just + like explicit-start ranges and single indices. Bare `history` and unbounded ranges still + select only the current session unless `--all` is supplied. + - Running, editing, or saving an empty history selection now fails before replay, opening an + editor, or creating or overwriting a file. This includes `history -o FILE` and `history -e` + with no selector in an empty session, and actions on searches with no matches. Empty unbounded + selections and search misses report `No matching history items` for these actions; displaying + them still succeeds without output. The no-argument `history --run` safety refusal is + unchanged. +- Bug Fixes + - Fixed history ranges starting past the end selecting the last command. Invalid indices and + bounded ranges selecting no items report `No history items in range` before running, editing, + or saving commands. + - Negative single-index history selections now display and return the positive command number. + ## 4.3.1 (TBD) - Bug Fixes diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 94138cb32..d6b8e7adc 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -153,7 +153,10 @@ SkipPostcommandHooks, ) from .history import ( + NO_MATCHING_HISTORY_ITEMS, History, + HistoryFormatError, + HistoryIndexError, HistoryItem, ) from .parsing import ( @@ -5498,18 +5501,23 @@ def do_history(self, args: argparse.Namespace) -> bool | None: return None + if args.run and not args.arg: + self.perror("Cowardly refusing to run all previously entered commands.") + self.perror("If this is what you want to do, specify '1:' as the range of history.") + return None + # If an argument was supplied, then retrieve partial contents of the history, otherwise retrieve it all - history = self._get_history(args) + try: + history = self._get_history(args) + except (HistoryIndexError, HistoryFormatError) as ex: + self.perror(str(ex)) + return None if args.run: - if not args.arg: - self.perror("Cowardly refusing to run all previously entered commands.") - self.perror("If this is what you want to do, specify '1:' as the range of history.") - else: - stop = self.runcmds_plus_hooks(list(history.values())) - self.last_result = True - return stop - elif args.edit: + stop = self.runcmds_plus_hooks(list(history.values())) + self.last_result = True + return stop + if args.edit: fd, fname = tempfile.mkstemp(suffix=".txt", text=True) fobj: TextIO with os.fdopen(fd, "w") as fobj: @@ -5550,14 +5558,18 @@ def do_history(self, args: argparse.Namespace) -> bool | None: def _get_history(self, args: argparse.Namespace) -> dict[int, HistoryItem]: """If an argument was supplied, then retrieve partial contents of the history; otherwise retrieve entire history. - This function returns a dictionary with history items keyed by their 1-based index in ascending order. + This function returns a dictionary with history items keyed by their positive 1-based index in ascending order. + Selection errors, including empty selections for actions, propagate to the caller. """ if args.arg: try: int_arg = int(args.arg) - return {int_arg: self.history.get(int_arg)} except ValueError: pass + else: + item = self.history.get(int_arg) + index = len(self.history) + int_arg + 1 if int_arg < 0 else int_arg + return {index: item} if ".." in args.arg or ":" in args.arg: # Get a slice of history @@ -5569,6 +5581,10 @@ def _get_history(self, args: argparse.Namespace) -> dict[int, HistoryItem]: else: # Get a copy of the history so it doesn't get mutated while we are using it history = self.history.span(":", args.all) + + # An empty display is harmless, but actions must have something to run, edit, or save. + if not history and (args.run or args.edit or args.output_file): + raise HistoryIndexError(NO_MATCHING_HISTORY_ITEMS) return history def _initialize_history(self, hist_file: str) -> None: diff --git a/cmd2/history.py b/cmd2/history.py index c99a8aac5..47602c641 100644 --- a/cmd2/history.py +++ b/cmd2/history.py @@ -18,6 +18,11 @@ shlex_split, ) +NO_HISTORY_ITEMS = "No history items in range" +NO_MATCHING_HISTORY_ITEMS = "No matching history items" +FIRST_HISTORY_INDEX = "The first command in history is command 1." +INVALID_HISTORY_RANGE = "History indices must be positive or negative integers, and may not be zero." + def single_line_format(statement: Statement) -> str: """Format a command line to display on a single line. @@ -131,6 +136,14 @@ def from_dict(source_dict: dict[str, Any]) -> "HistoryItem": return HistoryItem(Statement.from_dict(statement_dict)) +class HistoryIndexError(IndexError): + """Raised when a history index is invalid or a bounded range selects no items.""" + + +class HistoryFormatError(ValueError): + """Raised when a history range has invalid syntax or a zero endpoint.""" + + class History(list[HistoryItem]): """A list of [HistoryItem][cmd2.history.HistoryItem] objects with additional methods for searching and managing the list. @@ -190,9 +203,12 @@ def get(self, index: int) -> HistoryItem: :param index: optional item to get :return: a single [cmd2.history.HistoryItem][] + :raises HistoryIndexError: if index is zero or outside the history """ if index == 0: - raise IndexError("The first command in history is command 1.") + raise HistoryIndexError(f"{NO_HISTORY_ITEMS}. {FIRST_HISTORY_INDEX}") + if abs(index) > len(self): + raise HistoryIndexError(NO_HISTORY_ITEMS) if index < 0: return self[index] return self[index - 1] @@ -227,9 +243,12 @@ def span(self, span: str, include_persisted: bool = False) -> dict[int, "History """Return a slice of the History list. :param span: string containing an index or a slice - :param include_persisted: if True, then retrieve full results including from persisted history + :param include_persisted: if True, include persisted history for unbounded ranges. + Ranges with either endpoint always use the full history. :return: a dictionary of history items keyed by their 1-based index in ascending order, - or an empty dictionary if no results were found + or an empty dictionary if an unbounded range (``:`` or ``..``) has no results + :raises HistoryFormatError: if the range format is invalid or an index is zero + :raises HistoryIndexError: if a range with an explicit start or end selects no history items This method can accommodate input in any of these forms: @@ -252,17 +271,17 @@ def span(self, span: str, include_persisted: bool = False) -> dict[int, "History results = self.spanpattern.search(span) if not results: # our regex doesn't match the input, bail out - raise ValueError("History indices must be positive or negative integers, and may not be zero.") + raise HistoryFormatError(f"{INVALID_HISTORY_RANGE} {FIRST_HISTORY_INDEX}") start_token = results.group("start") + end_token = results.group("end") if start_token: - start = min(self._zero_based_index(start_token), len(self) - 1) + start = self._zero_based_index(start_token) if start < 0: start = max(0, len(self) + start) else: - start = 0 if include_persisted else self.session_start_index + start = 0 if include_persisted or end_token else self.session_start_index - end_token = results.group("end") if end_token: end = min(int(end_token), len(self)) if end < 0: @@ -270,6 +289,9 @@ def span(self, span: str, include_persisted: bool = False) -> dict[int, "History else: end = len(self) + if (start_token or end_token) and (start >= len(self) or end <= start): + raise HistoryIndexError(NO_HISTORY_ITEMS) + return self._build_result_dictionary(start, end) def str_search(self, search: str, include_persisted: bool = False) -> dict[int, "HistoryItem"]: diff --git a/docs/features/history.md b/docs/features/history.md index c9ece9765..6580d4329 100644 --- a/docs/features/history.md +++ b/docs/features/history.md @@ -87,8 +87,10 @@ separated by `..` or `:`, and you will see all commands between, and including, 2 alias create two !echo two 3 alias create three !echo three -If you omit the first number, it will start at the beginning. If you omit the last number, it will -continue to the end: +Command numbers refer to the full history, including commands loaded from a persistent history file. +A single number or a range with either endpoint can select those commands without `--all`. If you +omit the first number, the range starts at command 1. If you omit the last number, it continues to +the end of the full history: (Cmd) history :2 1 alias create one !echo one @@ -98,6 +100,11 @@ continue to the end: 3 alias create three !echo three 4 alias create four !echo four +With both endpoints omitted (`:` or `..`), only the current session is selected, as with plain +`history`. Use `--all` to include persisted commands in these unbounded selections or in searches. +For example, after loading five persisted commands into a fresh session, `history :5` and +`history 1:5` both display commands 1 through 5, while `history :` displays nothing. + If you want to display the last three commands entered: (Cmd) history -- -3: @@ -162,11 +169,19 @@ file are executed. To edit and then re-run commands 2-4 you would: If you want to save the commands to a text file, but not edit and re-run them, use the `-o` or `--output-file` option. This is a great way to create [Scripts](./scripting.md), which can be -executed using the `run_script` command. To save the first 5 commands entered in this session to a -text file: +executed using the `run_script` command. To save commands 1 through 5 from the full history, +including persisted commands, to a text file: (Cmd) history :5 -o history.txt +To save only commands entered in the current session, use `history -o history.txt`. Running, +editing, or saving an empty selection reports an error before executing commands, opening an editor, +or creating or overwriting a file. Empty searches report `No matching history items` for these +actions; displaying a search with no matches succeeds without output. The same display behavior +applies to empty unbounded selections. Invalid indices and bounded ranges selecting no commands +report `No history items in range`. Index zero also includes a reminder that the first command is +command 1. Malformed ranges report a format error. + The last action the history command can perform is to clear the command history using `-c` or `--clear`: diff --git a/tests/test_history.py b/tests/test_history.py index ca19d2364..e34e397cb 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -2,6 +2,7 @@ import contextlib import os +import re import tempfile from unittest import ( mock, @@ -10,6 +11,7 @@ import pytest import cmd2 +from cmd2.history import HistoryFormatError, HistoryIndexError from .conftest import ( normalize, @@ -228,6 +230,52 @@ def test_history_class_span(hist) -> None: hist.span(tryit) +@pytest.mark.parametrize("suffix", [":", ":999", ":-1", "..", "..999", "..-1"]) +def test_history_span_past_end(hist, suffix) -> None: + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.span(f"{len(hist) + 1}{suffix}") + + +@pytest.mark.parametrize( + ("span", "indices"), + [("4:", [4]), ("1:999", [1, 2, 3, 4]), ("-999:-1", [1, 2, 3, 4])], +) +def test_history_span_boundaries(hist, span, indices) -> None: + assert list(hist.span(span)) == indices + + +@pytest.mark.parametrize("span", ["999:1", "3:1", "2:-4", "1:-999"]) +def test_history_span_explicit_start_without_matches(hist, span) -> None: + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.span(span) + + +@pytest.mark.parametrize("span", ["1:", "-1:"]) +def test_history_span_explicit_start_on_empty_history(hist, span) -> None: + hist.clear() + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.span(span) + + +@pytest.mark.parametrize("span", [":", ".."]) +@pytest.mark.parametrize("empty_history", [False, True]) +def test_history_span_unbounded_without_matches(hist, span, empty_history) -> None: + if empty_history: + hist.clear() + hist.start_session() + assert hist.span(span) == {} + + +@pytest.mark.parametrize("span", [":-999", "..-999"]) +@pytest.mark.parametrize("empty_history", [False, True]) +def test_history_span_end_only_without_matches(hist, span, empty_history) -> None: + if empty_history: + hist.clear() + hist.start_session() + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.span(span) + + def test_persisted_history_span(persisted_hist) -> None: span = persisted_hist.span("2..") assert len(span) == 5 @@ -269,8 +317,8 @@ def test_persisted_history_span(persisted_hist) -> None: assert span[5].statement.raw == "fifth" span = persisted_hist.span(":-2", include_persisted=False) - assert len(span) == 1 - assert span[5].statement.raw == "fifth" + assert list(span) == [1, 2, 3, 4, 5] + assert span == persisted_hist.span("1:-2") value_errors = ["fred", "fred:joe", "2", "-2", "a..b", "2 ..", "1 : 3", "1:0", "0:3"] expected_err = "History indices must be positive or negative integers, and may not be zero." @@ -284,14 +332,33 @@ def test_history_class_get(hist) -> None: assert hist.get(3).statement.raw == "third" assert hist.get(-2) == hist[-2] assert hist.get(-1).statement.raw == "fourth" + assert hist.get(len(hist)) == hist[-1] + assert hist.get(-len(hist)) == hist[0] - with pytest.raises(IndexError): + with pytest.raises(HistoryIndexError, match="No history items in range"): hist.get(0) - with pytest.raises(IndexError): + with pytest.raises(HistoryIndexError, match="No history items in range"): hist.get(5) +@pytest.mark.parametrize("index", [0, 5, -5, 10**100, -(10**100)]) +@pytest.mark.parametrize("empty_history", [False, True]) +def test_history_get_invalid_index_has_clear_error(hist, index, empty_history) -> None: + if empty_history: + hist.clear() + with pytest.raises(HistoryIndexError, match="No history items in range") as error: + hist.get(index) + assert isinstance(error.value, IndexError) + + +@pytest.mark.parametrize("index", [1, -1]) +def test_history_get_on_empty_history(hist, index) -> None: + hist.clear() + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.get(index) + + def test_history_str_search(hist) -> None: items = hist.str_search("ir") assert len(items) == 2 @@ -644,13 +711,502 @@ def test_history_with_span_end(base_app) -> None: verify_hi_last_result(base_app, 2) +# Each of these selects nothing when history holds a single item +EMPTY_HISTORY_SPANS = ["2", "5", "999", "-2", "2:", "2:3", "2..", "2..3", "2:1", ":-999"] + + +@pytest.mark.parametrize( + ("selector", "empty_selection"), [("", False), (":", False), ("..", False), (":-999", True), ("..-999", True)] +) +@pytest.mark.parametrize("empty_history", [False, True]) +def test_history_selections_on_fresh_session(tmp_path, selector, empty_selection, empty_history) -> None: + history_file = str(tmp_path / "history.dat") + previous_app = cmd2.Cmd(persistent_history_file=history_file) + if not empty_history: + run_cmd(previous_app, "help") + run_cmd(previous_app, "shortcuts") + previous_app._persist_history() + app = cmd2.Cmd(persistent_history_file=history_file) + assert app.history.session_start_index == len(app.history) + + out, err = run_cmd(app, f"history {selector}") + + assert out == [] + if empty_selection: + assert err == ["No history items in range"] + assert app.last_result is False + else: + assert err == [] + assert app.last_result == {} + + +@pytest.mark.parametrize("action", ["--run", "--edit", "--output-file"]) +@pytest.mark.parametrize( + ("selector", "empty_history", "include_persisted"), + [ + (selector, empty_history, include_persisted) + for selector in ("", '""', ":", "..", '" : "', '" "', "no_such_command", "/no_such_regex/") + for empty_history in (False, True) + for include_persisted in (False, True) + if empty_history or not include_persisted or selector in ('" "', "no_such_command", "/no_such_regex/") + ], +) +@pytest.mark.parametrize("debug", [False, True]) +def test_history_empty_actions_have_no_side_effects( + base_app, hist, monkeypatch, tmp_path, action, selector, empty_history, include_persisted, debug +) -> None: + base_app.history = hist + base_app.debug = debug + if empty_history: + hist.clear() + hist.start_session() + initial_history = list(hist) + output_file = tmp_path / "out.txt" + output_file.write_text("old contents\n") + missing_file = tmp_path / "missing.txt" + mkstemp_mock = mock.MagicMock(name="mkstemp", wraps=tempfile.mkstemp) + monkeypatch.setattr("cmd2.cmd2.tempfile.mkstemp", mkstemp_mock) + edit_mock = mock.MagicMock(name="run_editor") + monkeypatch.setattr(base_app, "run_editor", edit_mock) + run_script_mock = mock.MagicMock(name="do_run_script") + monkeypatch.setattr(base_app, "do_run_script", run_script_mock) + run_mock = mock.MagicMock(name="runcmds_plus_hooks") + monkeypatch.setattr(base_app, "runcmds_plus_hooks", run_mock) + for target in (output_file, missing_file): + options = f'--output-file "{target}"' if action == "--output-file" else action + all_option = "--all" if include_persisted else "" + _out, err = run_cmd(base_app, f"history {all_option} {options} {selector}") + if action == "--run" and selector in ("", '""'): + assert err == [ + "Cowardly refusing to run all previously entered commands.", + "If this is what you want to do, specify '1:' as the range of history.", + ] + else: + assert err == ["No matching history items"] + assert base_app.last_result is False + + mkstemp_mock.assert_not_called() + edit_mock.assert_not_called() + run_script_mock.assert_not_called() + run_mock.assert_not_called() + assert output_file.read_text() == "old contents\n" + assert not missing_file.exists() + assert list(hist) == initial_history + + +@pytest.mark.parametrize("action", ["--run", "--edit", "--output-file"]) +@pytest.mark.parametrize("selector", [":", "..", '" : "', "first", "/first/"]) +@pytest.mark.parametrize("include_persisted", [False, True]) +def test_history_nonempty_actions_are_preserved( + base_app, hist, monkeypatch, tmp_path, action, selector, include_persisted +) -> None: + base_app.history = hist + if include_persisted: + hist.start_session() + expected = list(hist[:1] if selector in ("first", "/first/") else hist) + expected_text = "".join(f"{item.raw}\n" for item in expected) + output_file = tmp_path / "out.txt" + output_file.write_text("old contents\n") + run_mock = mock.Mock(return_value=False) + monkeypatch.setattr(base_app, "runcmds_plus_hooks", run_mock) + edited = [] + + def fake_editor(fname) -> None: + with open(fname) as script: + edited.append(script.read()) + + monkeypatch.setattr(base_app, "run_editor", fake_editor) + run_script_mock = mock.Mock() + monkeypatch.setattr(base_app, "do_run_script", run_script_mock) + options = f'--output-file "{output_file}"' if action == "--output-file" else action + all_option = "--all" if include_persisted else "" + + _out, err = run_cmd(base_app, f"history {all_option} {options} {selector}") + + assert err == [] + if action == "--run": + run_mock.assert_called_once_with(expected) + assert base_app.last_result is True + elif action == "--edit": + assert edited == [expected_text] + run_script_mock.assert_called_once() + else: + assert output_file.read_text() == expected_text + assert base_app.last_result is True + + +@pytest.mark.parametrize("action", ["--run", "--edit", "--output-file"]) +def test_history_unbounded_empty_action_after_reload(base_app, monkeypatch, tmp_path, action) -> None: + history_file = tmp_path / "history.dat" + base_app.persistent_history_file = str(history_file) + run_cmd(base_app, "help") + base_app._persist_history() + base_app._initialize_history(str(history_file)) + assert len(base_app.history) == base_app.history.session_start_index == 1 + output_file = tmp_path / "out.txt" + output_file.write_text("keep me\n") + mkstemp_mock = mock.Mock(wraps=tempfile.mkstemp) + monkeypatch.setattr("cmd2.cmd2.tempfile.mkstemp", mkstemp_mock) + editor = mock.Mock() + replay = mock.Mock() + monkeypatch.setattr(base_app, "run_editor", editor) + monkeypatch.setattr(base_app, "runcmds_plus_hooks", replay) + options = f'--output-file "{output_file}"' if action == "--output-file" else action + + out, err = run_cmd(base_app, f"history {options} :") + + assert out == [] + assert err == ["No matching history items"] + assert base_app.last_result is False + assert output_file.read_text() == "keep me\n" + mkstemp_mock.assert_not_called() + editor.assert_not_called() + replay.assert_not_called() + + +@pytest.mark.parametrize("span", [":2", "..2", "..-3"]) +def test_history_end_only_with_all_includes_persisted(base_app, hist, span) -> None: + base_app.history = hist + hist.start_session() + + out, err = run_cmd(base_app, f"history --all {span}") + + assert len(out) == 2 + assert err == [] + assert list(base_app.last_result) == [1, 2] + + +@pytest.mark.parametrize( + ("span", "session_start"), + [(span, start) for span in ("3:1", "2:-4", "1:-999", ":-999", "..-999") for start in (0, 2, 4)], +) +@pytest.mark.parametrize("action", ["", "--run", "--edit", "--output-file"]) +@pytest.mark.parametrize("debug", [False, True]) +def test_history_empty_bounded_span_has_no_side_effects( + base_app, hist, monkeypatch, tmp_path, span, session_start, action, debug +) -> None: + base_app.history = hist + hist.session_start_index = session_start + base_app.debug = debug + mkstemp_mock = mock.MagicMock(name="mkstemp") + monkeypatch.setattr("cmd2.cmd2.tempfile.mkstemp", mkstemp_mock) + edit_mock = mock.MagicMock(name="run_editor") + monkeypatch.setattr(base_app, "run_editor", edit_mock) + run_script_mock = mock.MagicMock(name="do_run_script") + monkeypatch.setattr(base_app, "do_run_script", run_script_mock) + run_mock = mock.MagicMock(name="runcmds_plus_hooks") + monkeypatch.setattr(base_app, "runcmds_plus_hooks", run_mock) + existing_file = tmp_path / "existing.txt" + existing_file.write_text("keep me\n") + missing_file = tmp_path / "missing.txt" + + for output_file in (existing_file, missing_file): + options = f'--output-file "{output_file}"' if action == "--output-file" else action + out, err = run_cmd(base_app, f"history {options} {span}") + assert out == [] + assert err == ["No history items in range"] + assert base_app.last_result is False + + mkstemp_mock.assert_not_called() + edit_mock.assert_not_called() + run_script_mock.assert_not_called() + run_mock.assert_not_called() + assert len(base_app.history) == 4 + assert existing_file.read_text() == "keep me\n" + assert not missing_file.exists() + + +@pytest.fixture +def debug_history_app(base_app) -> cmd2.Cmd: + """App whose only history item is 'set debug true', with debug reset so a rerun is detectable""" + run_cmd(base_app, "set debug true") + assert len(base_app.history) == 1 + base_app.debug = False + return base_app + + +@pytest.mark.parametrize("span", EMPTY_HISTORY_SPANS) +def test_history_empty_span(debug_history_app, span) -> None: + out, err = run_cmd(debug_history_app, f"history {span}") + + assert out == [] + assert err == ["No history items in range"] + assert debug_history_app.last_result is False + + +@pytest.mark.parametrize("span", EMPTY_HISTORY_SPANS) +def test_history_run_empty_span(debug_history_app, span) -> None: + out, err = run_cmd(debug_history_app, f"history --run {span}") + + assert out == [] + assert err == ["No history items in range"] + assert debug_history_app.last_result is False + assert debug_history_app.debug is False + assert len(debug_history_app.history) == 1 + + +@pytest.mark.parametrize("span", EMPTY_HISTORY_SPANS) +def test_history_edit_empty_span(debug_history_app, monkeypatch, span) -> None: + mkstemp_mock = mock.MagicMock(name="mkstemp", wraps=tempfile.mkstemp) + monkeypatch.setattr("cmd2.cmd2.tempfile.mkstemp", mkstemp_mock) + edit_mock = mock.MagicMock(name="run_editor") + monkeypatch.setattr("cmd2.Cmd.run_editor", edit_mock) + run_script_mock = mock.MagicMock(name="do_run_script") + monkeypatch.setattr("cmd2.Cmd.do_run_script", run_script_mock) + + out, err = run_cmd(debug_history_app, f"history --edit {span}") + + assert out == [] + assert err == ["No history items in range"] + assert debug_history_app.last_result is False + mkstemp_mock.assert_not_called() + edit_mock.assert_not_called() + run_script_mock.assert_not_called() + assert debug_history_app.debug is False + + +@pytest.mark.parametrize("span", EMPTY_HISTORY_SPANS) +def test_history_output_file_empty_span(debug_history_app, tmp_path, span) -> None: + existing_file = tmp_path / "existing.txt" + existing_file.write_text("keep me\n") + missing_file = tmp_path / "missing.txt" + + for output_file in (existing_file, missing_file): + out, err = run_cmd(debug_history_app, f'history --output-file "{output_file}" {span}') + + assert out == [] + assert err == ["No history items in range"] + assert debug_history_app.last_result is False + + assert existing_file.read_text() == "keep me\n" + assert not missing_file.exists() + + +@pytest.mark.parametrize("action", ["", "--run", "--edit", "--output-file"]) +def test_history_index_past_end_with_three_items(base_app, monkeypatch, tmp_path, action) -> None: + run_cmd(base_app, "help") + run_cmd(base_app, "shortcuts") + run_cmd(base_app, "set debug true") + base_app.debug = False + edit_mock = mock.MagicMock(name="run_editor") + monkeypatch.setattr("cmd2.Cmd.run_editor", edit_mock) + output_file = tmp_path / "existing.txt" + output_file.write_text("keep me\n") + if action == "--output-file": + action = f'--output-file "{output_file}"' + + out, err = run_cmd(base_app, f"history {action} 5") + + assert out == [] + assert err == ["No history items in range"] + assert base_app.last_result is False + assert len(base_app.history) == 3 + assert base_app.debug is False + edit_mock.assert_not_called() + assert output_file.read_text() == "keep me\n" + + +@pytest.mark.parametrize("span", ["1", "-1", "1:", "-1:", "-999:"]) +def test_history_run_valid_span(debug_history_app, span) -> None: + _out, err = run_cmd(debug_history_app, f"history --run -- {span}") + + assert err == [] + assert debug_history_app.last_result is True + assert debug_history_app.debug is True + + +@pytest.mark.parametrize("span", ["1", "-1", "1:", "-1:", "-999:"]) +def test_history_output_file_valid_span(debug_history_app, tmp_path, span) -> None: + output_file = tmp_path / "out.txt" + output_file.write_text("old contents\n") + + _out, err = run_cmd(debug_history_app, f'history --output-file "{output_file}" -- {span}') + + assert err == [] + assert debug_history_app.last_result is True + assert output_file.read_text() == "set debug true\n" + + +@pytest.mark.parametrize("span", ["1", "-1", "1:", "-999:"]) +def test_history_edit_valid_span(debug_history_app, monkeypatch, span) -> None: + edited = [] + + def fake_editor(_self, fname) -> None: + with open(fname) as f: + edited.append(f.read()) + + monkeypatch.setattr("cmd2.Cmd.run_editor", fake_editor) + run_script_mock = mock.MagicMock(name="do_run_script") + monkeypatch.setattr("cmd2.Cmd.do_run_script", run_script_mock) + + _out, err = run_cmd(debug_history_app, f"history --edit -- {span}") + + assert err == [] + assert edited == ["set debug true\n"] + run_script_mock.assert_called_once() + + +@pytest.mark.parametrize("search", ["no_such_command", "/no_such_regex/"]) +def test_history_search_without_match_is_not_an_error(debug_history_app, search) -> None: + out, err = run_cmd(debug_history_app, f"history {search}") + + assert out == [] + assert err == [] + assert debug_history_app.last_result == {} + + +@pytest.mark.parametrize("debug", [False, True]) +@pytest.mark.parametrize("selector", ["0", "-0"]) +def test_history_index_zero_has_clean_error(debug_history_app, debug, selector) -> None: + debug_history_app.debug = debug + out, err = run_cmd(debug_history_app, f"history {selector}") + assert out == [] + assert err == ["No history items in range. The first command in history is command 1."] + assert debug_history_app.last_result is False + + +@pytest.mark.parametrize("selector", ["1", "1:"]) +@pytest.mark.parametrize("debug", [False, True]) +def test_history_does_not_hide_unexpected_index_errors(base_app, hist, monkeypatch, selector, debug) -> None: + base_app.history = hist + base_app.debug = debug + monkeypatch.setattr(type(hist), "__getitem__", mock.Mock(side_effect=IndexError("unexpected lookup failure"))) + + with pytest.raises(IndexError, match="unexpected lookup failure"): + base_app.onecmd(f"history {selector}") + + out, err = run_cmd(base_app, f"history {selector}") + assert out == [] + assert err != ["No history items in range"] + assert any("unexpected lookup failure" in line for line in err) + + def test_history_with_span_index_error(base_app) -> None: run_cmd(base_app, "help") run_cmd(base_app, "help history") run_cmd(base_app, "!ls -hal :") expected_err = "History indices must be positive or negative integers, and may not be zero." - with pytest.raises(ValueError, match=expected_err): - base_app.onecmd('history "hal :"') + out, err = run_cmd(base_app, 'history "hal :"') + assert out == [] + assert " ".join(err) == f"{expected_err} The first command in history is command 1." + assert base_app.last_result is False + + +@pytest.mark.parametrize("separator", [":", ".."]) +@pytest.mark.parametrize("session_start", [0, 2, 4]) +@pytest.mark.parametrize("include_persisted", [False, True]) +@pytest.mark.parametrize(("end", "expected"), [("2", [1, 2]), ("-3", [1, 2]), ("4", [1, 2, 3, 4]), ("999", [1, 2, 3, 4])]) +def test_history_bounded_ranges_use_absolute_indices(hist, separator, session_start, include_persisted, end, expected) -> None: + hist.session_start_index = session_start + assert list(hist.span(f"{separator}{end}", include_persisted)) == expected + assert list(hist.span(f"1{separator}{end}", include_persisted)) == expected + assert list(hist.span(f"-999{separator}{end}", include_persisted)) == expected + assert list(hist.span(separator, include_persisted)) == list(range(1 if include_persisted else session_start + 1, 5)) + + +@pytest.mark.parametrize("selector", [":2", "..2", "1:2", "1..2", ":-1", "..-1", "-2:", "-2.."]) +@pytest.mark.parametrize("action", ["", "--run", "--edit", "--output-file"]) +@pytest.mark.parametrize("include_persisted", [False, True]) +def test_history_bounded_actions_after_reload(base_app, monkeypatch, tmp_path, selector, action, include_persisted) -> None: + base_app.persistent_history_file = str(tmp_path / "history.dat") + run_cmd(base_app, "help") + run_cmd(base_app, "shortcuts") + base_app._persist_history() + base_app._initialize_history(base_app.persistent_history_file) + assert base_app.history.session_start_index == len(base_app.history) == 2 + expected = list(base_app.history) + expected_text = "help\nshortcuts\n" + output_file = tmp_path / "commands.txt" + replay = mock.Mock(return_value=False) + monkeypatch.setattr(base_app, "runcmds_plus_hooks", replay) + edited = [] + + def fake_editor(fname) -> None: + with open(fname) as script: + edited.append(script.read()) + + monkeypatch.setattr(base_app, "run_editor", fake_editor) + run_script = mock.Mock() + monkeypatch.setattr(base_app, "do_run_script", run_script) + options = f'--output-file "{output_file}"' if action == "--output-file" else action + all_option = "--all" if include_persisted else "" + out, err = run_cmd(base_app, f"history {all_option} {options} -- {selector}") + assert err == [] + if action == "--run": + replay.assert_called_once_with(expected) + assert base_app.last_result is True + elif action == "--edit": + assert edited == [expected_text] + run_script.assert_called_once() + elif action == "--output-file": + assert output_file.read_text() == expected_text + assert base_app.last_result is True + else: + assert out == [" 1 help", " 2 shortcuts"] + assert base_app.last_result == {1: expected[0], 2: expected[1]} + + +@pytest.mark.parametrize("selector", ["0", "-0", "0:2", "0:", "1:0", ":0", "-0..2", "..0", "1:2:3", "1..2:3", "a:b"]) +@pytest.mark.parametrize("action", ["", "--run", "--edit", "--output-file"]) +@pytest.mark.parametrize("debug", [False, True]) +def test_history_invalid_selection_has_no_side_effects(base_app, monkeypatch, tmp_path, selector, action, debug) -> None: + run_cmd(base_app, "help") + base_app.debug = debug + original_history = list(base_app.history) + effects = [mock.Mock() for _ in range(4)] + monkeypatch.setattr("cmd2.cmd2.tempfile.mkstemp", effects[0]) + monkeypatch.setattr(base_app, "run_editor", effects[1]) + monkeypatch.setattr(base_app, "do_run_script", effects[2]) + monkeypatch.setattr(base_app, "runcmds_plus_hooks", effects[3]) + existing = tmp_path / "existing.txt" + existing.write_text("keep me\n") + missing = tmp_path / "missing.txt" + for target in (existing, missing): + options = f'--output-file "{target}"' if action == "--output-file" else action + out, err = run_cmd(base_app, f"history {options} -- {selector}") + assert out == [] + message = ( + "No history items in range." + if selector in ("0", "-0") + else ("History indices must be positive or negative integers, and may not be zero.") + ) + assert " ".join(err) == f"{message} The first command in history is command 1." + assert base_app.last_result is False + for effect in effects: + effect.assert_not_called() + assert existing.read_text() == "keep me\n" + assert not missing.exists() + assert list(base_app.history) == original_history + + +@pytest.mark.parametrize("selector", ["0:2", "0:", "1:0", "1:2:3"]) +def test_history_invalid_range_preserves_value_error_api(hist, selector) -> None: + with pytest.raises(HistoryFormatError) as error: + hist.span(selector) + assert isinstance(error.value, ValueError) + + +@pytest.mark.parametrize(("index", "expected"), [(-1, 4), (-2, 3), (-4, 1)]) +@pytest.mark.parametrize("session_start", [0, 2, 4]) +def test_history_negative_index_uses_positive_key(base_app, hist, index, expected, session_start) -> None: + base_app.history = hist + hist.session_start_index = session_start + out, err = run_cmd(base_app, f"history {index}") + assert err == [] + assert out == [f" {expected:>4} {hist[expected - 1].raw}"] + assert base_app.last_result == {expected: hist[expected - 1]} + + +@pytest.mark.parametrize("selector", ["/[/", "no_such_command"]) +def test_history_does_not_hide_search_errors(base_app, monkeypatch, selector) -> None: + if selector == "no_such_command": + monkeypatch.setattr(base_app.history, "str_search", mock.Mock(side_effect=ValueError("unexpected search failure"))) + error = ValueError + else: + error = re.error + with pytest.raises(error): + base_app.onecmd(f"history -r {selector}") + assert base_app.last_result is False def test_history_output_file() -> None: