emacs-patch-review
Port of Thunderbird Patch Review to mu4e.
git clone git://mccd.space/emacs-patch-reviewcommit 47f6d91a65c7c73ce34f373b5b80e0e84e30377e
Author: Pi Agent <agent@pi.local>
Date: Sun, 2 Aug 2026 12:47:43 +0200
Bootstrap repository: docs, test runner, fixtures
Scaffold for the Emacs port of the Thunderbird patch-review add-on.
Fixtures are copied from thunderbird-review-ui/tests/fixtures so both
projects parse the same data; patch1.eml adds RFC 822 headers around
patch1.body for end-to-end tests.
Diffstat:
11 files changed, 343 insertions(+), 0 deletions(-)
diff --git a/.gitignore b/.gitignore
@@ -0,0 +1 @@
+*.elc
diff --git a/AGENTS.md b/AGENTS.md
@@ -0,0 +1,44 @@
+# Agent Notes
+
+## Commits
+
+Commits must be **atomic**: each commit is a single, self-contained change
+whose tests pass on its own. Do not bundle unrelated edits into one
+commit, and do not leave a change half-finished across commits. If a task
+touches more than one concern, split it into a series of small commits,
+each with a focused message describing exactly that concern.
+
+Scope of "atomic" here:
+
+- One logical change per commit (a fix, a feature, a refactor — not mixed).
+- `make test` passes after every commit, not only at the tip of the
+ series.
+- The commit message subject line summarizes the single concern; the body,
+ when needed, explains why.
+
+## Repository layout
+
+- `patch-review.el` — the major mode (derived from `diff-mode`): review
+ buffer, header-line controls, comment extraction (anchored diff against
+ the pristine text), reply composition.
+- `patch-review-parse.el` — pure parsing: patch-email → structured
+ diff; subject-tag heuristics. No side effects.
+- `patch-review-reply.el` — pure formatting: comments → interleaved
+ quoted reply body.
+- `patch-review-git.el` — git shell-out: applicability probe (throwaway
+ `--detach` worktree + `git am`), apply, dirty/am-in-progress guards.
+- `patch-review-mu4e.el` — thin mu4e adapter (untested in CI; no `mu` on
+ the dev machine).
+- `tests/` — ERT suite (`patch-review-test.el`) and fixtures shared with
+ the Thunderbird add-on (`*.body` files are copied from
+ thunderbird-review-ui/tests/fixtures; keep them in sync).
+
+## Running the tests
+
+```
+make test
+```
+
+which runs `emacs -Q --batch -L . -l tests/run.el`. The git integration
+tests create throwaway repositories under `temporary-file-directory` and
+need `git` and `diff` on PATH.
diff --git a/Makefile b/Makefile
@@ -0,0 +1,5 @@
+EMACS ?= emacs
+
+.PHONY: test
+test:
+ $(EMACS) -Q --batch -L . -l tests/run.el
diff --git a/README.org b/README.org
@@ -0,0 +1,130 @@
+#+TITLE: patch-review.el — review git patches from email in Emacs
+#+AUTHOR: Marc Coquand
+
+An Emacs port of the [[https://git.sr.ht/~marcc/thunderbird-review-ui][Thunderbird Patch Review]]
+add-on. Open a patch email, type review comments directly into the diff,
+send the review as an interleaved mailing-list reply, and apply the patch
+to a local repository with =git am=.
+
+Unlike the Thunderbird version, no privileged "host" component is needed:
+Emacs runs git directly, =diff-mode= provides the rendering, and
+=message-mode= the composition.
+
+* Installation
+
+Clone and add to =load-path=:
+
+#+begin_src elisp
+(add-to-list 'load-path "/path/to/emacs-patch-review")
+(require 'patch-review) ; core, diff-mode derived
+(require 'patch-review-mu4e) ; optional mu4e integration
+#+end_src
+
+Requires Emacs 28.1+, git, and diffutils. Requires mu4e only for the
+mail adapter.
+
+* Usage
+
+** From mu4e
+
+In a =*mu4e-view*= buffer showing a patch:
+
+#+begin_src elisp
+(define-key mu4e-view-mode-map (kbd "R") #'patch-review-mu4e-review)
+#+end_src
+
+** From a file
+
+Opening a =.patch= or =.diff= file activates =patch-review-mode= (via
+=auto-mode-alist=). =M-x patch-review-open-message-file= handles raw
+=.eml= messages.
+
+** Reviewing
+
+The review buffer is an ordinary, editable =diff-mode= buffer holding the
+patch body. There is no "add comment" command: *press RET anywhere and
+type*. Text you insert is your commentary; the patch text stays
+untouched. Text inserted above the first hunk becomes a general remark.
+
+When you send (=C-c C-c=), the buffer is diffed against the pristine
+original. Your insertions are extracted, anchored to the patch line above
+them, and formatted as an interleaved reply — quoted hunks with =>=
+prefix, comments underneath, kernel style:
+
+#+begin_example
+Looks good overall, two nits.
+
+> diff --git a/src/socket.c b/src/socket.c
+> @@ -1,15 +1,20 @@
+> #include <stdio.h>
+> +#include <errno.h>
+
+Is errno.h needed on all platforms?
+#+end_example
+
+The outgoing mail is composed from the *pristine* text, so accidental
+edits to the patch itself can never corrupt the quote (they are reported
+before sending). The reply lands in a =message-mode= buffer; send it with
+=C-c C-c= again.
+
+** Applying
+
+The header line shows the target project and whether the patch applies to
+it (probed by running =git am= in a throwaway detached worktree — your
+repository is never touched by the probe).
+
+** Controls
+
+| Key | Action |
+|-----------+-----------------------------------------------|
+| =C-c C-c= | extract comments, compose review reply |
+| =C-c C-a= | apply patch to the project with =git am= |
+| =C-u C-c C-a= | apply with user git hooks suppressed |
+| =C-c C-p= | set/switch the target project |
+| =C-c C-r= | re-run the applicability probe |
+
+Plus all of =diff-mode='s navigation (=M-n= / =M-p= between hunks,
+=C-c C-b= word-level refinement, ...).
+
+Status values: =applies cleanly=, =already applied=, =worktree dirty=,
+=conflicts=, =am in progress=, =no project=.
+
+* Design notes
+
+- Comment extraction uses an anchored =diff -U0= between the pristine and
+ edited buffer; pure insertions become comments at the original line
+ above them. Deletions/modifications of patch text are counted and
+ confirmed before sending.
+- The applicability probe mirrors the Thunderbird add-on's patchHost:
+ =git diff-index --quiet HEAD= for dirtiness, then =git am= in a
+ temporary =--detach= worktree with =core.hooksPath= pointed at an empty
+ directory (probes must not run user hooks against unreviewed patch
+ content). =git am= itself gives the tri-state: "Patch already applied",
+ exit 0, or exit 128.
+- Applying runs the user's hooks (a deliberate act); =C-u= suppresses
+ them. Applying refuses to run while a previous =git am= session is
+ unfinished (=rebase-apply/= present) instead of aborting it.
+
+* Roadmap
+
+- Series collection: gather =[PATCH n/m]= siblings of the same thread
+ (mu find), review and apply the whole series at once (concatenated
+ mbox, matching =git am= semantics).
+- Sourcehut integration: =X-Sourcehut-Patchset-Update= header on send.
+- Asynchronous applicability probe.
+- Optional apply via [[https://docs.kyleam.com/piem/][piem]].
+
+* Development
+
+Run the tests:
+
+#+begin_src sh
+make test
+#+end_src
+
+Commits are atomic: one logical change per commit, and =make test=
+passes at every commit.
+
+* License
+
+GPLv3. See the Thunderbird project for the original implementation.
diff --git a/tests/fixtures/cover.body b/tests/fixtures/cover.body
@@ -0,0 +1,17 @@
+*** BLURB HERE ***
+
+Dev Example (3):
+ socket: validate file descriptors
+ log: add error logging helper
+ assets: add icon
+
+ icon.bin | Bin 0 -> 12 bytes
+ src/log.c | 6 ++++++
+ src/socket.c | 7 ++++++-
+ 3 files changed, 12 insertions(+), 1 deletion(-)
+ create mode 100644 icon.bin
+ create mode 100644 src/log.c
+
+--
+2.55.0
+
diff --git a/tests/fixtures/patch1.body b/tests/fixtures/patch1.body
@@ -0,0 +1,35 @@
+Reject negative descriptors in both open and close paths and
+report EBADF like the rest of the subsystem does.
+---
+ src/socket.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/src/socket.c b/src/socket.c
+index ceba5c8..6eab6c5 100644
+--- a/src/socket.c
++++ b/src/socket.c
+@@ -1,15 +1,20 @@
+ #include <stdio.h>
++#include <errno.h>
+
+ int open_socket(int fd)
+ {
+- if (fd < 0)
++ if (fd < 0) {
++ errno = EBADF;
+ return -1;
++ }
+ printf("opening %d\n", fd);
+ return fd;
+ }
+
+ int close_socket(int fd)
+ {
++ if (fd < 0)
++ return -1;
+ printf("closing %d\n", fd);
+ return 0;
+ }
+--
+2.55.0
+
diff --git a/tests/fixtures/patch1.eml b/tests/fixtures/patch1.eml
@@ -0,0 +1,42 @@
+From: Aisha Developer <aisha@example.org>
+To: project-dev@lists.example.org
+Cc: Reviewers <review@lists.example.org>
+Subject: [PATCH 1/3] Reject negative descriptors in socket paths
+Date: Sat, 18 Jul 2026 09:41:12 +0200
+Message-ID: <patch1@example.org>
+
+Reject negative descriptors in both open and close paths and
+report EBADF like the rest of the subsystem does.
+---
+ src/socket.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/src/socket.c b/src/socket.c
+index ceba5c8..6eab6c5 100644
+--- a/src/socket.c
++++ b/src/socket.c
+@@ -1,15 +1,20 @@
+ #include <stdio.h>
++#include <errno.h>
+
+ int open_socket(int fd)
+ {
+- if (fd < 0)
++ if (fd < 0) {
++ errno = EBADF;
+ return -1;
++ }
+ printf("opening %d\n", fd);
+ return fd;
+ }
+
+ int close_socket(int fd)
+ {
++ if (fd < 0)
++ return -1;
+ printf("closing %d\n", fd);
+ return 0;
+ }
+--
+2.55.0
+
diff --git a/tests/fixtures/patch2.body b/tests/fixtures/patch2.body
@@ -0,0 +1,20 @@
+---
+ src/log.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
+ create mode 100644 src/log.c
+
+diff --git a/src/log.c b/src/log.c
+new file mode 100644
+index 0000000..52ca3a1
+--- /dev/null
++++ b/src/log.c
+@@ -0,0 +1,6 @@
++#include <stdio.h>
++
++void log_err(const char *msg)
++{
++ fprintf(stderr, "error: %s\n", msg);
++}
+--
+2.55.0
+
diff --git a/tests/fixtures/patch3.body b/tests/fixtures/patch3.body
@@ -0,0 +1,18 @@
+---
+ icon.bin | Bin 0 -> 12 bytes
+ 1 file changed, 0 insertions(+), 0 deletions(-)
+ create mode 100644 icon.bin
+
+diff --git a/icon.bin b/icon.bin
+new file mode 100644
+index 0000000000000000000000000000000000000000..6735744d9c5bfa205ec44c128ac9007f124c6686
+GIT binary patch
+literal 12
+TcmeAS@N?(olHy`uWMT#Y5cvVH
+
+literal 0
+HcmV?d00001
+
+--
+2.55.0
+
diff --git a/tests/patch-review-test.el b/tests/patch-review-test.el
@@ -0,0 +1,20 @@
+;;; patch-review-test.el --- ERT suite -*- lexical-binding: t -*-
+
+(require 'ert)
+
+(defun patch-review-test-fixture (name)
+ "Return the contents of fixture NAME."
+ (with-temp-buffer
+ (insert-file-contents
+ (expand-file-name
+ (concat "fixtures/" name)
+ (file-name-directory (or load-file-name buffer-file-name))))
+ (buffer-string)))
+
+(ert-deftest patch-review-test-fixtures-load ()
+ (dolist (f '("patch1.body" "patch2.body" "patch3.body" "cover.body"
+ "patch1.eml"))
+ (should (stringp (patch-review-test-fixture f)))))
+
+(provide 'patch-review-test)
+;;; patch-review-test.el ends here
diff --git a/tests/run.el b/tests/run.el
@@ -0,0 +1,11 @@
+;;; run.el --- batch test runner -*- lexical-binding: t -*-
+
+;; Invoke from the repository root: emacs -Q --batch -L . -l tests/run.el
+
+(add-to-list 'load-path (expand-file-name "."))
+(add-to-list 'load-path (expand-file-name "tests"))
+
+(require 'patch-review-test)
+(ert-run-tests-batch-and-exit)
+
+;;; run.el ends here