Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 58 additions & 13 deletions pr_split/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ def _add_children(parent_id: str, prefix: str) -> None:
return f"## Dependency graph\n\nMerge in this order:\n\n```\n{tree_block}\n```"


def _check_pr_template() -> None:
try:
validate_pr_template()
except PRSplitError as exc:
console.print(f"[red]{exc}[/red]")
raise typer.Exit(1) from exc


def _require_local_branch(base: str) -> None:
"""Exit unless ``base`` names a local branch head.

Expand Down Expand Up @@ -226,6 +234,8 @@ def _validate_inputs(
if not dry_run and not check_gh_auth():
console.print(f"[red]{ErrorMsg.GH_AUTH_FAILED()}[/red]")
raise typer.Exit(1)
if not dry_run:
_check_pr_template()
if not dry_run and stacked:
_require_gh_stack()

Expand Down Expand Up @@ -565,11 +575,56 @@ def _format_file_list(files: list[str]) -> str:
return "\n".join(lines)


_PR_TEMPLATE_PLACEHOLDERS = (
"description",
"files",
"added",
"removed",
"loc",
"dependencies",
"dag",
"id",
"title",
)


def _render_pr_template(template_vars: dict[str, object]) -> str:
template_path = _pr_template_path()
try:
template = template_path.read_text(encoding="utf-8")
return template.format(**template_vars)
except (KeyError, ValueError, IndexError, AttributeError, TypeError) as exc:
# AttributeError/TypeError cover "{title.nope}" and "{added[0]}",
# which str.format raises as plain attribute/subscript errors.
available = ", ".join(f"{{{k}}}" for k in sorted(template_vars))
raise PRSplitError(
f"Invalid PR template at {template_path}: {exc}. "
f"Available placeholders: {available}. "
"Escape literal braces with {{ and }}."
) from exc
except OSError as exc:
raise PRSplitError(f"Could not read PR template at {template_path}: {exc}") from exc


def validate_pr_template() -> None:
"""Render the custom template against placeholder values.

The template is otherwise first used when the PRs are opened, i.e. after
every branch has been created and pushed; a typo in it must fail before
that. Raises PRSplitError with the same message the real render would.
"""
if not _pr_template_path().exists():
return
sample: dict[str, object] = {k: k for k in _PR_TEMPLATE_PLACEHOLDERS}
sample.update({"added": 0, "removed": 0, "loc": 0})
_render_pr_template(sample)


def _build_pr_body(group: Group, all_groups: list[Group]) -> str:
template_path = _pr_template_path()
if template_path.exists():
files = [a.file_path for a in group.assignments]
template_vars = {
template_vars: dict[str, object] = {
"description": group.description,
"files": _format_file_list(files),
"added": group.estimated_added,
Expand All @@ -580,18 +635,7 @@ def _build_pr_body(group: Group, all_groups: list[Group]) -> str:
"id": group.id,
"title": group.title,
}
try:
template = template_path.read_text(encoding="utf-8")
return template.format(**template_vars)
except (KeyError, ValueError, IndexError) as exc:
available = ", ".join(f"{{{k}}}" for k in sorted(template_vars))
raise PRSplitError(
f"Invalid PR template at {template_path}: {exc}. "
f"Available placeholders: {available}. "
"Escape literal braces with {{ and }}."
) from exc
except OSError as exc:
raise PRSplitError(f"Could not read PR template at {template_path}: {exc}") from exc
return _render_pr_template(template_vars)

files = [a.file_path for a in group.assignments]
sections = [group.description]
Expand Down Expand Up @@ -1968,6 +2012,7 @@ def execute(
if not check_gh_auth():
console.print(f"[red]{ErrorMsg.GH_AUTH_FAILED()}[/red]")
raise typer.Exit(1)
_check_pr_template()
if plan.stacked:
_require_gh_stack()

Expand Down
89 changes: 89 additions & 0 deletions tests/test_cli_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,95 @@ def test_template_os_error(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
_build_pr_body(group, [group])


class TestPrTemplateEarlyValidation:
def _use_template(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, text: str) -> None:
template_file = tmp_path / "template.md"
template_file.write_text(text, encoding="utf-8")
monkeypatch.setattr("pr_split.cli._pr_template_path", lambda: template_file)

@pytest.mark.parametrize(
"text",
["{title.nope}", "{added[0]}", "{unknown}", "{title", "{0}"],
)
def test_bad_placeholders_are_friendly_errors(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, text: str
) -> None:
from pr_split.cli import validate_pr_template

self._use_template(tmp_path, monkeypatch, text)
with pytest.raises(PRSplitError, match="Invalid PR template"):
validate_pr_template()
group = _group("pr-1", "t", files=["a.py"])
with pytest.raises(PRSplitError, match="Invalid PR template"):
_build_pr_body(group, [group])

def test_valid_template_passes_and_renders(
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
from pr_split.cli import validate_pr_template

self._use_template(tmp_path, monkeypatch, "# {title}\n\n{files}\n\n{{literal}} {loc}")
validate_pr_template()
group = _group("pr-1", "t", files=["a.py"])
body = _build_pr_body(group, [group])
assert body.startswith("# t\n\n- `a.py`")
assert "{literal} 15" in body

@patch("pr_split.cli.check_gh_auth", return_value=True)
@patch("pr_split.cli.is_worktree_clean", return_value=True)
@patch("pr_split.cli.branch_exists", return_value=True)
def test_split_inputs_reject_a_bad_template_before_planning(
self,
mock_be: MagicMock,
mock_clean: MagicMock,
mock_auth: MagicMock,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
from pr_split.cli import console

self._use_template(tmp_path, monkeypatch, "{title.nope}")
with console.capture() as capture, pytest.raises(typer.Exit):
_validate_inputs("feature", "main")
assert "Invalid PR template" in " ".join(capture.get().split())
# a dry run never opens PRs, so the template is not consulted
_validate_inputs("feature", "main", dry_run=True)

@patch("pr_split.cli._create_branches_and_commits")
@patch("pr_split.cli.typer.confirm", return_value=True)
@patch("pr_split.cli.check_gh_auth", return_value=True)
@patch("pr_split.cli.is_worktree_clean", return_value=True)
@patch("pr_split.cli.commit_exists", return_value=True)
@patch("pr_split.cli.branch_exists", return_value=True)
@patch("pr_split.cli.load_plan")
@patch("pr_split.cli.plan_exists", return_value=True)
def test_execute_rejects_a_bad_template_before_creating_branches(
self,
mock_pe: MagicMock,
mock_load: MagicMock,
mock_be: MagicMock,
mock_commit: MagicMock,
mock_clean: MagicMock,
mock_auth: MagicMock,
mock_confirm: MagicMock,
mock_create: MagicMock,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
self._use_template(tmp_path, monkeypatch, "{added[0]}")
mock_plan_file = MagicMock()
mock_plan_file.git_state.branches = []
mock_plan_file.git_state.prs = []
mock_plan_file.plan.raw_diff = "some diff"
mock_plan_file.plan.merge_base_sha = "abc123"
mock_load.return_value = mock_plan_file
result = runner.invoke(app, ["execute"])
assert result.exit_code == 1
assert "Invalid PR template" in " ".join(result.output.split())
mock_confirm.assert_not_called()
mock_create.assert_not_called()


# ---------------------------------------------------------------------------
# _send_webhook
# ---------------------------------------------------------------------------
Expand Down
Loading