summaryrefslogtreecommitdiff
path: root/todo.org
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-16 01:31:36 -0500
committerCraig Jennings <c@cjennings.net>2026-05-16 01:31:36 -0500
commit1ca46c2b477afd014ef993ed0ca5ca50e257adce (patch)
tree66ddfecc234c18f8d90e6e7908204de0afd3e3d2 /todo.org
parent531f0f19f298e28b42dfb216f6008a1cbc6164d3 (diff)
downloaddotemacs-1ca46c2b477afd014ef993ed0ca5ca50e257adce.tar.gz
dotemacs-1ca46c2b477afd014ef993ed0ca5ca50e257adce.zip
fix(ai-config): hook gptel-magit wiring per-feature, not on magit
The wiring keyed on `with-eval-after-load 'magit` fires while two of its three references are still undefined. `magit.el` calls `(provide 'magit)` BEFORE its `cl-eval-when (load eval)` block requires `magit-commit` and `magit-stash`. At that moment the `magit-commit` transient prefix doesn't exist, and `transient-append-suffix` silently no-ops on missing prefixes (default `transient-error-on-insert-failure` is nil). The "g Generate commit" and "x Explain" suffixes never landed. Only the M-g binding worked, because `git-commit` IS required before provide. Three per-feature hooks replace the single `'magit` hook: one each on `git-commit`, `magit-commit`, and `magit-diff`. Each hooks the exact dependency the wiring needs, side-stepping the load-order race entirely. The companion test was rewritten to check `after-load-alist` registration rather than drive the hooks through `provide`. Emacs 30 batch mode doesn't fire registered `eval-after-load` callbacks on `provide` alone -- only an actual `load` does. Inspecting the registration is the stronger guard anyway: the regression is "a single `'magit` hook," and the right shape of that check is "no entry under `magit`, entries under `git-commit`, `magit-commit`, `magit-diff`."
Diffstat (limited to 'todo.org')
-rw-r--r--todo.org33
1 files changed, 23 insertions, 10 deletions
diff --git a/todo.org b/todo.org
index e72da3fc..d9d3635d 100644
--- a/todo.org
+++ b/todo.org
@@ -2601,16 +2601,29 @@ No backup is created when the operation is a no-op.
operation plus the wrapper -- 48 tests green. Added =update_text_file=
to =cj/gptel-local-tool-features= so gptel exposes it on next restart.
-*** TODO [#B] Fix gptel-magit triggers :bug:
-
-Wired up in =modules/ai-config.el= as three lazy entry points:
-- =M-g= in =git-commit-mode-map= → =gptel-magit-generate-message=
-- =g= in the =magit-commit= transient → =gptel-magit-commit-generate=
-- =x= in the =magit-diff= transient → =gptel-magit-diff-explain=
-
-Specific failure mode TBD. First step: reproduce, note which entry point fails and how, then trace from there. Likely suspects: autoload chain (the three =autoload= calls run inside =with-eval-after-load 'magit=), transient suffix attachment via =transient-append-suffix=, or gptel-side backend/model config.
-
-Migration to the current lazy form: commit =3eb1a0c refactor(gptel): lazy-load gptel-magit, rebind rewrite/context keys=.
+*** 2026-05-16 Sat @ 01:31:03 -0500 Split the magit wiring into per-feature with-eval-after-load blocks
+
+Root cause: =magit.el= calls =(provide 'magit)= BEFORE its
+=(cl-eval-when (load eval) ...)= block requires =magit-commit= and
+=magit-stash=. A single =with-eval-after-load 'magit= fires while
+those transient prefixes are still undefined, and
+=transient-append-suffix= silently no-ops on missing prefixes
+(documented behavior unless =transient-error-on-insert-failure= is
+set). Two of three triggers failed silently because of this; only
+M-g worked, because =git-commit= IS required before the provide.
+
+Fix: replace the single =with-eval-after-load 'magit= with three
+per-feature blocks (=git-commit=, =magit-commit=, =magit-diff=). Each
+hooks the exact dependency the wiring needs.
+
+The existing lazy-loading test was rewritten to check
+=after-load-alist= registration directly rather than driving the
+hooks via =provide= -- in Emacs 30 batch mode, =provide= does not
+fire registered =eval-after-load= callbacks; only an actual =load=
+does. Inspecting the registration is stronger evidence anyway: the
+guard against the regression is "no entry for =magit=, entries for
+=git-commit=, =magit-commit=, =magit-diff=," which is exactly what
+the test asserts.
*** TODO [#B] Add ERT coverage for ai-conversations.el :tests: