diff --git a/pcapkit/protocols/internet/mh.py b/pcapkit/protocols/internet/mh.py index 23651ebfb..fd07b6a2f 100644 --- a/pcapkit/protocols/internet/mh.py +++ b/pcapkit/protocols/internet/mh.py @@ -581,25 +581,28 @@ class FastBindingAcknowledgmentStatus(EnumLookup, IntEnum): This class carried its own hand-rolled ``get()`` override through #930, and briefly again through GitHub issue #935's first attempt, which widened the override to accept ``default`` rather than delete - it outright. A ruling given in review of the work for #935 went the - other way, verbatim -- asked *"why must we have the two overrides - tho? cant they directly fall back to the base class's?"*, the answer - was *"I prefer (2) directly"*, ``(2)`` naming deletion among the - ruling's own options. Measured before acting on it: the override's own - docstring called it a "Backport support for original codes", but - this class mints no alias -- ``__members__`` and ``list(cls)`` - agree at 6 -- so what the override actually did was resolve an - :class:`int` by direct construction and a name by subscript, - exactly the dual resolution + it outright. An earlier lean on issue #935 had preferred that + widening; a later ruling in review of the attempt went the other + way: delete both ``get`` overrides in this module rather than widen + them, so :class:`FastBindingAcknowledgmentStatus` and + :class:`IPv6AddressPrefixCode` inherit + :meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also + removes the ``# type: ignore[override]`` suppressions the overrides + needed, and makes all seven re-parented classes behave alike on + ``get``, which had not been literally true. Measured before acting + on it: the override's own docstring called it a "Backport support + for original codes", but this class mints no alias -- + ``__members__`` and ``list(cls)`` agree at 6 -- so what the override + actually did was resolve an :class:`int` by direct construction and + a name by subscript, exactly the dual resolution :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for every other :class:`int`-valued registry in this tree. There was - nothing left to backport. ``get``/``get_all`` now come from the - base alone, the same as the five other re-parents #930 finished - alongside this one -- including :class:`LocalizedRoutingStatus` - and :class:`LMAAddressCode` below, whose own hand-rolled ``get()`` - GitHub issue #880 had already deleted outright, for the same - reason: zero callers depended on anything the base does not - already do. + nothing left to backport. ``get``/``get_all`` now come from the base + alone, the same as the five other re-parents #930 finished alongside + this one -- including :class:`LocalizedRoutingStatus` and + :class:`LMAAddressCode` below, whose own hand-rolled ``get()`` + GitHub issue #880 had already deleted outright, for the same reason: + zero callers depended on anything the base does not already do. A behaviour change comes with the deletion, deliberately: the override branched on ``isinstance(key, int)`` and routed every @@ -698,26 +701,30 @@ class IPv6AddressPrefixCode(EnumLookup, IntEnum): This class carried its own hand-rolled ``get()`` override through #930, and briefly again through GitHub issue #935's first attempt, which widened the override to accept ``default`` rather than delete - it outright. A ruling given in review of the work for #935 went the - other way, verbatim -- asked *"why must we have the two overrides - tho? cant they directly fall back to the base class's?"*, the answer - was *"I prefer (2) directly"*, ``(2)`` naming deletion among the - ruling's own options. Measured before acting on it: the override's own - docstring called it a "Backport support for original codes", but - this class mints no alias -- ``__members__`` and ``list(cls)`` - agree at 4 -- so what the override actually did was resolve an - :class:`int` by direct construction and a name by subscript, - exactly the dual resolution + it outright. An earlier lean on issue #935 had preferred that + widening; a later ruling in review of the attempt went the other + way: delete both ``get`` overrides in this module rather than widen + them, so :class:`FastBindingAcknowledgmentStatus` and + :class:`IPv6AddressPrefixCode` inherit + :meth:`~pcapkit.corekit.enum.EnumLookup.get` outright. That also + removes the ``# type: ignore[override]`` suppressions the overrides + needed, and makes all seven re-parented classes behave alike on + ``get``, which had not been literally true. Measured before acting + on it: the override's own docstring called it a "Backport support + for original codes", but this class mints no alias -- + ``__members__`` and ``list(cls)`` agree at 4 -- so what the override + actually did was resolve an :class:`int` by direct construction and + a name by subscript, exactly the dual resolution :meth:`~pcapkit.corekit.enum.EnumLookup.get` already provides for every other :class:`int`-valued registry in this tree. There was - nothing left to backport. ``get``/``get_all`` now come from the - base alone, the same as the five other re-parents #930 finished - alongside this one -- including + nothing left to backport. ``get``/``get_all`` now come from the base + alone, the same as the five other re-parents #930 finished alongside + this one -- including :class:`~pcapkit.protocols.internet.mh.LocalizedRoutingStatus` and :class:`~pcapkit.protocols.internet.mh.LMAAddressCode` below, whose own hand-rolled ``get()`` GitHub issue #880 had already deleted - outright, for the same reason: zero callers depended on anything - the base does not already do. + outright, for the same reason: zero callers depended on anything the + base does not already do. A behaviour change comes with the deletion, deliberately: the override branched on ``isinstance(key, int)`` and routed every @@ -839,10 +846,9 @@ class LocalizedRoutingStatus(EnumLookup, IntEnum): -- tests included -- so GitHub issue #880 deleted it outright rather than rebuilding it on the immutable contract, the same conclusion #935 reached separately for the other two, on a ruling given in - review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)`` - being deletion of those two overrides rather than widening them to - match the base. - GitHub issue #930's re-parenting above gives this class + review of that work: delete those two overrides rather than widen + them to match the base, which an earlier lean on the issue had + preferred. GitHub issue #930's re-parenting above gives this class ``get``/``get_all`` again, but as the base's own bare lookup rather than a bespoke override -- it still cannot mint, so an unassigned value raises through ``get`` exactly as it does through the bare @@ -919,10 +925,9 @@ class LMAAddressCode(EnumLookup, IntEnum): -- tests included -- so GitHub issue #880 deleted it outright rather than rebuilding it on the immutable contract, the same conclusion #935 reached separately for the other two, on a ruling given in - review of that work, verbatim: *"I prefer (2) directly"* -- ``(2)`` - being deletion of those two overrides rather than widening them to - match the base. - GitHub issue #930's re-parenting above gives this class + review of that work: delete those two overrides rather than widen + them to match the base, which an earlier lean on the issue had + preferred. GitHub issue #930's re-parenting above gives this class ``get``/``get_all`` again, but as the base's own bare lookup rather than a bespoke override -- it still cannot mint, so an unassigned value raises through ``get`` exactly as it does through the bare diff --git a/pcapkit/utilities/exceptions.py b/pcapkit/utilities/exceptions.py index be86a13b3..d91d8a3fd 100644 --- a/pcapkit/utilities/exceptions.py +++ b/pcapkit/utilities/exceptions.py @@ -716,10 +716,10 @@ class EnumKeyError(BaseError, KeyError): taste: ``E['nosuch']`` raises :exc:`KeyError` and ``E(999)`` raises :exc:`ValueError`, so a lookup that misses by *name* is :exc:`KeyError`-derived and one that misses by *value* is - :exc:`ValueError`-derived. A ruling recorded on GitHub issue #923, - verbatim: *"Either ``ValueError`` or ``KeyError``, that's depending on how - stdlib's ``Enum`` would raise on these circumstances. And we should raise - one from ``pcapkit.utilities.exceptions`` rather builtin exceptions."* + :exc:`ValueError`-derived. That is a ruling given in review of #877's + phase-2 re-parenting, carried out by GitHub issue #923: raise + whichever of the two stdlib :class:`~enum.Enum` would raise in the same + circumstances, and raise it from this module rather than as a builtin. Deriving from :exc:`KeyError` is what makes that ruling cheap to carry out: :meth:`~pcapkit.corekit.enum.EnumLookup.get` raised a bare builtin diff --git a/pcapkit/vendor/__main__.py b/pcapkit/vendor/__main__.py index 92eb770e4..87beac897 100644 --- a/pcapkit/vendor/__main__.py +++ b/pcapkit/vendor/__main__.py @@ -51,12 +51,13 @@ def get_parser() -> 'ArgumentParser': def _snapshot_and_restore(vendor: 'Type[Vendor]') -> 'Iterator[None]': """Copy a target's const file aside before it runs; restore it if it raises. - A ruling given in review of the work for #872, verbatim: *"an easier - path is simply keep a copy before running the sub-vendor and revert if - anything failed."* This is that -- at the per-target boundary - :func:`run` already owns, which is also exactly where the ruling's - ``(b)``, "only discard changes made by a non-zero sub-vendor", wants - the discarding to happen. + A ruling given in review of the work for #872 settled how a failed target + is undone: keep a copy of its const file before running the sub-vendor and + revert if anything failed, rather than making the write itself atomic. + This is that -- at the per-target boundary :func:`run` already owns, which + is also exactly where the earlier ruling on the same work wants the + discarding to happen: only a non-zero sub-vendor's changes are discarded, + and a zero-exited one's are kept. It is a *wider* guarantee than protecting the single ``open``/``print`` pair :meth:`~pcapkit.vendor.default.Vendor.__init__` @@ -101,18 +102,17 @@ def _snapshot_and_restore(vendor: 'Type[Vendor]') -> 'Iterator[None]': A symlinked destination has a related divergence from the pre-#872 behaviour -- documented against the write path by rounds 4-8 (deleted - along with :meth:`~pcapkit.vendor.default.Vendor._write_atomic`, though - the underlying behaviour persists here instead): ``open(const_file, - 'w')`` writes *through* a symlink, into whatever file it points at, - leaving the link itself untouched. ``os.replace(backup, const_file)`` - on restore instead replaces the *link itself* with a regular file. - Measured, for ``link.py`` symlinked to ``real.py``: after a failure and - restore, ``real.py`` is left however the crawler's failed write left - it (truncated, in this case -- nothing here restores the file a - symlink used to point at), ``link.py`` is now a regular file holding - the backup's content, and ``os.path.islink(link.py)`` is - :data:`False`. ``find pcapkit/const -type l`` is still empty, so this - remains latent. + along with ``_write_atomic``, though the underlying behaviour persists + here instead): ``open(const_file, 'w')`` writes *through* a symlink, + into whatever file it points at, leaving the link itself untouched. + ``os.replace(backup, const_file)`` on restore instead replaces the *link + itself* with a regular file. Measured, for ``link.py`` symlinked to + ``real.py``: after a failure and restore, ``real.py`` is left however + the crawler's failed write left it (truncated, in this case -- nothing + here restores the file a symlink used to point at), ``link.py`` is now a + regular file holding the backup's content, and + ``os.path.islink(link.py)`` is :data:`False`. ``find pcapkit/const + -type l`` is still empty, so this remains latent. Nothing is copied at all when the destination does not exist yet: there is no previous file for a failure to discard, so there is nothing this