Skip to content

Commit 447a162

Browse files
committed
Capture(fix[review]): Refuse to read a failed capture as silence
why: Pane.capture_pane returns tmux's stdout without inspecting stderr, so a failed capture and a blank pane were indistinguishable — the one case where this module could return an incomplete delta without setting lines_missed. Separately, the exhausted-retry path took three unsynchronized reads, pairing an anchor row number with a fingerprint sampled at a different instant. what: - Issue capture-pane directly and raise on stderr; the module only ever needs -p, -S and -E, so it does not need the wrapper's flag matrix - Return the last attempt's paired reads when stability retries are exhausted, anchoring cursor and fingerprint to one sample and saving three round-trips - Hash each row once in the fingerprint search instead of once per overlapping window - Make the module-level entry point private; Pane.capture_since and CaptureCursor are the public surface - Add a module logger and a DEBUG line explaining a missed capture - Correct an inverted bound in the delta comment: start is below pane_height, and may be negative to address retained history
1 parent 9c0137a commit 447a162

3 files changed

Lines changed: 81 additions & 21 deletions

File tree

src/libtmux/capture.py

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,17 @@
2828
import dataclasses
2929
import hashlib
3030
import json
31+
import logging
3132
import typing as t
3233

3334
from libtmux import exc
35+
from libtmux.common import raise_if_stderr
3436

3537
if t.TYPE_CHECKING:
3638
from libtmux.pane import Pane
3739

40+
logger = logging.getLogger(__name__)
41+
3842

3943
#: Serialized-cursor prefix. Versioned so the wire format can change without
4044
#: a decoder silently misreading an older payload as a newer one.
@@ -553,10 +557,15 @@ def _find_unique_cursor_match(rows: list[str], cursor: CaptureCursor) -> int | N
553557
if len(rows) < len(fingerprint):
554558
return None
555559

560+
# Hash each row once. Windows overlap, so hashing per-window would
561+
# re-hash a row once for every window it appears in -- up to the
562+
# fingerprint's length -- on the path that is already the expensive
563+
# fallback.
564+
hashes = [_line_hash(line) for line in rows]
565+
556566
match_index: int | None = None
557-
for index in range(len(rows) - len(fingerprint) + 1):
558-
candidate = rows[index : index + len(fingerprint)]
559-
if tuple(_line_hash(line) for line in candidate) != fingerprint:
567+
for index in range(len(hashes) - len(fingerprint) + 1):
568+
if tuple(hashes[index : index + len(fingerprint)]) != fingerprint:
560569
continue
561570
if match_index is not None:
562571
return None
@@ -775,15 +784,27 @@ def _capture_rows(
775784
start: t.Literal["-"] | int | None = None,
776785
end: t.Literal["-"] | int | None = None,
777786
) -> list[str]:
778-
"""Capture pane rows as a concrete list.
787+
"""Capture pane rows, refusing to mistake a failed read for silence.
788+
789+
Issues ``capture-pane`` directly rather than through
790+
:meth:`~libtmux.pane.Pane.capture_pane`, which returns tmux's stdout
791+
without inspecting stderr. A blank pane and a failed capture both
792+
yield no rows there, and this module cannot tell a caller "nothing
793+
was written" unless it knows the read succeeded.
779794
780795
Examples
781796
--------
782797
>>> isinstance(_capture_rows(pane), list)
783798
True
784799
"""
785-
rows = pane.capture_pane(start=start, end=end)
786-
return [] if rows is None else list(rows)
800+
args = ["capture-pane", "-p"]
801+
if start is not None:
802+
args.extend(["-S", str(start)])
803+
if end is not None:
804+
args.extend(["-E", str(end)])
805+
proc = pane.cmd(*args)
806+
raise_if_stderr(proc, "capture-pane")
807+
return list(proc.stdout)
787808

788809

789810
def _capture_cursor_rows(pane: Pane, state: _PaneState) -> list[str]:
@@ -816,6 +837,14 @@ def _read_stable_visible(
816837
the read is retried. After :data:`_STABLE_READ_ATTEMPTS` the rows are
817838
returned with ``lines_missed`` set rather than presented as exact.
818839
840+
Exhausting the attempts returns the *last attempt's* reads rather than
841+
taking fresh ones. On a pane writing continuously enough to defeat
842+
three brackets, another round of unbracketed samples would pair an
843+
anchor row number with a fingerprint taken at a different instant, and
844+
the resulting cursor would claim to anchor content it never saw. The
845+
last attempt's ``before`` snapshot and the rows captured against it at
846+
least describe one moment.
847+
819848
Parameters
820849
----------
821850
pane : Pane
@@ -836,6 +865,9 @@ def _read_stable_visible(
836865
>>> isinstance(read.lines, list)
837866
True
838867
"""
868+
before = _read_pane_state(pane)
869+
lines: list[str] = []
870+
cursor_rows: list[str] = []
839871
for _attempt in range(_STABLE_READ_ATTEMPTS):
840872
before = _read_pane_state(pane)
841873
if baseline_pid is None:
@@ -857,15 +889,15 @@ def _read_stable_visible(
857889
lines_missed=False,
858890
)
859891

860-
state = _read_pane_state(pane)
861-
if baseline_pid is None:
862-
_raise_if_dead_without_baseline(pane, state)
863-
else:
864-
_raise_if_lifecycle_changed(pane.pane_id, state, baseline_pid)
892+
logger.debug(
893+
"pane never settled across %s reads; reporting a missed capture",
894+
_STABLE_READ_ATTEMPTS,
895+
extra={"tmux_pane": pane.pane_id, "tmux_stdout_len": len(lines)},
896+
)
865897
return _PaneRead(
866-
state=state,
867-
cursor_rows=_capture_cursor_rows(pane, state),
868-
lines=_capture_rows(pane),
898+
state=before,
899+
cursor_rows=cursor_rows,
900+
lines=lines,
869901
lines_missed=True,
870902
)
871903

@@ -915,8 +947,9 @@ def _read_delta(pane: Pane, cursor: CaptureCursor) -> _PaneRead:
915947
rows = _capture_rows(pane, start="-", end=None)
916948
else:
917949
# ``_cursor_anchor_lost`` returning False above already proved
918-
# ``anchor_abs`` sits at or above the grid bottom, so ``start``
919-
# is always inside the visible region here.
950+
# ``anchor_abs`` sits at or below the grid bottom, so ``start``
951+
# is always below ``pane_height``. It may still be negative,
952+
# which is how ``capture-pane -S`` addresses retained history.
920953
rows = _capture_rows(pane, start=start, end=None)
921954
cursor_rows = _capture_cursor_rows(pane, before)
922955

@@ -952,7 +985,7 @@ def _missed_read(pane: Pane, cursor: CaptureCursor) -> _PaneRead:
952985
return missed._replace(lines_missed=True)
953986

954987

955-
def capture_since(pane: Pane, cursor: CaptureCursor | None = None) -> CaptureSince:
988+
def _capture_since(pane: Pane, cursor: CaptureCursor | None = None) -> CaptureSince:
956989
"""Capture rows written to ``pane`` since ``cursor``.
957990
958991
Implements :meth:`libtmux.pane.Pane.capture_since`; call that instead.
@@ -979,8 +1012,8 @@ def capture_since(pane: Pane, cursor: CaptureCursor | None = None) -> CaptureSin
9791012
9801013
Examples
9811014
--------
982-
>>> first = capture_since(pane)
983-
>>> capture_since(pane, first.cursor).lines
1015+
>>> first = _capture_since(pane)
1016+
>>> _capture_since(pane, first.cursor).lines
9841017
[]
9851018
"""
9861019
if pane.pane_id is None:

src/libtmux/pane.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from libtmux import exc
1717
from libtmux._internal.env import pane_id_from_env
18-
from libtmux.capture import CaptureCursor, CaptureSince, capture_since
18+
from libtmux.capture import CaptureCursor, CaptureSince, _capture_since
1919
from libtmux.common import get_version_str, has_gte_version, raise_if_stderr, tmux_cmd
2020
from libtmux.constants import (
2121
PANE_DIRECTION_FLAG_MAP,
@@ -766,7 +766,7 @@ def capture_since(self, cursor: CaptureCursor | None = None) -> CaptureSince:
766766
767767
.. versionadded:: 0.63
768768
"""
769-
return capture_since(self, cursor)
769+
return _capture_since(self, cursor)
770770

771771
def send_keys(
772772
self,

tests/test_capture_since.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,33 @@ def is_dead() -> bool:
273273
pane.capture_since()
274274

275275

276+
def test_a_failed_capture_raises_instead_of_reading_as_empty(
277+
session: Session, monkeypatch: pytest.MonkeyPatch
278+
) -> None:
279+
"""A tmux capture failure is never reported as "nothing was written".
280+
281+
``Pane.capture_pane`` returns tmux's stdout without inspecting stderr,
282+
so a failed capture and a blank pane are indistinguishable there. Uses
283+
``monkeypatch`` because provoking a real ``capture-pane`` failure
284+
against a live, healthy pane is not otherwise reachable.
285+
"""
286+
pane = session.new_window(window_name="capture_since_failed_read").active_pane
287+
assert pane is not None
288+
real_cmd = type(pane).cmd
289+
290+
def failing_capture(self: Pane, *args: str) -> t.Any:
291+
proc = real_cmd(self, *args)
292+
if args and args[0] == "capture-pane":
293+
proc.stderr = ["no such pane"]
294+
proc.stdout = []
295+
return proc
296+
297+
monkeypatch.setattr(type(pane), "cmd", failing_capture)
298+
299+
with pytest.raises(exc.LibTmuxException, match="capture-pane"):
300+
pane.capture_since()
301+
302+
276303
def test_cursor_round_trips_through_a_string(session: Session) -> None:
277304
"""A serialized cursor decodes back to an equal cursor."""
278305
pane = session.new_window(window_name="capture_since_codec").active_pane

0 commit comments

Comments
 (0)