diff --git a/src/tenzir_ship/cli/_core.py b/src/tenzir_ship/cli/_core.py index 231cc91..f51302e 100644 --- a/src/tenzir_ship/cli/_core.py +++ b/src/tenzir_ship/cli/_core.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re import sys from dataclasses import dataclass from importlib.metadata import PackageNotFoundError, version as metadata_version @@ -11,6 +12,7 @@ Callable, Iterable, Mapping, + NamedTuple, Optional, TypeVar, ) @@ -64,6 +66,8 @@ "_command_help_text", "_format_author", "_format_section_title", + "PrRef", + "_parse_pr_refs", "_parse_pr_numbers", "_build_prs_structured", "_build_authors_structured", @@ -567,34 +571,66 @@ def _type_emoji(entry_type: str, *, include_emoji: bool = True) -> str: return "\u2022" -def _parse_pr_numbers(metadata: Mapping[str, Any]) -> list[int]: - """Extract PR numbers from metadata, handling various formats.""" +GITHUB_PR_URL_RE = re.compile( + r"^https://github\.com/(?P[^/\s]+/[^/\s]+)/pull/(?P\d+)/?$" +) + + +class PrRef(NamedTuple): + """A pull-request reference. + + A bare number is relative to the project's configured repository, so its + URL can only be built when that is known. An explicit URL carries its own + repository, which is what lets entries reference pull requests outside the + configured one. + """ + + number: int | None + url: str | None + + @property + def label(self) -> str: + return f"#{self.number}" if self.number is not None else (self.url or "") + + +def _parse_pr_ref(item: Any, repository: str | None) -> PrRef | None: + """Parse a single PR reference, or return None if it is not one.""" + if isinstance(item, bool): + return None + if isinstance(item, int): + number = item + else: + if not isinstance(item, str): + return None + text = item.strip() + if not text: + return None + match = GITHUB_PR_URL_RE.match(text) + if match: + # Keep the URL verbatim: it may point at a different repository + # than the configured one, which is the whole point of allowing it. + return PrRef(number=int(match.group("number")), url=text) + text = text.lstrip("#") + if not text.isdigit(): + return None + number = int(text) + url = f"https://github.com/{repository}/pull/{number}" if repository else None + return PrRef(number=number, url=url) + + +def _parse_pr_refs(metadata: Mapping[str, Any], repository: str | None = None) -> list[PrRef]: + """Extract PR references from metadata, handling numbers and full URLs.""" raw = metadata.get("prs") if raw is None: return [] - if isinstance(raw, int): - return [raw] - if isinstance(raw, str): - raw = raw.strip() - if not raw: - return [] - if raw.startswith("#"): - raw = raw[1:] - try: - return [int(raw)] - except ValueError: - return [] - if isinstance(raw, list): - result: list[int] = [] - for item in raw: - if isinstance(item, int): - result.append(item) - elif isinstance(item, str): - item = item.strip().lstrip("#") - if item.isdigit(): - result.append(int(item)) - return result - return [] + items = raw if isinstance(raw, list) else [raw] + refs = [_parse_pr_ref(item, repository) for item in items] + return [ref for ref in refs if ref is not None] + + +def _parse_pr_numbers(metadata: Mapping[str, Any]) -> list[int]: + """Extract PR numbers from metadata, handling various formats.""" + return [ref.number for ref in _parse_pr_refs(metadata) if ref.number is not None] def _build_prs_structured( @@ -602,10 +638,12 @@ def _build_prs_structured( ) -> list[dict[str, str | int]]: """Build structured PR metadata for JSON export.""" prs: list[dict[str, str | int]] = [] - for num in _parse_pr_numbers(metadata): - entry: dict[str, str | int] = {"number": num} - if config.repository: - entry["url"] = f"https://github.com/{config.repository}/pull/{num}" + for ref in _parse_pr_refs(metadata, config.repository): + entry: dict[str, str | int] = {} + if ref.number is not None: + entry["number"] = ref.number + if ref.url: + entry["url"] = ref.url prs.append(entry) return prs @@ -699,16 +737,12 @@ def _collect_author_pr_text( author_handles = [_format_author(author, explicit_links=explicit_links) for author in authors] author_text = _join_with_conjunction(author_handles) - prs = _parse_pr_numbers(metadata) - - repo = config.repository pr_refs: list[str] = [] - for pr in prs: - label = f"#{pr}" - if explicit_links and repo: - pr_refs.append(f"[{label}](https://github.com/{repo}/pull/{pr})") + for ref in _parse_pr_refs(metadata, config.repository): + if explicit_links and ref.url: + pr_refs.append(f"[{ref.label}]({ref.url})") else: - pr_refs.append(label) + pr_refs.append(ref.label) pr_text = _join_with_conjunction(pr_refs) return author_text, pr_text diff --git a/tests/test_cli.py b/tests/test_cli.py index 2e589cc..9c7eb2d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -15,6 +15,7 @@ from tenzir_ship import __version__ from tenzir_ship.cli import INFO_PREFIX, cli, main +from tenzir_ship.cli._core import _parse_pr_numbers, _parse_pr_refs from tenzir_ship.cli._core import create_cli_context from tenzir_ship.cli._show import _collect_unused_entries_for_release from tenzir_ship.config import Config, ReleaseConfig, load_config, save_config @@ -9145,3 +9146,82 @@ def test_validate_rejects_duplicate_ids_within_one_manifest(tmp_path: Path) -> N assert result.exit_code != 0 assert "non-unique elements" in result.output + + +def test_parse_pr_refs_accepts_numbers_and_full_urls() -> None: + """PR references may be bare numbers or full URLs to any repository.""" + refs = _parse_pr_refs( + { + "prs": [ + 6368, + "#42", + "https://github.com/tenzir/tenzir/pull/6441", + "https://github.com/tenzir/mono/pull/35/", + "not-a-pr", + ] + }, + "tenzir/mono", + ) + + assert [ref.number for ref in refs] == [6368, 42, 6441, 35] + # Bare numbers resolve against the configured repository; explicit URLs + # keep whatever repository they name. + assert refs[0].url == "https://github.com/tenzir/mono/pull/6368" + assert refs[1].url == "https://github.com/tenzir/mono/pull/42" + assert refs[2].url == "https://github.com/tenzir/tenzir/pull/6441" + assert refs[3].url == "https://github.com/tenzir/mono/pull/35/" + + +def test_parse_pr_refs_without_repository_keeps_explicit_urls() -> None: + """A project without a configured repository can still carry URLs.""" + refs = _parse_pr_refs( + {"prs": [7, "https://github.com/tenzir/tenzir/pull/6441"]}, + None, + ) + + assert refs[0].url is None + assert refs[1].url == "https://github.com/tenzir/tenzir/pull/6441" + + +def test_parse_pr_numbers_still_returns_plain_numbers() -> None: + """The display-only helper keeps its list[int] contract.""" + assert _parse_pr_numbers({"prs": [1, "#2", "https://github.com/tenzir/tenzir/pull/3"]}) == [ + 1, + 2, + 3, + ] + + +def test_explicit_links_uses_the_entry_url_for_foreign_repositories( + tmp_path: Path, +) -> None: + """A full URL must survive rendering instead of being rebuilt from config.""" + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / "config.yaml").write_text( + "id: test-project\nname: Test Project\nrepository: tenzir/mono\n", + encoding="utf-8", + ) + entry_dir = project_dir / "unreleased" + entry_dir.mkdir(parents=True) + (entry_dir / "foreign-pr.md").write_text( + "---\n" + "title: Foreign PR reference\n" + "type: feature\n" + "authors:\n" + " - someone\n" + "prs:\n" + " - https://github.com/tenzir/tenzir/pull/6441\n" + "---\n\nBody.\n", + encoding="utf-8", + ) + + result = runner.invoke( + cli, + ["--root", str(project_dir), "show", "-m", "--explicit-links", "unreleased"], + ) + + assert result.exit_code == 0, result.output + assert "[#6441](https://github.com/tenzir/tenzir/pull/6441)" in result.output + assert "tenzir/mono/pull/6441" not in result.output