feat: simplify finalize options (#58)

This change removes the revert and clean options. This simplifies the
overall API: finalize never touches the worktree.

Author: mtth

docs/git-draft.adoc

@@ -19,7 +19,7 @@ IMPORTANT: `git-draft` is WIP.
 
 [verse]
 git draft [options] [--generate] [--bot BOT] [--edit] [--reset | --no-reset] [--sync] [TEMPLATE [VARIABLE...]]
-git draft [options] --finalize [--clean | --revert] [--delete]
+git draft [options] --finalize [--delete]
 git draft [options] --show-drafts [--json]
 git draft [options] --show-prompts [--json] [PROMPT]
 git draft [options] --show-templates [--json | [--edit] TEMPLATE]
@@ -36,10 +36,6 @@ git draft [options] --show-templates [--json | [--edit] TEMPLATE]
 --bot=BOT::
   Bot name.
 
--c::
---clean::
-  TODO
-
 -d::
 --delete::
   Delete finalized branch.
@@ -71,10 +67,6 @@ git draft [options] --show-templates [--json | [--edit] TEMPLATE]
 --no-reset::
   TODO
 
--r::
---revert::
-  Abandon any changes since draft creation.
-
 --root::
   Repository search root.
 

src/git_draft/__main__.py

@@ -70,12 +70,6 @@ def callback(
         dest="bot",
         help="bot name",
     )
-    parser.add_option(
-        "-c",
-        "--clean",
-        help="remove deleted files from work directory",
-        action="store_true",
-    )
     parser.add_option(
         "-d",
         "--delete",
@@ -94,12 +88,6 @@ def callback(
         help="use JSON for table output",
         action="store_true",
     )
-    parser.add_option(
-        "-r",
-        "--revert",
-        help="abandon any changes since draft creation",
-        action="store_true",
-    )
     parser.add_option(
         "-s",
         "--sync",
@@ -218,11 +206,8 @@ def main() -> None:  # noqa: PLR0912 PLR0915
         )
         print(f"Refined {name}.")
     elif command == "finalize":
-        name = drafter.exit_draft(
-            revert=opts.revert, clean=opts.clean, delete=opts.delete
-        )
-        verb = "Reverted" if opts.revert else "Finalized"
-        print(f"{verb} {name}.")
+        name = drafter.finalize_draft(delete=opts.delete)
+        print(f"Finalized {name}.")
     elif command == "show-drafts":
         table = drafter.history_table(args[0] if args else None)
         if table:

src/git_draft/drafter.py

@@ -7,8 +7,6 @@
 from datetime import datetime
 import json
 import logging
-import os
-import os.path as osp
 from pathlib import PurePosixPath
 import re
 from re import Match
@@ -175,9 +173,7 @@ def generate_draft(  # noqa: PLR0913
         _logger.info("Completed generation for %s.", branch)
         return str(branch)
 
-    def exit_draft(
-        self, *, revert: bool, clean: bool = False, delete: bool = False
-    ) -> str:
+    def finalize_draft(self, *, delete: bool = False) -> str:
         branch = _Branch.active(self._repo)
         if not branch:
             raise RuntimeError("Not currently on a draft branch")
@@ -190,53 +186,13 @@ def exit_draft(
                 raise RuntimeError("Unrecognized draft branch")
             [(origin_branch, origin_sha, sync_sha)] = rows
 
-        if (
-            revert
-            and sync_sha
-            and self._repo.commit(origin_branch).hexsha != origin_sha
-        ):
-            raise RuntimeError("Parent branch has moved, please rebase first")
-
-        if clean and not revert:
-            # We delete files which have been deleted in the draft manually,
-            # otherwise they would still show up as untracked.
-            origin_delta = self._delta(f"{origin_branch}..{branch}")
-            deleted = self._untracked() & origin_delta.deleted
-            for path in deleted:
-                os.remove(osp.join(self._repo.working_dir, path))
-            _logger.info("Cleaned up files. [deleted=%s]", deleted)
-
         # We do a small dance to move back to the original branch, keeping the
         # draft branch untouched. See https://stackoverflow.com/a/15993574 for
         # the inspiration.
         self._repo.git.checkout(detach=True)
         self._repo.git.reset("-N", origin_branch)
         self._repo.git.checkout(origin_branch)
 
-        if revert:
-            # We revert the relevant files if needed. If a sync commit had been
-            # created, we simply revert to it. Otherwise we compute which files
-            # have changed due to draft commits and revert only those.
-            if sync_sha:
-                delta = self._delta(sync_sha)
-                if delta.changed:
-                    self._repo.git.checkout(sync_sha, "--", ".")
-                _logger.info("Reverted to sync commit. [sha=%s]", sync_sha)
-            else:
-                origin_delta = self._delta(f"{origin_branch}..{branch}")
-                head_delta = self._delta("HEAD")
-                changed = head_delta.touched & origin_delta.changed
-                if changed:
-                    self._repo.git.checkout("--", *changed)
-                deleted = head_delta.touched & origin_delta.deleted
-                if deleted:
-                    self._repo.git.rm("--", *deleted)
-                _logger.info(
-                    "Reverted touched files. [changed=%s, deleted=%s]",
-                    changed,
-                    deleted,
-                )
-
         if delete:
             self._repo.git.branch("-D", branch.name)
             _logger.debug("Deleted branch %s.", branch)

tests/git_draft/drafter_test.py

@@ -70,11 +70,6 @@ def act(self, _goal: Goal, toolbox: Toolbox) -> Action:
         self._drafter.generate_draft("hello", CustomBot())
         assert self._commit_files("HEAD") == set(["p2", "p3"])
 
-    def test_generate_then_revert_draft(self) -> None:
-        self._drafter.generate_draft("hello", FakeBot())
-        self._drafter.exit_draft(revert=True)
-        assert len(self._commits()) == 1
-
     def test_generate_outside_branch(self) -> None:
         self._repo.git.checkout("--detach")
         with pytest.raises(RuntimeError):
@@ -140,98 +135,12 @@ def act(self, _goal: Goal, toolbox: Toolbox) -> Action:
 
         self._drafter.generate_draft("hello", CustomBot())
 
-    def test_sync_delete_revert(self) -> None:
-        self._write("p1", "a")
-        self._repo.git.add(all=True)
-        self._repo.index.commit("advance")
-        self._delete("p1")
-
-        class CustomBot(Bot):
-            def act(self, _goal: Goal, toolbox: Toolbox) -> Action:
-                toolbox.write_file(PurePosixPath("p2"), "b")
-                return Action()
-
-        self._drafter.generate_draft("hello", CustomBot(), sync=True)
-        assert self._read("p1") is None
-
-        self._drafter.exit_draft(revert=True)
-        assert self._read("p1") is None
-
-    def test_generate_delete_finalize_clean(self) -> None:
-        self._write("p1", "a")
-        self._repo.git.add(all=True)
-        self._repo.index.commit("advance")
-
-        class CustomBot(Bot):
-            def act(self, _goal: Goal, toolbox: Toolbox) -> Action:
-                toolbox.delete_file(PurePosixPath("p1"))
-                return Action()
-
-        self._drafter.generate_draft("hello", CustomBot())
-        assert self._read("p1") == "a"
-
-        self._drafter.exit_draft(revert=False, clean=True)
-        assert self._read("p1") is None
-
-    def test_revert_outside_draft(self) -> None:
-        with pytest.raises(RuntimeError):
-            self._drafter.exit_draft(revert=True)
-
-    def test_revert_after_branch_move(self) -> None:
-        self._write("log", "11")
-        self._drafter.generate_draft("hi", FakeBot(), sync=True)
-        branch = self._repo.active_branch
-        self._repo.git.checkout("main")
-        self._repo.index.commit("advance")
-        self._repo.git.checkout(branch)
-        with pytest.raises(RuntimeError):
-            self._drafter.exit_draft(revert=True)
-
-    def test_revert_restores_worktree(self) -> None:
-        self._write("p1.txt", "a1")
-        self._write("p2.txt", "b1")
-        self._drafter.generate_draft("hello", FakeBot(), sync=True)
-        self._write("p1.txt", "a2")
-        self._drafter.exit_draft(revert=True, delete=True)
-        assert self._read("p1.txt") == "a1"
-        assert self._read("p2.txt") == "b1"
-
-    def test_revert_discards_unused_files(self) -> None:
-        self._drafter.generate_draft("hello", FakeBot())
-        assert self._read("PROMPT") is None
-        self._drafter.exit_draft(revert=True)
-        assert self._read("PROMPT") is None
-
-    def test_revert_keeps_untouched_files(self) -> None:
-        class CustomBot(Bot):
-            def act(self, _goal: Goal, toolbox: Toolbox) -> Action:
-                toolbox.write_file(PurePosixPath("p2.txt"), "t2")
-                toolbox.write_file(PurePosixPath("p4.txt"), "t2")
-                return Action()
-
-        self._write("p1.txt", "t0")
-        self._write("p2.txt", "t0")
-        self._repo.git.add(all=True)
-        self._repo.index.commit("update")
-        self._write("p1.txt", "t1")
-        self._write("p2.txt", "t1")
-        self._write("p3.txt", "t1")
-        self._drafter.generate_draft("hello", CustomBot())
-        self._write("p1.txt", "t3")
-        self._write("p2.txt", "t3")
-        self._drafter.exit_draft(revert=True)
-
-        assert self._read("p1.txt") == "t3"
-        assert self._read("p2.txt") == "t0"
-        assert self._read("p3.txt") == "t1"
-        assert self._read("p4.txt") is None
-
     def test_finalize_keeps_changes(self) -> None:
         self._write("p1.txt", "a1")
         self._drafter.generate_draft("hello", FakeBot())
         self._checkout()
         self._write("p1.txt", "a2")
-        self._drafter.exit_draft(revert=False)
+        self._drafter.finalize_draft()
         assert self._read("p1.txt") == "a2"
         assert self._read("PROMPT") == "hello"