[PATCH b4 v2 17/44] edit_in_editor: work in the tree the caller names

[Christian Brauner <brauner@kernel.org>]

The scratch file, the branch lookup and the core.editor lookup all
came from the process cwd. The review TUI scopes every other git call
to session['topdir']; this was the one place where the tree was
implied rather than named. Add a topdir argument and have the review
TUI pass the one it already tracks.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/__init__.py               | 26 ++++++++++++++++++--------
 src/b4/review_tui/_review_app.py | 12 +++++++++---
 src/tests/test_tui_review.py     |  2 +-
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 3d22c89..a22c28d 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -6176,10 +6176,17 @@ def edit_in_editor(
     bdata: bytes,
     filehint: str = 'COMMIT_EDITMSG',
     *,
+    topdir: Optional[str] = None,
     guard_branch: bool = False,
 ) -> bytes:
     """Open the user's editor on bdata and return what they saved.
 
+    topdir is the working tree the edit belongs to: the scratch file is
+    created there, and it is the tree HEAD is read from.  It defaults to the
+    working tree of the current directory, which is only right for callers
+    that operate on the process cwd -- anything driving a specific worktree
+    (the TUIs) must pass it explicitly.
+
     guard_branch opts into a collision check, and only callers that store
     the result into whatever branch HEAD happens to point at may set it
     (b4 prep keeps the cover letter in the current branch's tracking commit,
@@ -6191,12 +6198,16 @@ def edit_in_editor(
     where their data lands, so refusing the edit would throw away the user's
     work to prevent a collision that cannot happen.
     """
-    # Read before the edit and compare after, so this can never end up
-    # comparing two different points in time.  A detached HEAD reads as None
-    # and still guards: None is not a branch we started on, so checking one
-    # out mid-edit is caught like any other switch.
-    read_branch = git_get_current_branch() if guard_branch else None
-    corecfg = get_config_from_git(r'core\..*')
+    if topdir is None:
+        topdir = git_get_toplevel()
+    # Read before the edit and compare after, both in topdir, so this can
+    # never end up comparing two different repositories' HEADs.  A detached
+    # HEAD reads as None and still guards: None is not a branch we started
+    # on, so checking one out mid-edit is caught like any other switch.
+    read_branch = git_get_current_branch(topdir) if guard_branch else None
+    # core.editor comes from the same tree as everything else here, so a
+    # repository-local setting is the one belonging to the edited branch.
+    corecfg = get_config_from_git(r'core\..*', gitdir=topdir)
     editor = (
         os.environ.get('GIT_EDITOR')
         or corecfg.get('editor')
@@ -6206,7 +6217,6 @@ def edit_in_editor(
     )
     logger.debug('editor=%s', editor)
 
-    topdir = git_get_toplevel()
     if topdir is not None:
         p = Path(topdir)
     else:
@@ -6233,7 +6243,7 @@ def edit_in_editor(
     bdata = bdata.replace(b'\r\n', b'\n').replace(b'\r', b'\n')
 
     if guard_branch:
-        write_branch = git_get_current_branch()
+        write_branch = git_get_current_branch(topdir)
         if write_branch != read_branch:
             with tempfile.NamedTemporaryFile(
                 mode='wb',
diff --git a/src/b4/review_tui/_review_app.py b/src/b4/review_tui/_review_app.py
index 8139518..74f6aa0 100644
--- a/src/b4/review_tui/_review_app.py
+++ b/src/b4/review_tui/_review_app.py
@@ -1329,7 +1329,9 @@ class ReviewApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[None]):
 
         with self.suspend():
             result = b4.edit_in_editor(
-                editor_text.encode(), filehint='reply.b4-review.eml'
+                editor_text.encode(),
+                filehint='reply.b4-review.eml',
+                topdir=self._topdir,
             )
 
         if not result:
@@ -1449,7 +1451,9 @@ class ReviewApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[None]):
         """Launch $EDITOR for the maintainer's note on *target*."""
         editor_text = existing + self._NOTE_FOOTER
         with self.suspend():
-            result = b4.edit_in_editor(editor_text.encode(), filehint='note.txt')
+            result = b4.edit_in_editor(
+                editor_text.encode(), filehint='note.txt', topdir=self._topdir
+            )
 
         if not result:
             self.notify('Editor returned no content')
@@ -1713,7 +1717,9 @@ class ReviewApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[None]):
             editor_text = f'On {orig_date}, {orig_author} wrote:\n{quoted}\n\n'
 
         with self.suspend():
-            result = b4.edit_in_editor(editor_text.encode(), filehint='reply.eml')
+            result = b4.edit_in_editor(
+                editor_text.encode(), filehint='reply.eml', topdir=self._topdir
+            )
         reply_text = result.decode(errors='replace')
         if reply_text == editor_text:
             self.notify('No changes made')
diff --git a/src/tests/test_tui_review.py b/src/tests/test_tui_review.py
index 7387bd2..b19e3f9 100644
--- a/src/tests/test_tui_review.py
+++ b/src/tests/test_tui_review.py
@@ -260,7 +260,7 @@ class TestReplyVerbatim:
         )
         seen: List[str] = []
 
-        def fake_editor(data: bytes, filehint: str = '') -> bytes:
+        def fake_editor(data: bytes, filehint: str = '', **kwargs: Any) -> bytes:
             seen.append(data.decode())
             return buffer.encode()
 

-- 
2.53.0