[elpa] externals/transient 7b8a7d718a 08/38: Use a more targeted approach to suspending transient override

[Jonas Bernoulli]

branch: externals/transient
commit 7b8a7d718ab8d1811d86357988b664f6090f07d0
Author: Jonas Bernoulli <jonas@bernoul.li>
Commit: Jonas Bernoulli <jonas@bernoul.li>

    Use a more targeted approach to suspending transient override
    
    There are three distinct kinds of minibuffer usage that occur in
    combination with the usage of transient commands.  Until now we used
    the same two hook functions to suspend the transient state for all of
    these but that resulted in unintended behavior.
    
    The three types of minibuffer usage are:
    
    1. An infix command uses the minibuffer to read a new value.
    
       Transient has complete control over the code that does this, but
       we did not take full advantage of that.
    
    2. A suffix command uses the minibuffer.
    
       Since any command can be used as a suffix command Transient has no
       direct control over it.
    
    3. The minibuffer is already active when a transient prefix command is
       invoked.
    
       We were forced to disallow this case because the old implementation
       was unable to reliably differentiate this case from the others.
    
    Previously we tried to handle all three cases by potentially
    suspending the transient state on `minibuffer-setup-hook' and resuming
    it using a function on `minibuffer-exit-hook'.  It turned out that
    these functions could not reliably determine where in the active
    transient's life-cycle the active minibuffer appears, so now we handle
    each case separately.
    
    -- No longer permanently add any functions to the minibuffer hooks.
    
    -- Permanently add a function to `post-command-hook', which doesn't do
       anything by default.
    
    1. Add the macro `transient--with-suspended-override' to deal with the
       first case only.  `transient-infix-read' must use this macro around
       any code that uses the minibuffer but because the around method for
       `transient-infix' object takes care of that, other methods usually
       don't have to do so themselves.
    
       This macro adds anonymous functions to the minibuffer hooks and by
       using `unwind-protect' it can guarantee that our suspend and resume
       functions are called and then removed when done, even in case of an
       error.  It also guarantees that no recursive minibuffer uses are
       affected by making the hook functions no-opts when they are called
       on another `minibuffer-depth'.
    
    2. In the second case we cannot use this macro, or something similar,
       because the code that uses the minibuffer can be arbitrary third-
       party code.
    
       If a command uses the minibuffer, then `post-commmand-hook' is run
       twice for that command; once before entering the minibuffer and
       then again when the command is actually done running.
    
       The first, premature `post-command-hook' run can be differentiated
       from other runs because `this-command-keys-vector' returns an empty
       vector in this case only.
    
       We use this first `post-command-hook' run to suspend the transient.
       (We cannot just perform the usual post-command work at this time,
       before the command has actually finished, because the whole point
       is to set some variables before the command runs and to unset them
       after it is done running.)
    
       The second `post-command-hook' run only happens if the minibuffer
       is exited normally.  If the minibuffer is aborted, then the second,
       normal run does not happen and `minibuffer-exit-hook' has to be
       used instead.
    
       (It would be tempting to deal with this by just always delaying the
       post-command work only until `minibuffer-exit-hook', but that would
       not be appropriate.  The command may of course use `transient-args'
       and `transient-current-*' variables after using the minibuffer.)
    
       So instead we have to delay until the second `post-command-hook' if
       the minibuffer is exited normally, and until `minibuffer-exit-hook'
       if it is aborted.
    
       There is only `minibuffer-exit-hook' for both exited and aborted
       uses of the minibuffer, so our function has to determine which it
       is.  We do so using a heuristic.  We check if the command used to
       get out of the minibuffer was one of the known abort commands and
       because someone might use a non-standard command we otherwise also
       check if the used key binding is "C-g".
    
       We cannot add a function to `post-command-hook' during its first
       run, expecting it to be called during the second run, since such an
       addition does not take affect until the next command.  So we add it
       to `transient--post-command-hook' and make `post-command-hook'
       always run that secondary hook.
    
    3. In the third case, when the minibuffer is already active when a
       transient prefix command is invoked, then that finally just works.
---
 lisp/transient.el | 160 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 112 insertions(+), 48 deletions(-)

diff --git a/lisp/transient.el b/lisp/transient.el
index fb5752609f..4935e2b587 100644
--- a/lisp/transient.el
+++ b/lisp/transient.el
@@ -1319,6 +1319,8 @@ variable instead.")
 
 (defvar transient--stack nil)
 
+(defvar transient--minibuffer-depth nil)
+
 (defvar transient--buffer-name " *transient*"
   "Name of the transient buffer.")
 
@@ -1340,6 +1342,16 @@ Usually it remains current while the transient is 
active.")
 
 (defvar transient--history nil)
 
+(defvar transient--abort-commands
+  '(abort-minibuffers                   ; (minibuffer-quit-recursive-edit)
+    abort-recursive-edit                ; (throw 'exit t)
+    exit-recursive-edit                 ; (throw 'exit nil)
+    keyboard-quit                       ; (signal 'quit nil)
+    minibuffer-keyboard-quit            ; (abort-minibuffers)
+    minibuffer-quit-recursive-edit      ; (throw 'exit (lambda ()
+                                        ;      (signal 'minibuffer-quit nil)))
+    top-level))                         ; (throw 'top-level nil)
+
 (defvar transient--scroll-commands
   '(transient-scroll-up
     transient-scroll-down
@@ -1695,8 +1707,6 @@ be nil and PARAMS may be (but usually is not) used to set 
e.g. the
 This function is also called internally in which case LAYOUT and
 EDIT may be non-nil."
   (transient--debug 'setup)
-  (when (> (minibuffer-depth) 0)
-    (user-error "Cannot invoke transient %s while minibuffer is active" name))
   (transient--with-emergency-exit
     (cond
      ((not name)
@@ -1724,6 +1734,7 @@ EDIT may be non-nil."
     (setq transient--redisplay-map (transient--make-redisplay-map))
     (setq transient--original-window (selected-window))
     (setq transient--original-buffer (current-buffer))
+    (setq transient--minibuffer-depth (minibuffer-depth))
     (transient--redisplay)
     (transient--init-transient)
     (transient--suspend-which-key-mode)))
@@ -1899,11 +1910,8 @@ value.  Otherwise return CHILDREN as is."
   (transient--debug 'init-transient)
   (transient--push-keymap 'transient--transient-map)
   (transient--push-keymap 'transient--redisplay-map)
-  (add-hook 'pre-command-hook      #'transient--pre-command)
-  (add-hook 'minibuffer-setup-hook #'transient--minibuffer-setup)
-  (add-hook 'minibuffer-exit-hook  #'transient--minibuffer-exit)
-  (add-hook 'post-command-hook     #'transient--post-command)
-  (advice-add 'abort-recursive-edit :after #'transient--minibuffer-exit)
+  (add-hook 'pre-command-hook  #'transient--pre-command)
+  (add-hook 'post-command-hook #'transient--post-command)
   (when transient--exitp
     ;; This prefix command was invoked as the suffix of another.
     ;; Prevent `transient--post-command' from removing the hooks
@@ -1978,12 +1986,17 @@ value.  Otherwise return CHILDREN as is."
 
 (defun transient--delete-window ()
   (when (window-live-p transient--window)
-    (let ((buf (window-buffer transient--window)))
+    (let ((remain-in-minibuffer-window
+           (and (minibuffer-selected-window)
+                (selected-window)))
+          (buf (window-buffer transient--window)))
       ;; Only delete the window if it never showed another buffer.
       (unless (eq (car (window-parameter transient--window 'quit-restore)) 
'other)
         (with-demoted-errors "Error while exiting transient: %S"
           (delete-window transient--window)))
-      (kill-buffer buf))))
+      (kill-buffer buf)
+      (when remain-in-minibuffer-window
+        (select-window remain-in-minibuffer-window)))))
 
 (defun transient--export ()
   (setq transient-current-prefix transient--prefix)
@@ -1991,49 +2004,91 @@ value.  Otherwise return CHILDREN as is."
   (setq transient-current-suffixes transient--suffixes)
   (transient--history-push transient--prefix))
 
-(defun transient--minibuffer-setup ()
-  (transient--debug 'minibuffer-setup)
-  (unless (> (minibuffer-depth) 1)
-    (unless transient--exitp
-      (transient--pop-keymap 'transient--transient-map)
-      (transient--pop-keymap 'transient--redisplay-map))
-    (remove-hook 'pre-command-hook  #'transient--pre-command)
-    (remove-hook 'post-command-hook #'transient--post-command)))
-
-(defun transient--minibuffer-exit ()
-  (transient--debug 'minibuffer-exit)
-  (unless (> (minibuffer-depth) 1)
-    (unless transient--exitp
-      (transient--push-keymap 'transient--transient-map)
-      (transient--push-keymap 'transient--redisplay-map))
-    (add-hook 'pre-command-hook  #'transient--pre-command)
-    (add-hook 'post-command-hook #'transient--post-command)))
-
-(defun transient--suspend-override (&optional minibuffer-hooks)
+(defun transient--suspend-override ()
   (transient--debug 'suspend-override)
   (transient--pop-keymap 'transient--transient-map)
   (transient--pop-keymap 'transient--redisplay-map)
   (remove-hook 'pre-command-hook  #'transient--pre-command)
-  (remove-hook 'post-command-hook #'transient--post-command)
-  (when minibuffer-hooks
-    (remove-hook   'minibuffer-setup-hook #'transient--minibuffer-setup)
-    (remove-hook   'minibuffer-exit-hook  #'transient--minibuffer-exit)
-    (advice-remove 'abort-recursive-edit  #'transient--minibuffer-exit)))
+  (remove-hook 'post-command-hook #'transient--post-command))
 
-(defun transient--resume-override (&optional minibuffer-hooks)
+(defun transient--resume-override ()
   (transient--debug 'resume-override)
   (transient--push-keymap 'transient--transient-map)
   (transient--push-keymap 'transient--redisplay-map)
   (add-hook 'pre-command-hook  #'transient--pre-command)
-  (add-hook 'post-command-hook #'transient--post-command)
-  (when minibuffer-hooks
-    (add-hook   'minibuffer-setup-hook #'transient--minibuffer-setup)
-    (add-hook   'minibuffer-exit-hook  #'transient--minibuffer-exit)
-    (advice-add 'abort-recursive-edit :after #'transient--minibuffer-exit)))
+  (add-hook 'post-command-hook #'transient--post-command))
+
+(defmacro transient--with-suspended-override (&rest body)
+  (let ((depth (make-symbol "depth"))
+        (setup (make-symbol "setup"))
+        (exit  (make-symbol "exit")))
+    `(if (and transient--transient-map
+              (memq transient--transient-map
+                    overriding-terminal-local-map))
+         (let ((,depth (1+ (minibuffer-depth))) ,setup ,exit)
+           (setq ,setup
+                 (lambda () "@transient--with-suspended-override"
+                   (transient--debug 'minibuffer-setup)
+                   (remove-hook 'minibuffer-setup-hook ,setup)
+                   (transient--suspend-override)))
+           (setq ,exit
+                 (lambda () "@transient--with-suspended-override"
+                   (transient--debug 'minibuffer-exit)
+                   (when (= (minibuffer-depth) ,depth)
+                     (transient--resume-override))))
+           (unwind-protect
+               (progn
+                 (add-hook 'minibuffer-setup-hook ,setup)
+                 (add-hook 'minibuffer-exit-hook ,exit)
+                 ,@body)
+             (remove-hook 'minibuffer-setup-hook ,setup)
+             (remove-hook 'minibuffer-exit-hook ,exit)))
+       ,@body)))
+
+(defun transient--post-command-hook ()
+  (run-hooks 'transient--post-command-hook))
+
+(add-hook 'post-command-hook 'transient--post-command-hook)
+
+(defun transient--delay-post-command ()
+  (let ((depth (minibuffer-depth))
+        (command this-command)
+        (delayed (if transient--exitp
+                     #'transient--post-exit
+                   #'transient--resume-override))
+        post-command abort-minibuffer)
+    (setq post-command
+          (lambda () "@transient--delay-post-command"
+            (let ((act (and (eq this-command command)
+                            (not (eq (this-command-keys-vector) [])))))
+              (transient--debug 'post-command-hook "act: %s" act)
+              (when act
+                (remove-hook 'transient--post-command-hook post-command)
+                (remove-hook 'minibuffer-exit-hook abort-minibuffer)
+                (funcall delayed)))))
+    (setq abort-minibuffer
+          (lambda () "@transient--delay-post-command"
+            (let ((act (and (or (memq this-command transient--abort-commands)
+                                (equal (this-command-keys) ""))
+                            (= (minibuffer-depth) depth))))
+              (transient--debug
+               'abort-minibuffer
+               "mini: %s|%s, act %s" (minibuffer-depth) depth act)
+              (when act
+                (remove-hook 'transient--post-command-hook post-command)
+                (remove-hook 'minibuffer-exit-hook abort-minibuffer)
+                (funcall delayed)))))
+    (add-hook 'transient--post-command-hook post-command)
+    (add-hook 'minibuffer-exit-hook abort-minibuffer)))
 
 (defun transient--post-command ()
   (transient--debug 'post-command)
   (cond
+   ((and (eq (this-command-keys-vector) [])
+         (= (minibuffer-depth)
+            (1+ transient--minibuffer-depth)))
+    (transient--suspend-override)
+    (transient--delay-post-command))
    (transient--exitp
     (transient--post-exit))
    ((eq this-command (oref transient--prefix command)))
@@ -2051,9 +2106,6 @@ value.  Otherwise return CHILDREN as is."
                    ;; but decided not to call `transient-setup'.
                    (prog1 nil (transient--stack-zap))))
     (remove-hook 'pre-command-hook  #'transient--pre-command)
-    (remove-hook 'minibuffer-setup-hook #'transient--minibuffer-setup)
-    (remove-hook 'minibuffer-exit-hook #'transient--minibuffer-exit)
-    (advice-remove 'abort-recursive-edit #'transient--minibuffer-exit)
     (remove-hook 'post-command-hook #'transient--post-command))
   (setq transient-current-prefix nil)
   (setq transient-current-command nil)
@@ -2063,6 +2115,7 @@ value.  Otherwise return CHILDREN as is."
     (setq transient--exitp nil)
     (setq transient--helpp nil)
     (setq transient--editp nil)
+    (setq transient--minibuffer-depth nil)
     (run-hooks 'transient-exit-hook)
     (when resume
       (transient--stack-pop))))
@@ -2523,13 +2576,24 @@ on the previous value.")
 (cl-defmethod transient-infix-read :around ((obj transient-infix))
   "Highlight the infix in the popup buffer.
 
-Also arrange for the transient to be exited in case of an error
-because otherwise Emacs would get stuck in an inconsistent state,
-which might make it necessary to kill it from the outside."
+This also wraps the call to `cl-call-next-method' with two
+macros.
+
+`transient--with-suspended-override' is necessary to allow
+reading user input using the minibuffer.
+
+`transient--with-emergency-exit' arranges for the transient to
+be exited in case of an error because otherwise Emacs would get
+stuck in an inconsistent state, which might make it necessary to
+kill it from the outside.
+
+If you replace this method, then you must make sure to always use
+the latter macro and most likely also the former."
   (let ((transient--active-infix obj))
     (transient--show))
   (transient--with-emergency-exit
-    (cl-call-next-method obj)))
+    (transient--with-suspended-override
+     (cl-call-next-method obj))))
 
 (cl-defmethod transient-infix-read ((obj transient-infix))
   "Read a value while taking care of history.
@@ -3632,9 +3696,9 @@ search instead."
   (transient--debug 'edebug--recursive-edit)
   (if (not transient--prefix)
       (funcall fn arg-mode)
-    (transient--suspend-override t)
+    (transient--suspend-override)
     (funcall fn arg-mode)
-    (transient--resume-override t)))
+    (transient--resume-override)))
 
 (advice-add 'edebug--recursive-edit :around 
#'transient--edebug--recursive-edit)