diff --git a/docs/git-draft.adoc b/docs/git-draft.adoc index 4587e0a..46b624b 100644 --- a/docs/git-draft.adoc +++ b/docs/git-draft.adoc @@ -74,10 +74,6 @@ git draft [options] --show-templates [--json | [--edit] TEMPLATE] Lists available templates. With an template name argument, displays the corresponding template's contents or, if the `--edit` option is set, opens an interactive editor. --t TIMEOUT:: ---timeout=TIMEOUT:: - Action timeout. - --version:: Show version and exit. diff --git a/src/git_draft/__main__.py b/src/git_draft/__main__.py index 4c06575..3efa68c 100644 --- a/src/git_draft/__main__.py +++ b/src/git_draft/__main__.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Sequence +import enum import importlib.metadata import logging import optparse @@ -11,7 +12,7 @@ from .bots import load_bot from .common import PROGRAM, Config, UnreachableError, ensure_state_home -from .drafter import Accept, Drafter +from .drafter import Drafter, DraftMergeStrategy from .editor import open_editor from .git import Repo from .prompt import Template, TemplatedPrompt, find_template, templates_table @@ -95,15 +96,28 @@ def callback( action="store_const", const=0, ) - parser.add_option( - "--timeout", - dest="timeout", - help="generation timeout", - ) return parser +class Accept(enum.Enum): + """Valid change accept mode""" + + MANUAL = 0 + MERGE = enum.auto() + MERGE_THEIRS = enum.auto() + FINALIZE = enum.auto() + + def merge_strategy(self) -> DraftMergeStrategy | None: + match self.value: + case Accept.MANUAL: + return None + case Accept.MERGE: + return "ignore-all-space" + case _: + return "theirs" + + class ToolPrinter(ToolVisitor): """Visitor implementation which prints invocations to stdout""" @@ -199,17 +213,17 @@ def main() -> None: # noqa: PLR0912 PLR0915 drafter.generate_draft( prompt, bot, - accept=accept, - bot_name=opts.bot, prompt_transform=open_editor if editable else None, + merge_strategy=accept.merge_strategy(), tool_visitors=[ToolPrinter()], ) match accept: case Accept.MANUAL: print("Generated draft.") - case Accept.MERGE: + case Accept.MERGE | Accept.MERGE_THEIRS: print("Merged draft.") case Accept.FINALIZE: + drafter.finalize_folio() print("Finalized draft.") case _: raise UnreachableError() diff --git a/src/git_draft/bots/common.py b/src/git_draft/bots/common.py index e1da2ff..d9a9df8 100644 --- a/src/git_draft/bots/common.py +++ b/src/git_draft/bots/common.py @@ -14,7 +14,7 @@ class Goal: """Bot request""" prompt: str - timeout: float | None + # TODO: Add timeout. @dataclasses.dataclass diff --git a/src/git_draft/drafter.py b/src/git_draft/drafter.py index 9191d99..47b99c8 100644 --- a/src/git_draft/drafter.py +++ b/src/git_draft/drafter.py @@ -5,13 +5,13 @@ from collections.abc import Callable, Sequence import dataclasses from datetime import datetime, timedelta -import enum import json import logging from pathlib import PurePosixPath import re import textwrap import time +from typing import Literal from .bots import Action, Bot, Goal from .common import JSONObject, qualified_class_name @@ -24,14 +24,6 @@ _logger = logging.getLogger(__name__) -class Accept(enum.Enum): - """Valid change accept mode""" - - MANUAL = 0 - MERGE = enum.auto() - FINALIZE = enum.auto() - - @dataclasses.dataclass(frozen=True) class Draft: """Generated changes""" @@ -81,6 +73,18 @@ def _active_folio(repo: Repo) -> Folio | None: return Folio(int(match[1])) +#: Select ort strategies. +DraftMergeStrategy = Literal[ + "ours", + "theirs", + "ignore-space-change", + "ignore-all-space", + "ignore-space-at-eol", + "ignore-cr-at-eol", + "find-renames", +] + + class Drafter: """Draft state orchestrator""" @@ -94,19 +98,14 @@ def create(cls, repo: Repo, store: Store) -> Drafter: cursor.executescript(sql("create-tables")) return cls(store, repo) - def generate_draft( # noqa: PLR0913 + def generate_draft( self, prompt: str | TemplatedPrompt, bot: Bot, - accept: Accept = Accept.MANUAL, - bot_name: str | None = None, + merge_strategy: DraftMergeStrategy | None = None, prompt_transform: Callable[[str], str] | None = None, - timeout: float | None = None, tool_visitors: Sequence[ToolVisitor] | None = None, ) -> Draft: - if timeout is not None: - raise NotImplementedError() # TODO: Implement - # Handle prompt templating and editing. We do this first in case this # fails, to avoid creating unnecessary branches. toolbox, dirty = RepoToolbox.for_working_dir(self._repo) @@ -134,7 +133,7 @@ def generate_draft( # noqa: PLR0913 operation_recorder = _OperationRecorder() change = self._generate_change( bot, - Goal(prompt_contents, timeout), + Goal(prompt_contents), toolbox.with_visitors( [operation_recorder, *list(tool_visitors or [])], ), @@ -159,7 +158,6 @@ def generate_draft( # noqa: PLR0913 { "commit_sha": commit_sha, "prompt_id": prompt_id, - "bot_name": bot_name, "bot_class": qualified_class_name(bot.__class__), "walltime_seconds": change.walltime.total_seconds(), "request_count": change.action.request_count, @@ -181,18 +179,21 @@ def generate_draft( # noqa: PLR0913 ) _logger.info("Created new change in folio %s.", folio.id) - if accept.value >= Accept.MERGE.value: + if merge_strategy: + if parent_commit_rev != "HEAD": + # If there was a sync(prompt) commit, we move forward to it. + # This will avoid conflicts with changes that happened earlier. + self._repo.git("reset", "--soft", parent_commit_rev) self._sync_head("merge") self._repo.git( "merge", "--no-ff", - "-Xtheirs", + "-X", + merge_strategy, "-m", "draft! merge", commit_sha, ) - if accept.value >= Accept.FINALIZE.value: - self.finalize_folio() return Draft( folio=folio, diff --git a/src/git_draft/git.py b/src/git_draft/git.py index 98fb87c..1a92852 100644 --- a/src/git_draft/git.py +++ b/src/git_draft/git.py @@ -50,9 +50,10 @@ def sync( stdout, stderr = popen.communicate(input=stdin) code = popen.returncode if expect_codes and code not in expect_codes: - raise GitError( - f"Git command failed with code {code}\n{stderr}\n{stdout}" - ) + message = f"Git command failed with code exit {code}: {stderr}" + if stdout: + message += f"\n{stdout}" + raise GitError(message) return cls(code, stdout.rstrip(), stderr.rstrip()) diff --git a/src/git_draft/queries/add-action.sql b/src/git_draft/queries/add-action.sql index 990994b..c438cd2 100644 --- a/src/git_draft/queries/add-action.sql +++ b/src/git_draft/queries/add-action.sql @@ -1,7 +1,6 @@ insert into actions ( commit_sha, prompt_id, - bot_name, bot_class, walltime_seconds, request_count, @@ -9,7 +8,6 @@ insert into actions ( values ( :commit_sha, :prompt_id, - :bot_name, :bot_class, :walltime_seconds, :request_count, diff --git a/src/git_draft/queries/create-tables.sql b/src/git_draft/queries/create-tables.sql index 0b4ac3c..2e1e60c 100644 --- a/src/git_draft/queries/create-tables.sql +++ b/src/git_draft/queries/create-tables.sql @@ -23,7 +23,6 @@ create table if not exists actions ( commit_sha text primary key, created_at timestamp default current_timestamp, prompt_id integer not null, - bot_name text, bot_class text not null, walltime_seconds real not null, request_count int, diff --git a/tests/git_draft/drafter_test.py b/tests/git_draft/drafter_test.py index 6c9c841..a57112b 100644 --- a/tests/git_draft/drafter_test.py +++ b/tests/git_draft/drafter_test.py @@ -6,7 +6,7 @@ from git_draft.bots import Action, Bot, Goal, Toolbox import git_draft.drafter as sut -from git_draft.git import SHA, Repo +from git_draft.git import SHA, GitError, Repo from git_draft.store import Store from .conftest import RepoFS @@ -71,28 +71,61 @@ def test_generate_empty_draft(self) -> None: assert len(self._commits()) == 1 assert len(self._commits("@{u}")) == 2 - def test_generate_draft_accept_merge(self) -> None: + def test_generate_draft_merge(self) -> None: self._fs.write("p1", "a") + self._drafter.generate_draft( - "hello", - _SimpleBot({"p2": "b"}), - accept=sut.Accept.MERGE, + "hello", _SimpleBot({"p2": "b"}), merge_strategy="ignore-all-space" ) - assert len(self._commits()) == 5 # init, sync, prompt, sync, merge + # No sync(merge) commit since no changes happened between. + assert len(self._commits()) == 4 # init, sync(prompt), prompt, merge assert self._fs.read("p1") == "a" assert self._fs.read("p2") == "b" - def test_generate_draft_accept_finalize(self) -> None: + def test_generate_draft_merge_no_conflict(self) -> None: self._fs.write("p1", "a") + + def update(_goal: Goal) -> str: + self._fs.write("p2", "b") + return "A" + self._drafter.generate_draft( "hello", - _SimpleBot({"p1": "A", "p2": "b"}), - accept=sut.Accept.FINALIZE, + _SimpleBot({"p1": update}), + merge_strategy="ignore-all-space", ) - assert len(self._commits()) == 1 # init + assert len(self._commits()) == 5 # init, sync, prompt, sync, merge assert self._fs.read("p1") == "A" assert self._fs.read("p2") == "b" + def test_generate_draft_merge_theirs(self) -> None: + self._fs.write("p1", "a") + + def update(_goal: Goal) -> str: + self._fs.write("p1", "b") + return "A" + + self._drafter.generate_draft( + "hello", _SimpleBot({"p1": update}), merge_strategy="theirs" + ) + # sync(merge) commit here since p1 was updated separately. + assert len(self._commits()) == 5 # init, sync, prompt, sync, merge + assert self._fs.read("p1") == "A" + + def test_generate_draft_merge_conflict(self) -> None: + self._fs.write("p1", "a") + + def update(_goal: Goal) -> str: + self._fs.write("p1", "b") + return "A" + + with pytest.raises(GitError): + self._drafter.generate_draft( + "hello", + _SimpleBot({"p1": update}), + merge_strategy="ignore-all-space", + ) + def test_generate_outside_branch(self) -> None: self._repo.git("checkout", "--detach") with pytest.raises(RuntimeError): @@ -104,8 +137,8 @@ def test_generate_empty_prompt(self) -> None: def test_generate_reuse_branch(self) -> None: bot = _SimpleBot({"prompt": lambda goal: goal.prompt}) - self._drafter.generate_draft("prompt1", bot, sut.Accept.MERGE) - self._drafter.generate_draft("prompt2", bot, sut.Accept.MERGE) + self._drafter.generate_draft("prompt1", bot, "theirs") + self._drafter.generate_draft("prompt2", bot, "theirs") assert self._fs.read("prompt") == "prompt2" def test_delete_unknown_file(self) -> None: @@ -113,9 +146,7 @@ def test_delete_unknown_file(self) -> None: def test_finalize_keeps_changes(self) -> None: self._fs.write("p1.txt", "a1") - self._drafter.generate_draft( - "hello", _SimpleBot.prompt(), sut.Accept.MERGE - ) + self._drafter.generate_draft("hello", _SimpleBot.prompt(), "theirs") self._fs.write("p1.txt", "a2") self._drafter.finalize_folio() assert self._fs.read("p1.txt") == "a2"