Actionable comments posted: 2

In `@dfetch/vcs/patch.py`:
- Around line 310-313: The header construction needs to special-case /dev/null
to avoid producing double slashes; update the loop that sets file.header for
items in patch.items so that when file.source == b"/dev/null" you do not prefix
it with b"a/" (and likewise when file.target == b"/dev/null" you do not prefix
it with b"b/"), building the header from chosen left/right parts (e.g., left =
file.source if file.source == b"/dev/null" else b"a/" + file.source; right =
file.target if file.target == b"/dev/null" else b"b/" + file.target) and then
set file.header = [b"diff --git " + left + b" " + right + b"\n"].
- Around line 304-321: Change the two separate if branches in convert_patch_to
to an if/elif (check patch_ng.GIT then elif patch_ng.SVN) so the GIT and SVN
handling are explicit and mutually exclusive, and after attempting conversion
validate the required_type: if it is not one of patch_ng.GIT or patch_ng.SVN
raise a clear error (e.g., ValueError) instead of silently setting patch.type;
ensure you still update each file.header and file.type inside the respective
branch and only set patch.type = required_type when a known conversion was
performed.

tests/test_patch.py (1)

365-366: Minor type mismatch: parse_patch expects str but receives pathlib.Path.

original_patch_file is a pathlib.Path (from tmp_path / "original.patch"), but parse_patch(file_path: str) declares str. This works at runtime since patch_ng.fromfile accepts path-like objects, but it's inconsistent with the type annotation. Consider either updating the annotation in parse_patch to accept str | pathlib.Path, or wrapping with str(original_patch_file) here.

Option A: Fix at the call site

-    parsed_patch = parse_patch(original_patch_file)
+    parsed_patch = parse_patch(str(original_patch_file))

dfetch/commands/format_patch.py (1)

153-162: _determine_target_patch_type — remove redundant str() conversion.

All three constants (patch_ng.GIT, patch_ng.SVN, patch_ng.PLAIN) exist and are used consistently throughout the codebase. The isinstance-based dispatch is clean and the function is appropriately module-private.

However, the str() cast on line 162 is redundant. These constants are already strings—the function signature declares a -> str return type, and in convert_patch_to (line 306), required_type is compared directly to patch.type with ==, which works only because both are strings. Remove the str() wrapper for clarity: return required_type.

Actionable comments posted: 1

In `@pyproject.toml`:
- Line 100: The inline comment next to the dependency string 'setuptools;
python_version >= "3.12"' is stale because sphinxcontrib.details.directive is
being removed; either remove this setuptools marker from the docs extras if
nothing else requires pkg_resources, or update the comment to name the actual
consumer that needs setuptools (e.g., the specific Sphinx extension importing
pkg_resources). Locate the dependency string 'setuptools; python_version >=
"3.12"' in the pyproject.toml extras section and either delete the comment or
replace it with an accurate explanation of why setuptools is present.

doc/_ext/scenario_directive.py (1)

81-96: Approach looks good — raw HTML replaces the removed sphinxcontrib-details-directive.

The sequential .. raw:: html blocks wrapping the literalinclude will produce a valid collapsible <details> element in the final output. One minor concern: scenario_title on line 84 is interpolated directly into raw HTML without escaping. If any Gherkin scenario title contains characters like <, >, or &, the HTML could break.

Optional: escape the title

+import html
 ...
-   <summary><strong>Example</strong>: {scenario_title}</summary>
+   <summary><strong>Example</strong>: {html.escape(scenario_title)}</summary>

dfetch/commands/format_patch.py (1)

153-162: str() wrapping is redundant — patch_ng.GIT / SVN / PLAIN are already strings.

Very minor: patch_ng.GIT, patch_ng.SVN, and patch_ng.PLAIN are string constants in patch-ng, so return str(required_type) could simply be return required_type. Not a problem, just a nit.