transient--suffixes: Safely populate by side-effect

`transient--suffixes' is populated by side-effect.
`transient--init-suffix' pushes elements onto that list.

`transient--init-suffix' is called indirectly via
`transient--init-suffixes'.  `transient--init-objects', one of the
two callers of the latter dealt with this properly, but the other,
`transient-suffixes' did not.  The latter is called indirectly by
`transient-args', which happens when no prefix is active and
`transient--suffixes' should therefore remain nil.

In most cases it did not matter that it ended up non-nil anyway,
but it broke `transient-suffix-object', which ended up preferring
the invalid non-nil value of `transient--suffixes' in situations
where it should have fallen back to `transient-current-suffixes'.
This caused https://github.com/magit/magit/issues/5528.

Fix containing the side-effect within `transient--init-suffixes'.
The value is still created incrementally by side-effect, but that
value is captured in `transient--init-suffixes' and added to its
return value.  Callers can then either use the returned value to
set the global value of `transient--suffixes', or discard it.

1 files changed, 11 insertions, 11 deletions

diff --git a/lisp/transient.el b/lisp/transient.el
index a6c9a85..d0803c8 100644
--- a/lisp/transient.el
+++ b/lisp/transient.el
@@ -2621,10 +2621,8 @@ value.  Otherwise return CHILDREN as is.")
   (cond ((and (not transient--refreshp) layout)
          (setq transient--layout layout)
          (setq transient--suffixes (transient--flatten-suffixes layout)))
-        (t
-         (setq transient--suffixes nil)
-         (setq transient--layout (transient--init-suffixes name))
-         (setq transient--suffixes (nreverse transient--suffixes))))
+        ((pcase-setq `(,transient--layout ,transient--suffixes)
+                     (transient--init-suffixes name))))
   (slot-makeunbound transient--prefix 'value))
 
 (defun transient--init-prefix (name &optional params)
@@ -2640,11 +2638,14 @@ value.  Otherwise return CHILDREN as is.")
     obj))
 
 (defun transient--init-suffixes (name)
-  (let ((levels (alist-get name transient-levels)))
-    (mapcan (lambda (c) (transient--init-child levels c nil))
-            (append (transient--get-children name)
-                    (and (not transient--editp)
-                         (transient--get-children 'transient-common-commands))))))
+  (let ((levels (alist-get name transient-levels))
+        (transient--suffixes nil))
+    (list (mapcan (lambda (c) (transient--init-child levels c nil))
+                  (append (transient--get-children name)
+                          (and (not transient--editp)
+                               (transient--get-children
+                                'transient-common-commands))))
+          (nreverse transient--suffixes))))
 
 (defun transient--flatten-suffixes (layout)
   (named-let flatten ((def layout))
@@ -4318,8 +4319,7 @@ they can be returned.  That does not cause the menu to be displayed."
   (if (eq transient-current-command prefix)
       transient-current-suffixes
     (let ((transient--prefix (transient--init-prefix prefix)))
-      (transient--flatten-suffixes
-       (transient--init-suffixes prefix)))))
+      (cdr (transient--init-suffixes prefix)))))
 
 (defun transient-get-value ()
   "Return the value of the extant prefix.