Fix history ranges starting past the end

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

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:

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,24 +271,27 @@ 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:
                 end = max(0, len(self) + end + 1)
         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"]:

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`: