From 1409c293b5424568510509cbca680ed5705c32ce Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 4 Oct 2026 18:36:49 -0400 Subject: [PATCH] docs(utilities): drop the change-over-time narration from the small subtrees Covers `pcapkit/utilities/`, `toolkit/`, `dumpkit/`, `interface/` and the top-level `pcapkit/*.py`. Timed-context cuts per #719, plus three prose repairs a cross-review asked for. - `exceptions.py` carried the most: the `BaseError` bullets, the `stacklevel` "negated" note, the `_excepthook` `limit=0` note and the `_threading_excepthook` paragraph all narrated what the design used to be. They now state why it is what it is. The `extract_stack()`-versus-frame-walk rationale is kept. - `interface/misc.py`, `dumpkit/pcap.py`, `common.py`, `__main__.py`, `compat.py`, `decorators.py` and `toolkit/scapy.py` lose "used to", "now", "today", "previously" and "no longer". - The `Formats` comment claimed the registries' eight keys are what make the aliases spellable. They are not -- the `Literal` listing all eight is. Measured: the eight keys resolve to **four** dumpers, so the old list of three aliases also omitted `'text'`. It now names every alias by its target. - `__main__.py` had a cut leave "only worked" with nothing to anchor it; now present tense. `dumpkit/pcap.py` called `UInt32Field` "the alternative way to pack them", asserting uniqueness for a class the dumper does not use -- it packs with `struct.Struct(' 'ArgumentParser': # ``-L Internet`` working, as it did before the choices were declared, since # ``Extractor.__init__`` lowercases the value anyway. # - # The defaults are :data:`None` and not the ``'None'``/``'null'`` strings they - # used to be: ``Extractor.__init__`` substitutes its own sentinels for an - # omitted value, so passing the strings only worked because it happened to - # lowercase ``'None'`` into the sentinel it wanted. + # The defaults are :data:`None` and not the ``'None'``/``'null'`` strings: + # ``Extractor.__init__`` substitutes its own sentinels for an + # omitted value, so passing the strings would only work because it happens to + # lowercase ``'None'`` into the sentinel it wants. parser.add_argument('-L', '--layer', action='store', dest='layer', default=None, metavar='LAYER', type=str.lower, choices=['link', 'internet', 'transport', 'application', 'none'], help='Indicate extract frames until which layer.') diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py index e3de85778..794e509d3 100644 --- a/pcapkit/dumpkit/common.py +++ b/pcapkit/dumpkit/common.py @@ -121,12 +121,11 @@ def __init_subclass__(cls, /, fmt: 'Optional[str]' = None, has guarded on its own ``code`` keyword all along. Note: - The previous behaviour inferred ``fmt`` from the subclass' - :attr:`~dictdumper.dumper.Dumper.kind` property, which it could - only read off an *instance* -- so it constructed one against a - :func:`tempfile.NamedTemporaryFile` while the ``class`` statement - was still executing. Guarding on ``fmt`` removes that: a class - definition no longer touches the filesystem. + Inferring ``fmt`` from the subclass' + :attr:`~dictdumper.dumper.Dumper.kind` property would need an + *instance*, hence a :func:`tempfile.NamedTemporaryFile` created while + the ``class`` statement is still executing. Guarding on ``fmt`` + avoids that: a class definition does not touch the filesystem. See Also: - :func:`pcapkit.foundation.registry.foundation.register_dumper` @@ -244,7 +243,7 @@ def make_dumper(output: 'Type[ABCDumper]') -> 'Type[ABCDumper]': # double-escape those three. :class:`~dictdumper.xml.XML` itself defines # no ``_append_string`` of its own -- its own module docstring says not to # use it directly -- so :class:`~dictdumper.plist.PLIST` is the only - # concrete writer this applies to today. + # concrete writer this applies to. escape_strings = issubclass(output, dictdumper.plist.PLIST) def escape_key(key: 'Any') -> 'Any': diff --git a/pcapkit/dumpkit/pcap.py b/pcapkit/dumpkit/pcap.py index dc28bc2fb..429d93474 100644 --- a/pcapkit/dumpkit/pcap.py +++ b/pcapkit/dumpkit/pcap.py @@ -41,7 +41,7 @@ } #: Truncation mask for those four fields. :class:`~pcapkit.corekit.fields.numbers.UInt32Field`, -#: which used to pack them, masks to the field width in +#: the field class that would otherwise pack them, masks to the field width in #: :meth:`~pcapkit.corekit.fields.numbers.NumberField.pre_process` rather than #: rejecting an out-of-range value -- a ``ts_sec`` of ``2**32 + 5`` was written as #: ``5``. :func:`struct.pack` raises instead, so the mask is applied here to keep @@ -94,7 +94,7 @@ def __init__(self, fname: 'str', *, protocol: 'Enum_LinkType | StdlibIntEnum | A """ #: int: Frame counter. self._fnum = 1 - # NOTE: Both of these now only record how the dumper was configured -- the + # NOTE: Both of these only record how the dumper was configured -- the # values that shape the output reach it through :meth:`self._dump_header # <_dump_header>`'s own arguments, and are readable afterwards from # :attr:`self._ghdr <_ghdr>`. They are kept because they are part of the diff --git a/pcapkit/interface/misc.py b/pcapkit/interface/misc.py index 4829df640..896eb59d0 100644 --- a/pcapkit/interface/misc.py +++ b/pcapkit/interface/misc.py @@ -41,9 +41,9 @@ #: ` and in #: :attr:`TraceFlowBase.__output__ #: ` -- the two - #: registries expose the same eight keys. This used to name only four of them, - #: which made ``'cap'`` and the ``'txt'``/``'xml'`` aliases unspellable for a - #: type checker even though every one of them is accepted at runtime. + #: registries expose the same eight keys, which resolve to four dumpers (``'cap'`` to + #: ``'pcap'``, ``'xml'`` to ``'plist'``, ``'text'`` and ``'txt'`` to ``'tree'``), so the + #: ``Literal`` lists all eight for a type checker to accept every spelling the runtime does. Formats = Literal['pcap', 'cap', 'json', 'tree', 'text', 'txt', 'plist', 'xml'] # NOTE: this alias duplicates the one in ``pcapkit.foundation.extraction``; # both copies need updating when a new engine lands. @@ -122,8 +122,8 @@ def follow_tcp_stream(fin: 'Optional[str]' = None, verbose: 'bool' = False, # ``'pcap'``, so following a stream through either engine would crash *during # extraction*, before the reassembly below ever runs. # - # :class:`Extractor ` now guards both - # engines itself, so this is no longer what keeps the extraction alive -- it is + # :class:`Extractor ` guards both + # engines itself, so this is not what keeps the extraction alive -- it is # what keeps it *quiet*. The two guards choose the same replacement format and so # produce byte-identical traces; they differ only in when they complain. The # Extractor warns for every substitution it makes, including the one nobody asked diff --git a/pcapkit/toolkit/scapy.py b/pcapkit/toolkit/scapy.py index cd39648e9..3164ba717 100644 --- a/pcapkit/toolkit/scapy.py +++ b/pcapkit/toolkit/scapy.py @@ -327,7 +327,7 @@ def tcp_traceflow(packet: 'Packet', *, count: 'int' = -1) -> 'TF_TCP_Packet | No # protocol class, so neither is an honest stand-in for "unknown link # type" and this must not paper over the miss with either. An # IP-rooted Scapy packet's ``(IP()/TCP()).name`` is ``'IP'``, which is - # not a LinkType member name, and now raises. Note the asymmetry, which + # not a LinkType member name, and raises. Note the asymmetry, which # is not a choice made here: an IPv6-rooted packet's name uppercases to # ``'IPV6'``, which *is* a member (``LinkType.IPV6``, 229), so it # resolves silently -- to a DLT the caller never chose. Only the v4 name diff --git a/pcapkit/utilities/compat.py b/pcapkit/utilities/compat.py index 2b9291ccb..dc73fa595 100644 --- a/pcapkit/utilities/compat.py +++ b/pcapkit/utilities/compat.py @@ -159,7 +159,7 @@ def __get__(self, instance: 'Optional[_S]', # what this library needs from ``final`` is not the name but the side effect: # from 3.11 on (gh-90500) ``typing.final`` records ``__final__ = True`` on the # decorated object, and before that it is a pure annotation for the type checker -# leaving no runtime trace at all. That dunder is now *read* -- +# leaving no runtime trace at all. That dunder is *read* -- # :meth:`pcapkit.corekit.infoclass.Info.__init_subclass__` and # :meth:`pcapkit.protocols.schema.schema.Schema.__init_subclass__` refuse to # derive from a class carrying it in its own ``__dict__`` -- so on 3.10, which is diff --git a/pcapkit/utilities/decorators.py b/pcapkit/utilities/decorators.py index 9764bfe8f..a94b73168 100644 --- a/pcapkit/utilities/decorators.py +++ b/pcapkit/utilities/decorators.py @@ -149,7 +149,7 @@ def behold(*args: 'P.args', **kwargs: 'P.kwargs') -> 'R_beholder': # ProtocolUnbound('unknown field: payload') *from the recovery path*, # turning a next-layer parse failure that should have degraded to # Raw into a crash. Unreachable until something was registered on an - # SCTP payload protocol identifier, which NGAP now is. + # SCTP payload protocol identifier, which NGAP is. file_ = self._get_payload() # NOTE: ``alias=proto`` matches what ``_import_next_layer`` passes, so @@ -209,7 +209,7 @@ def prepare(func: 'Callable[Concatenate[Type[R_prepare], bytes | IO[bytes], Opti nonetheless promised implementors a trailing ``*args, **kwargs``, which the wrapper below never populated. A caller relying on that promise got extras silently discarded instead of forwarded -- see :issue:`454` -- so the - wrapper now raises :exc:`TypeError` for a fifth positional argument or + wrapper raises :exc:`TypeError` for a fifth positional argument or an unconsumed keyword, the same as an ordinary call with too many arguments would. diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index 8af4dcacb..ceee237da 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -104,19 +104,15 @@ def stacklevel() -> 'int': the :mod:`sys` module. The walk goes through :func:`inspect.currentframe` and ``f_back`` rather than - :func:`traceback.extract_stack`, which *used to* be unusable here for a sharper - reason than cost: :meth:`traceback.StackSummary.extract` honours - :data:`sys.tracebacklimit`, and :class:`BaseError` used to set that to ``0`` for - every loud error outside development mode. One such error therefore made - ``extract_stack()`` return an *empty* list for the rest of the process, which is - where the old ``-1`` came from -- so in ordinary use the first error silently - broke the attribution of every warning after it. :class:`BaseError` no longer - touches :data:`sys.tracebacklimit` at all (it prints its terse line through an - exception hook instead), which removes that failure mode -- but the frame walk - remains the right approach regardless of it: it skips building the + :func:`traceback.extract_stack`: it skips building the :class:`~traceback.FrameSummary` objects and the :mod:`linecache` lookups :func:`traceback.extract_stack` does for every frame, which is worth avoiding on - a function called once per warning. + a function called once per warning. It is also immune to + :data:`sys.tracebacklimit`, which :meth:`traceback.StackSummary.extract` + honours: a limit of ``0`` would make ``extract_stack()`` return an *empty* list + and silently break the attribution of every warning. :class:`BaseError` does not + set that limit (it prints its terse line through an exception hook instead), but + user code may. Important: The level is relative to the *caller* of :func:`stacklevel`. A function @@ -185,14 +181,10 @@ class BaseError(Exception): Important: - * This terseness used to come from setting :data:`sys.tracebacklimit` to - ``0``, which is process-global: one loud error truncated the tracebacks - of every *other*, unrelated exception for the rest of the process, - including ones that had nothing to do with :mod:`pcapkit`. It also, - incidentally, was what kept a loud error raised on a worker thread - terse, since the interpreter's default :data:`threading.excepthook` - itself consults :data:`sys.tracebacklimit`. The two hooks here replace - that mechanism precisely because neither has that reach -- each + * The terseness is deliberately not done by setting + :data:`sys.tracebacklimit` to ``0``: that is process-global, so one loud + error would truncate the tracebacks of every *other*, unrelated + exception for the rest of the process. Each of the two hooks here shortens the printing of a :class:`BaseError` only, and hands every other exception, unchanged, to whatever hook :mod:`pcapkit` found installed before it first needed its own. @@ -206,10 +198,9 @@ class BaseError(Exception): neither. See GitHub issue :issue:`719`. * The ``stacklevel`` of the log record is the relative level :func:`stacklevel` computes, so the record is attributed to the caller - whose operation failed rather than to this module. It used to be - *negated*, which :meth:`logging.Logger.findCaller` reads as "do not walk - out at all" and which therefore blamed :mod:`logging` itself for every - error pcapkit raised. + whose operation failed rather than to this module. It must not be + negated: :meth:`logging.Logger.findCaller` reads that as "do not walk + out at all" and would blame :mod:`logging` itself for every error. See Also: :func:`pcapkit.utilities.exceptions.stacklevel` @@ -276,10 +267,9 @@ def _excepthook(etype: 'Type[BaseException]', value: 'BaseException', only ever hands it the top exception's own traceback -- the chain's inner links keep their *real* ``__cause__``/``__context__`` traceback attributes regardless, untouched by this call, and get printed in full. ``limit=0`` - against the real ``tb`` is what reaches every link, matching exactly what - :data:`sys.tracebacklimit` set to ``0`` used to produce for the same - exception -- the one-line answer for a plain :class:`BaseError` and the - *same* chain-without-frames answer this used to get wrong. + against the real ``tb`` is what reaches every link, giving the one-line + answer for a plain :class:`BaseError` and a chain without frames for a + chained one. Anything else is handed to ``_previous_excepthook`` exactly as received, so a program with its own hook installed before :mod:`pcapkit` needed one -- @@ -356,15 +346,10 @@ def _threading_excepthook(args: 'ExceptHookArgs') -> 'None': was previously installed -- unchanged, header included, since that path does not touch the default hook's own printing at all. - Replacing this hook is the other half of why :data:`sys.tracebacklimit` - cannot simply be dropped without a replacement: the interpreter's *default* - :data:`threading.excepthook` itself consults :data:`sys.tracebacklimit` when - printing an uncaught exception from a worker thread, which is what made a - loud :class:`BaseError` there terse before this hook existed, incidentally - rather than by any thread-aware design on this module's part. Without this, - removing the global would have *regressed* that path from one line to a - full default traceback, even though the defect it was fixing is itself - thread-independent. + This hook is needed because the interpreter's *default* + :data:`threading.excepthook` consults :data:`sys.tracebacklimit`, which + :class:`BaseError` does not set; without it a loud :class:`BaseError` raised + on a worker thread would print a full default traceback. """ if getattr(_threading_hook_state, 'running', False):