fix: address four code-review findings

add.py: _resolve_entry_version now short-circuits for archive-backed
subprojects (as_vcs() is None), returning Version(revision=raw_version)
instead of falling through to a VCS branch/tag lookup that always
produces an empty branch fallback for archives.

log.py: replace _LogRenderBase/_NoExpandLogRender class pair (each
requiring a pylint too-few-public-methods suppression) with a single
factory function _make_non_expanding_log_render; the only remaining
suppression is the pre-existing protected-access on handler._log_render.

svnsubproject.py: override latest_available_version so SVN revision-only
pins (revision set, no branch, no tag) return the pinned revision with
the default branch context; previously the base implementation called
latest_revision_on_branch(trunk), causing dfetch check to report pinned
SVN revisions as out of date.

tests/test_subproject.py: MockVcsFetcher.browse_tree now returns
nullcontext(lambda path="": []) so any consumer calling
with subproject.as_vcs().browse_tree(...) as ls: gets a valid
context manager instead of None.

https://claude.ai/code/session_01BMSF8XFAxV6hABQgL7RZ3z

Author: claude

dfetch/commands/add.py

@@ -273,7 +273,13 @@ def _finalize_add(
 
 
 def _resolve_entry_version(ctx: _AddContext, raw_version: str) -> Version:
-    """Resolve a raw version string to a ``Version`` using remote branches and tags."""
+    """Resolve a raw version string to a ``Version`` using remote branches and tags.
+
+    For archive-backed subprojects ``raw_version`` is preserved as a revision
+    identifier (URL or hash) because archives have no branch/tag semantics.
+    """
+    if ctx.subproject.as_vcs() is None:
+        return Version(revision=raw_version)
     branches = ctx.subproject.list_of_branches()
     tags = ctx.subproject.list_of_tags()
     choices: list[Version] = [

dfetch/log.py

@@ -5,7 +5,7 @@
 import sys
 import types
 from contextlib import nullcontext
-from typing import TYPE_CHECKING, Any, cast
+from typing import Any, cast
 
 from rich._log_render import LogRender  # type: ignore[import-untyped]
 from rich.console import Console
@@ -16,26 +16,23 @@
 
 from dfetch import __version__
 
-if TYPE_CHECKING:
 
-    class _LogRenderBase:  # pylint: disable=too-few-public-methods
-        def __init__(self, **_kwargs: Any) -> None: ...
+def _make_non_expanding_log_render(**kwargs: Any) -> Any:
+    """Return a LogRender callable that disables table expansion.
 
-        def __call__(self, *_args: Any, **_kwargs: Any) -> Any: ...
+    Used when recording with asciinema to prevent Rich's ``expand=True`` from
+    padding log lines to the full terminal width, which produces spurious blank
+    lines in the cast player.
+    """
+    renderer = LogRender(**kwargs)
 
-else:
-    _LogRenderBase = LogRender
-
-
-class _NoExpandLogRender(_LogRenderBase):  # pylint: disable=too-few-public-methods
-    """LogRender that disables table expansion to prevent blank lines in asciicasts."""
-
-    def __call__(self, *args: Any, **kwargs: Any) -> Any:
-        """Render log entry without expanding the table to the full terminal width."""
-        table = super().__call__(*args, **kwargs)
+    def _render(*args: Any, **kw: Any) -> Any:
+        table = renderer(*args, **kw)
         table.expand = False
         return table
 
+    return _render
+
 
 def make_console(no_color: bool = False) -> Console:
     """Create a Rich Console with proper color handling."""
@@ -67,9 +64,10 @@ def configure_root_logger(console: Console | None = None) -> None:
         # causing the subsequent newline to produce a blank line in the cast
         # player.  Wrapping _log_render so it returns a non-expanding table
         # removes the trailing spaces and avoids the spurious blank lines.
-        handler._log_render = _NoExpandLogRender(  # pylint: disable=protected-access
+        no_expand = _make_non_expanding_log_render(
             show_time=False, show_level=False, show_path=False
         )
+        handler._log_render = no_expand  # pylint: disable=protected-access
 
     logging.basicConfig(
         level=logging.INFO,

dfetch/project/svnsubproject.py

@@ -66,6 +66,18 @@ def does_revision_exist(self, revision: str) -> bool:
             "In SVN only a revision is NOT enough, this should not be called!"
         )
 
+    def latest_available_version(self, wanted: Version) -> Version | None:
+        """Return the latest version matching *wanted*, or None if unavailable.
+
+        For revision-only pins (no branch, no tag) the pinned revision is
+        returned with the default branch so version comparison works correctly.
+        SVN revisions are globally ordered within a repository, so a bare
+        ``revision:`` in the manifest is always relative to trunk.
+        """
+        if wanted.revision and not wanted.branch and not wanted.tag:
+            return Version(revision=wanted.revision, branch=self.get_default_branch())
+        return super().latest_available_version(wanted)
+
     def browse_tree(
         self, version: str
     ) -> AbstractContextManager[Callable[[str], list[tuple[str, bool]]]]:

tests/test_subproject.py

@@ -3,7 +3,7 @@
 # mypy: ignore-errors
 # flake8: noqa
 
-from contextlib import ExitStack
+from contextlib import ExitStack, nullcontext
 from typing import Optional, Union
 from unittest.mock import MagicMock, patch
 
@@ -48,7 +48,7 @@ def does_revision_exist(self, revision: str) -> bool:
         return True
 
     def browse_tree(self, version: str) -> object:
-        return None
+        return nullcontext(lambda path="": [])
 
     def patch_type(self) -> PatchType:
         return PatchType.GIT