diff --git a/CHANGELOG.md b/CHANGELOG.md index 21fa3a869..dbd0534a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,34 @@ +## 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. Starts before the beginning of history are + rejected rather than clamped. 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. + - Replaying or editing a selection containing persisted commands now requires `--all`. This + includes single indices and bounded ranges. Displaying or saving these selections does not + require `--all`, and command numbers are unchanged. + - 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 report `No history items in range`, while search misses report + `No matching history items`; 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. + - History regular expressions containing range separators are now handled as regex searches. + Other selectors are treated as numbers or ranges only when they match the supported syntax; + strings such as URLs, `a:b`, `+1`, and `1_0` remain literal searches. + ## 4.3.1 (TBD) - Bug Fixes diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 94138cb32..de2e140a4 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -153,7 +153,11 @@ SkipPostcommandHooks, ) from .history import ( + NO_HISTORY_ITEMS, + NO_MATCHING_HISTORY_ITEMS, History, + HistoryFormatError, + HistoryIndexError, HistoryItem, ) from .parsing import ( @@ -5446,13 +5450,14 @@ def _build_history_parser(cls) -> Cmd2ArgumentParser: "-a", "--all", action="store_true", - help="display all commands, including ones persisted from previous sessions", + help="include persisted commands; required to run or edit commands from previous sessions", ) history_arg_help = ( - "empty all history items\n" - "a one history item by number\n" - "a..b, a:b, a:, ..b items by indices (inclusive)\n" + "empty current session (--all includes persisted commands)\n" + "a one item by number in the full history\n" + "a..b, a:b, a:, ..b inclusive ranges in the full history\n" + ":, .. current session (--all includes persisted commands)\n" "string items containing string\n" "/regex/ items matching regular expression" ) @@ -5498,18 +5503,33 @@ 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 + + # Selection is independent of the requested action. Empty displays are harmless. + if not history and (args.run or args.edit or args.output_file): + is_search = args.arg and not self.history.spanpattern.fullmatch(args.arg) + self.perror(NO_MATCHING_HISTORY_ITEMS if is_search else NO_HISTORY_ITEMS) + return None + + if (args.run or args.edit) and not args.all and any(index <= self.history.session_start_index for index in history): + self.perror("Use --all to run or edit commands from previous sessions.") + 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,26 +5570,28 @@ 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. + Index and range errors propagate to the caller. Empty selections return an empty dictionary. """ if args.arg: - try: - int_arg = int(args.arg) - return {int_arg: self.history.get(int_arg)} - except ValueError: - pass - - if ".." in args.arg or ":" in args.arg: - # Get a slice of history - history = self.history.span(args.arg, args.all) - elif args.arg.startswith(r"/") and args.arg.endswith(r"/"): - history = self.history.regex_search(args.arg, args.all) - else: - history = self.history.str_search(args.arg, args.all) - else: - # Get a copy of the history so it doesn't get mutated while we are using it - history = self.history.span(":", args.all) - return history + if args.arg.startswith(r"/") and args.arg.endswith(r"/"): + return self.history.regex_search(args.arg, args.all) + if re.fullmatch(r"-?[0-9]+", args.arg): + try: + index = int(args.arg) + except ValueError: + # Python may reject integer strings exceeding its conversion limit. + raise HistoryIndexError(NO_HISTORY_ITEMS) from None + if index == 0: + # Preserve the dedicated index-zero error and command-one reminder. + self.history.get(index) + return self.history.span(f"{index}:{index}") + if self.history.spanpattern.fullmatch(args.arg): + return self.history.span(args.arg, args.all) + return self.history.str_search(args.arg, args.all) + + # Get a copy of the history so it doesn't get mutated while we are using it. + return self.history.span(":", args.all) def _initialize_history(self, hist_file: str) -> None: """Initialize history using history related attributes. diff --git a/cmd2/history.py b/cmd2/history.py index c99a8aac5..abd168028 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,46 +203,30 @@ 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] - # This regular expression parses input for the span() method. There are five parts: - # - # ^\s* matches any whitespace at the beginning of the - # input. This is here so you don't have to trim the input - # - # (?P-?[1-9]{1}\d*)? create a capture group named 'start' which matches an - # optional minus sign, followed by exactly one non-zero - # digit, and as many other digits as you want. This group - # is optional so that we can match an input string like '..2'. - # This regex will match 1, -1, 10, -10, but not 0 or -0. - # - # (?P:|(\.{2,}))? create a capture group named 'separator' which matches either - # a colon or two periods. - # - # (?P-?[1-9]{1}\d*)? create a capture group named 'end' which matches an - # optional minus sign, followed by exactly one non-zero - # digit, and as many other digits as you want. This group is - # optional so that we can match an input string like ':' - # or '5:'. This regex will match 1, -1, 10, -10, but not - # 0 or -0. - # - # \s*$ match any whitespace at the end of the input. This is here so - # you don't have to trim the input - # - spanpattern = re.compile(r"^\s*(?P-?[1-9]\d*)?(?P:|(\.{2,}))(?P-?[1-9]\d*)?\s*$") + # Recognize ranges with optional signed ASCII integer endpoints. Zero is parsed so that + # span() can report its specific error; it is never a valid history index. + spanpattern = re.compile(r"\s*(?P-?[0-9]+)?(?P:|\.{2,})(?P-?[0-9]+)?\s*") def span(self, span: str, include_persisted: bool = False) -> dict[int, "HistoryItem"]: """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 the start is out of bounds or a bounded range selects no history items This method can accommodate input in any of these forms: @@ -249,20 +246,24 @@ def span(self, span: str, include_persisted: bool = False) -> dict[int, "History - off by one errors """ - results = self.spanpattern.search(span) + results = self.spanpattern.fullmatch(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(INVALID_HISTORY_RANGE) start_token = results.group("start") + end_token = results.group("end") + if any(token is not None and int(token) == 0 for token in (start_token, end_token)): + raise HistoryFormatError(f"{INVALID_HISTORY_RANGE} {FIRST_HISTORY_INDEX}") 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) + start += len(self) + if start < 0: + raise HistoryIndexError(NO_HISTORY_ITEMS) 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 +271,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..59320f948 100644 --- a/docs/features/history.md +++ b/docs/features/history.md @@ -59,7 +59,8 @@ that saved input. The examples to follow all assume that you have entered the fo Alias 'four' created In its simplest form, the `history` command displays previously entered commands. With no additional -arguments, it displays all previously entered commands: +arguments, it displays commands entered in the current session. Use `history --all` to also display +commands loaded from a persistent history file: (Cmd) history 1 alias create one !echo one @@ -87,8 +88,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 +101,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: @@ -116,7 +124,9 @@ an argument: There is no zeroth command, so don't ask for it. If you are a Python programmer, you've probably noticed this looks a lot like the slice syntax for lists and arrays. It is, with the exception that -the first history command is 1, where the first element in a Python array is 0. +the first history command is 1, where the first element in a Python array is 0. A start before the +first command or after the last command is an error. An end after the last command stops at the end +of history. Besides selecting previous commands by number, you can also search for them. You can use a simple string search: @@ -132,6 +142,10 @@ Or a regular expression search by enclosing your regex in slashes: If your regular expression contains any characters that `argparse` finds interesting, like dash or plus, you also need to enclose your regular expression in quotation marks. +Regular expressions are recognized before ranges, so `/a..b/` and `/one:/` are regex searches. +Numbers use ASCII digits with an optional leading minus sign. Other arguments, such as `http://x`, +`a:b`, `+1`, and `1_0`, are literal searches rather than command numbers or ranges. + This all sounds great, but doesn't it seem like a bit of overkill to have all these ways to select commands if all we can do is display them? Turns out, displaying history commands is just the beginning. The history command can perform many other actions: @@ -160,13 +174,29 @@ file are executed. To edit and then re-run commands 2-4 you would: (Cmd) history --edit 2:4 +Running or editing a selection that includes commands from previous sessions requires `--all`, even +when selecting a single command or a bounded range. For example, use `history --all --run 1` to +replay a persisted command 1. Without `--all`, the entire action is refused if any selected command +is persisted; commands are not silently filtered out or renumbered. Current-session-only selections +do not require `--all`. + 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, while empty selections with no selector or an unbounded range report +`No history items in range`. Displaying these empty selections succeeds without output. Invalid +indices and bounded ranges selecting no commands also report `No history items in range`. Index zero +and zero range endpoints include a reminder that the first command is command 1. The +`History.span()` API reports malformed ranges as format errors; the command treats arguments that do +not match range syntax as literal searches. + 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..7ff3fca27 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -1,7 +1,10 @@ """Test history functions of cmd2""" +import argparse import contextlib import os +import re +import sys import tempfile from unittest import ( mock, @@ -10,6 +13,7 @@ import pytest import cmd2 +from cmd2.history import HistoryFormatError, HistoryIndexError from .conftest import ( normalize, @@ -228,6 +232,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]), ("-4:-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 +319,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 +334,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 +713,659 @@ def test_history_with_span_end(base_app) -> None: verify_hi_last_result(base_app, 2) -def test_history_with_span_index_error(base_app) -> None: +# 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: + message = ( + "No history items in range" if selector in ("", '""', ":", "..", '" : "') else "No matching history items" + ) + assert err == [message] + 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 history items in range"] + 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", "-5:", "-5..4", "-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:"]) +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:"]) +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:"]) +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_literal_search_containing_colon(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 err == [] + assert out == [" 3 !ls -hal :"] + assert list(base_app.last_result) == [3] + + +@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"-4{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}") + if action in ("--run", "--edit") and not include_persisted: + assert out == [] + assert err == ["Use --all to run or edit commands from previous sessions."] + assert base_app.last_result is False + replay.assert_not_called() + run_script.assert_not_called() + assert edited == [] + assert not output_file.exists() + return + 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"]) +@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 + + +@pytest.mark.parametrize("selector", ["-5:", "-5:-1", "-5:3", "-5..", "-5..-1", "-999:"]) +def test_history_span_rejects_overly_negative_starts(hist, selector) -> None: + with pytest.raises(HistoryIndexError, match="No history items in range"): + hist.span(selector) + + +@pytest.mark.parametrize( + ("selector", "needle"), + [ + ("/a..b/", "axxb"), + ("/one:/", "one:"), + ("http://x", "http://x"), + ("a:b", "a:b"), + ("1:2:3", "1:2:3"), + ("abc:", "abc:"), + ("1_0", "1_0"), + ("+1", "+1"), + ('" 1 "', " 1 "), + ("\u0661", "\u0661"), + ("1\u0661", "1\u0661"), + ("-\u0661", "-\u0661"), + ], +) +def test_history_search_dispatch_is_not_numeric_or_range(base_app, hist, selector, needle) -> None: + hist.clear() + for index in range(10): + text = f"help before{needle}after" if index == 2 else "help control" + hist.append(base_app.statement_parser.parse(text)) + base_app.history = hist + + out, err = run_cmd(base_app, f"history -- {selector}") + + assert err == [] + assert out == [f" 3 help before{needle}after"] + assert base_app.last_result == {3: hist[2]} + + +@pytest.mark.parametrize("selector", ["", ":", "no_such_command", "/no_such_regex/"]) +def test_get_history_returns_empty_dictionary_for_actions(base_app, selector) -> None: + args = argparse.Namespace(arg=selector, all=False, run=False, edit=True, output_file=None) + assert base_app._get_history(args) == {} + + +@pytest.mark.parametrize("selector", ["1:2:3", "abc:", "a:b", "\u0661:2", "1\u0661:2", "+1:2", "1_0:2"]) +def test_history_format_error_does_not_suggest_command_one(hist, selector) -> None: + with pytest.raises(HistoryFormatError) as error: + hist.span(selector) + assert "The first command" not in str(error.value) + + +@pytest.mark.parametrize("selector", ["0:2", "1:0", "-0..2", "..-0"]) +def test_history_zero_endpoint_has_command_one_hint(hist, selector) -> None: + with pytest.raises(HistoryFormatError) as error: + hist.span(selector) + assert "The first command in history is command 1." in str(error.value) + + +@pytest.mark.parametrize("selector", ["1", "-6", ":2", "1:5", "2:", "-6:-1", "4", "-3", "4:5"]) +@pytest.mark.parametrize("action", ["--run", "--edit"]) +def test_history_persisted_actions_require_all(base_app, persisted_hist, monkeypatch, selector, action) -> None: + base_app.history = persisted_hist + 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]) + + out, err = run_cmd(base_app, f"history {action} -- {selector}") + + assert out == [] + assert err == ["Use --all to run or edit commands from previous sessions."] + assert base_app.last_result is False + for effect in effects: + effect.assert_not_called() + + +@pytest.mark.parametrize("selector", ["5", "-2", "5:", "-2:"]) +@pytest.mark.parametrize("action", ["--run", "--edit"]) +def test_history_current_session_actions_do_not_require_all(base_app, persisted_hist, monkeypatch, selector, action) -> None: + base_app.history = persisted_hist + expected = list(persisted_hist[4:5] if selector in ("5", "-2") else persisted_hist[4:]) + 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) + + _out, err = run_cmd(base_app, f"history {action} -- {selector}") + + assert err == [] + if action == "--run": + replay.assert_called_once_with(expected) + else: + assert edited == ["".join(f"{item.raw}\n" for item in expected)] + run_script.assert_called_once() + + +@pytest.mark.parametrize("sign", ["", "-"]) +@pytest.mark.parametrize("action", ["", "--run", "--edit", "--output-file"]) +@pytest.mark.parametrize("debug", [False, True]) +def test_history_integer_conversion_limit_has_no_side_effects( + base_app, hist, monkeypatch, tmp_path, sign, action, debug +) -> None: + base_app.history = hist + base_app.debug = debug + 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]) + output_file = tmp_path / "existing.txt" + output_file.write_text("keep me\n") + options = f'--output-file "{output_file}"' if action == "--output-file" else action + limit = sys.int_info.str_digits_check_threshold + selector = sign + "9" * (limit + 1) + original_limit = sys.get_int_max_str_digits() + try: + sys.set_int_max_str_digits(limit) + out, err = run_cmd(base_app, f"history {options} -- {selector}") + finally: + sys.set_int_max_str_digits(original_limit) + + assert out == [] + assert err == ["No history items in range"] + assert base_app.last_result is False + for effect in effects: + effect.assert_not_called() + assert output_file.read_text() == "keep me\n" + assert len(base_app.history) == 4 + + +def test_history_help_explains_selection_and_persisted_actions(base_app) -> None: + out, err = run_cmd(base_app, "help history") + text = " ".join(out) + assert err == [] + assert "current session" in text + assert "full history" in text + assert "required to run or edit" in text def test_history_output_file() -> None: