Major refactor of ui/popup

+ Make it pass tests
+ Changes the behavior and arguments of functions passed to :autosave,
  :ttl, and :modeline.
+ Updated the documentation of set-popup-rule! to reflect these changes
+ Phase out map.el usage as per f6dc6ac7

modules/ui/popup/autoload/popup.el

@@ -33,6 +33,7 @@ the buffer is visible, then set another timer and try again later."
 default window parameters for popup windows, clears leftover transient timers
 and enables `+popup-buffer-mode'."
   (with-selected-window window
+    (setq alist (delq (assq 'actions alist) alist))
     (when (and alist +popup--populate-wparams)
       ;; Emacs 26+ will automatically map the window-parameters alist entry to
       ;; the popup window, so we need this for Emacs 25.x users
@@ -58,38 +59,41 @@ and enables `+popup-buffer-mode'."
   `transient' window parameter (see `+popup-window-parameters').
 + And finally deletes the window!"
   (let ((buffer (window-buffer window))
-        (inhibit-quit t)
+        (inhibit-quit t))
-        ttl)
+    (and (buffer-file-name buffer)
-    (when (and (buffer-file-name buffer)
          (buffer-modified-p buffer)
-               (or (+popup-parameter-fn 'autosave window buffer)
+         (let ((autosave (+popup-parameter 'autosave window)))
-                   (y-or-n-p "Popup buffer is modified. Save it?")))
+           (cond ((eq autosave 't))
+                 ((null autosave)
+                  (y-or-n-p "Popup buffer is modified. Save it?"))
+                 ((functionp autosave)
+                  (funcall autosave buffer))))
          (with-current-buffer buffer (save-buffer)))
-    (set-buffer-modified-p nil)
     (let ((ignore-window-parameters t))
       (delete-window window))
     (unless (window-live-p window)
       (with-current-buffer buffer
+        (set-buffer-modified-p nil)
         (+popup-buffer-mode -1)
-        ;; t = default
-        ;; integer = ttl
-        ;; nil = no timer
         (unless +popup--inhibit-transient
-          (setq ttl (+popup-parameter-fn 'ttl window buffer))
+          (let ((ttl (+popup-parameter 'ttl window)))
-          (when ttl
+            (when (eq ttl 't)
-            (when (eq ttl t)
+              (setq ttl (plist-get +popup-defaults :ttl)))
-              (setq ttl (or (plist-get +popup-defaults :ttl)
+            (cond ((null ttl))
-                            0)))
+                  ((functionp ttl)
-            (cl-assert (integerp ttl) t)
+                   (funcall ttl buffer))
-            (if (= ttl 0)
+                  ((not (integerp ttl))
-                (+popup--kill-buffer buffer 0)
+                   (signal 'wrong-type-argument (list 'integerp ttl)))
-              (add-hook 'kill-buffer-hook #'+popup|kill-buffer-hook nil t)
+                  ((= ttl 0)
-              (setq +popup--timer
+                   (+popup--kill-buffer buffer 0))
+                  ((add-hook 'kill-buffer-hook #'+popup|kill-buffer-hook nil t))
+                  ((setq +popup--timer
                          (run-at-time ttl nil #'+popup--kill-buffer
-                                 buffer ttl)))))))))
+                                      buffer ttl))))))))))
 
 (defun +popup--normalize-alist (alist)
   "Merge `+popup-default-alist' and `+popup-default-parameters' with ALIST."
+  (when alist
     (let ((alist  ; handle defaults
            (cl-remove-duplicates
             (append alist +popup-default-alist)
@@ -105,11 +109,11 @@ and enables `+popup-buffer-mode'."
                   (param (if (memq side '(left right))
                              'window-width
                            'window-height)))
-      (setq alist (map-delete alist 'size))
+        (setq list (assq-delete-all 'size alist))
-      (map-put alist param size))
+        (setcdr (assq param alist) size))
       (setcdr (assq 'window-parameters alist)
-            (cl-remove-if #'null parameters :key #'cdr))
+              parameters)
-    (cl-remove-if #'null alist :key #'cdr)))
+      alist)))
 
 
 ;;
@@ -120,23 +124,21 @@ and enables `+popup-buffer-mode'."
 (defun +popup-buffer-p (&optional buffer)
   "Return non-nil if BUFFER is a popup buffer. Defaults to the current buffer."
   (when +popup-mode
-    (unless buffer
+    (let ((buffer (or buffer (current-buffer))))
-      (setq buffer (current-buffer)))
+      (and (bufferp buffer)
-    (cl-assert (bufferp buffer) t)
+           (buffer-live-p buffer)
-    (and (buffer-live-p buffer)
            (buffer-local-value '+popup-buffer-mode buffer)
-         buffer)))
+           buffer))))
 
 ;;;###autoload
 (defun +popup-window-p (&optional window)
   "Return non-nil if WINDOW is a popup window. Defaults to the current window."
   (when +popup-mode
-    (unless window
+    (let ((window (or window (selected-window))))
-      (setq window (selected-window)))
+      (and (windowp window)
-    (cl-assert (windowp window) t)
+           (window-live-p window)
-    (and (window-live-p window)
            (window-parameter window 'popup)
-         window)))
+           window))))
 
 ;;;###autoload
 (defun +popup-buffer (buffer &optional alist)
@@ -216,11 +218,14 @@ restoring it if `+popup-buffer-mode' is disabled."
 + If a function, it takes the current buffer as its argument and must return one
   of the above values."
   (when (bound-and-true-p +popup-buffer-mode)
-    (let ((modeline (+popup-parameter-fn 'modeline nil (current-buffer))))
+    (let ((modeline (+popup-parameter 'modeline)))
       (cond ((eq modeline 't))
             ((or (eq modeline 'nil)
                  (null modeline))
+             ;; TODO use `mode-line-format' window parameter instead (emacs 26+)
              (hide-mode-line-mode +1))
+            ((functionp modeline)
+             (funcall modeline))
             ((symbolp modeline)
              (when-let* ((hide-mode-line-format (doom-modeline modeline)))
                (hide-mode-line-mode +1)))))))
@@ -246,9 +251,9 @@ restoring it if `+popup-buffer-mode' is disabled."
 (defun +popup|cleanup-rules ()
   "Cleans up any duplicate popup rules."
   (interactive)
-  (cl-delete-duplicates
+  (setq +popup--display-buffer-alist
-   +popup--display-buffer-alist
+        (cl-delete-duplicates +popup--display-buffer-alist
-   :key #'car :test #'equal :from-end t)
+                              :key #'car :test #'equal :from-end t))
   (when +popup-mode
     (setq display-buffer-alist +popup--display-buffer-alist)))
 
@@ -291,8 +296,7 @@ This will do nothing if the popup's `quit' window parameter is either nil or
   (interactive
    (list (selected-window)
          current-prefix-arg))
-  (unless window
+  (let ((window (or window (selected-window))))
-    (setq window (selected-window)))
     (when (and (+popup-window-p window)
                (or force-p
                    (memq (+popup-parameter-fn 'quit window window)
@@ -300,7 +304,7 @@ This will do nothing if the popup's `quit' window parameter is either nil or
       (when +popup--remember-last
         (+popup--remember (list window)))
       (delete-window window)
-    t))
+      t)))
 
 ;;;###autoload
 (defun +popup/close-all (&optional force-p)
@@ -426,7 +430,7 @@ Accepts the same arguments as `display-buffer-in-side-window'. You must set
                         (lambda (_side) (frame-root-window (selected-frame)))))
                (when-let* ((window (window--make-major-side-window buffer side slot alist)))
                  (set-window-parameter window 'window-vslot vslot)
-                 (map-put window-persistent-parameters 'window-vslot 'writable)
+                 (add-to-list 'window-persistent-parameters '(window-vslot . writable))
                  window)))
             (t
              ;; Scan windows on SIDE.
@@ -570,11 +574,11 @@ and may be called only if no window on SIDE exists yet."
         ;; Initialize `window-side' parameter of new window to SIDE and
         ;; make that parameter persistent.
         (set-window-parameter window 'window-side side)
-        (map-put window-persistent-parameters 'window-side 'writable)
+        (add-to-list 'window-persistent-parameters '(window-side . writable))
         ;; Install `window-slot' parameter of new window and make that
         ;; parameter persistent.
         (set-window-parameter window 'window-slot slot)
-        (map-put window-persistent-parameters 'window-slot 'writable)
+        (add-to-list 'window-persistent-parameters '(window-slot . writable))
         ;; Auto-adjust height/width of new window unless a size has been
         ;; explicitly requested.
         (unless (if left-or-right

modules/ui/popup/autoload/settings.el

@@ -10,11 +10,11 @@
         :quit   t
         :select #'ignore
         :ttl    5)
-  "Default setup for `set-popup-rule!' ")
+  "Default properties for popup rules defined with `set-popup-rule!'.")
 
 ;;;###autoload
 (defun +popup--make (predicate plist)
-  (cond ((not (keywordp (car plist)))
+  (cond ((and plist (not (keywordp (car plist))))
          ;; FIXME deprecated popup rule support
          (message "Warning: the old usage of `set-popup-rule!' is deprecated; update the rule for '%s'"
                   predicate)
@@ -51,14 +51,13 @@
 (defun set-popup-rule! (predicate &rest plist)
   "Define a popup rule.
 
-Buffers displayed by `pop-to-buffer' and `display-buffer' (or their siblings)
+These rules affect buffers displayed with `pop-to-buffer' and `display-buffer'
-will be tested against PREDICATE, which is either a) a regexp string (which is
+(or their siblings). Buffers displayed with `switch-to-buffer' (and its
-matched against the buffer's name) or b) a function that takes no arguments and
+variants) will not be affected by these rules (as they are unaffected by
-returns a boolean.
+`display-buffer-alist', which powers the popup management system).
 
-Buffers displayed with `switch-to-buffer' and its variants will not be affected
+PREDICATE can be either a) a regexp string (matched against the buffer's name)
-by these rules (as they are unaffected by `display-buffer-alist', which powers
+or b) a function that takes no arguments and returns a boolean.
-the popup management system).
 
 PLIST can be made up of any of the following properties:
 
@@ -72,83 +71,93 @@ PLIST can be made up of any of the following properties:
   `+popup-display-buffer-stacked-side-window' or `display-buffer-in-side-window'
   is in :actions or `+popup-default-display-buffer-actions'.
 
-:size/:width/:height FLOAT|INT
+:size/:width/:height FLOAT|INT|FN
-  Determines the size of the popup. If opened at the top or bottom, the width is
+  Determines the size of the popup. If more tha one of these size properties are
-  irrelevant unless it is opened in an adjacent slot. Same deal with the left
+  given :size always takes precedence, and is mapped with window-width or
-  and right side.
+  window-height depending on what :side the popup is opened. Setting a height
+  for a popup that opens on the left or right is harmless, but comes into play
+  if two popups occupy the same :vslot.
 
-  If given a FLOAT (0 < x < 1), the number represents how much of the window
+  If a FLOAT (0 < x < 1), the number represents how much of the window will be
-    will be consumed by the popup (a percentage).
+    consumed by the popup (a percentage).
-  If given an INT, the number determines the size in lines (height) or units of
+  If an INT, the number determines the size in lines (height) or units of
     character width (width).
+  If a function, it takes one argument: the popup window, and can do whatever it
+    wants with it, typically resize it, like `+popup-shrink-to-fit'.
 
 :slot/:vslot INT
-  This only applies to popups with a :side. For popups opened at the top or
+  (This only applies to popups with a :side and only if :actions is blank or
-  bottom, slot designates the horizontal positioning of a popup. If two popups
+  contains the `+popup-display-buffer-stacked-side-window' action) These control
-  are assigned the same slot (and same vslot), the later popup will replace the
+  how multiple popups are laid out. INT can be any integer, positive and
-  earlier one. If the later popup has a lower slot, it will open to the older
+  negative.
-  popup's left. A higher slot opens it to the old popup's right.
 
-  On the other hand, vslot operates the same way, but controls how popups are
+  :slot controls lateral positioning (e.g. the horizontal positioning for
-  stacked.
+    top/bottom popups, or vertical positioning for left/right popups).
+  :vslot controls popup stacking (from the edge of the frame toward the center).
 
-  When a popup is opened on the left and right, slot determines vertical
+  Let's assume popup A and B are opened with :side 'bottom, in that order.
-  position and vslot horizontal.
+    If they possess the same :slot and :vslot, popup B will replace popup A.
+    If popup B has a higher :slot, it will open to the right of popup A.
+    If popup B has a lower :slot, it will open to the left of popup A.
+    If popup B has a higher :vslot, it will open above popup A.
+    If popup B has a lower :vslot, it will open below popup A.
 
 :ttl INT|BOOL|FN
-  Stands for time-to-live. CDR can be t, an integer, nil or a function that
+  Stands for time-to-live. It can be t, an integer, nil or a function. This
-  returns one of these. It represents the number of seconds before the buffer
+  controls how (and if) the popup system will clean up after the popup.
-  belonging to a closed popup window is killed.
 
-  If t, CDR will default to `+popup-ttl'.
+  If any non-zero integer, wait that many seconds before killing the buffer (and
+    any associated processes).
   If 0, the buffer is immediately killed.
-  If nil, the buffer won't be killed.
+  If nil, the buffer won't be killed and is left to its own devices.
-  If a function, it must return one of the other possible values above. It takes
+  If t, resort to the default :ttl in `+popup-defaults'. If none exists, this is
-    the popup buffer as its sole argument.
+    the same as nil.
+  If a function, it takes one argument: the target popup buffer. The popup
+    system does nothing else and ignores the function's return value.
 
-:quit BOOL|FN
+:quit FN|BOOL|'other|'current
-  CDR can be t, 'other, 'current, nil, or a function that returns one of these.
+  Can be t, 'other, 'current, nil, or a function. This determines the behavior
-  This determines the behavior of the ESC/C-g keys in or outside of popup
+  of the ESC/C-g keys in or outside of popup windows.
-  windows.
 
-  If t, close the popup if ESC/C-g is pressed inside or outside of popups.
+  If t, close the popup if ESC/C-g is pressed anywhere.
   If 'other, close this popup if ESC/C-g is pressed outside of any popup. This
-    is great for popups you just want to peek at and discard, but might also
+    is great for popups you may press ESC/C-g a lot in.
-    want to poke around in, without the risk of closing it from the inside.
   If 'current, close the current popup if ESC/C-g is pressed from inside of the
-    popup.
+    popup. This makes it harder to accidentally close a popup until you really
-  If nil, pressing ESC/C-g will never close this buffer.
+    want to.
-  If a function, it is checked each time ESC/C-g is pressed to determine the
+  If nil, pressing ESC/C-g will never close this popup.
-    fate of the popup window. This function takes one argument: the popup window
+  If a function, it takes one argument: the to-be-closed popup window, and is
-    and must return one of the other possible values.
+    run when ESC/C-g is pressed while that popup is open. It must return one of
+    the other values to determine the fate of the popup.
 
 :select BOOL|FN
-  CDR can be a boolean or function. The boolean determines whether to focus the
+  Can be a boolean or function. The boolean determines whether to focus the
   popup window after it opens (non-nil) or focus the origin window (nil).
 
-  If a function, it takes two arguments: the popup window and the source window
+  If a function, it takes two arguments: the popup window and originating window
-  (where you were before the popup was opened). It does nothing else, and
+    (where you were before the popup opened). The popup system does nothing else
-  ignores its return value.
+    and ignores the function's return value.
 
 :modeline BOOL|SYMBOL|FN
-  CDR can be t (show the default modeline), a symbol representing the name of a
+  Can be t (show the default modeline), a symbol representing the name of a
   modeline defined with `def-modeline!', nil (show no modeline) or a function
-  that returns one of these. The function takes one argument: the popup buffer.
+  that returns a modeline format. The function takes no arguments and is run in
+  the context of the popup buffer.
 
 :autosave BOOL|FN
-  This parameter determines what to do with modified buffers in closing popup
+  This parameter determines what to do with modified buffers when closing popup
-  windows. CDR can be a t, 'ignore, a function or nil.
+  windows. It accepts t, 'ignore, a function or nil.
 
   If t, no prompts. Just save them automatically (if they're file-visiting
-    buffers).
+    buffers). Same as 'ignore for non-file-visiting buffers.
-  If 'ignore, no prompts, no saving. Just silently kill it.
   If nil (the default), prompt the user what to do if the buffer is
     file-visiting and modified.
-  If a function, the return value must return one of the other values. It takes
+  If 'ignore, no prompts, no saving. Just silently kill it.
-    two arguments: the popup window and buffer.
+  If a function, it is run with one argument: the popup buffer, and must return
+    non-nil to save or nil to do nothing (but no prompts).
 
 :parameters ALIST
-  An alist of custom window parameters. See \(info window-parameters)
+  An alist of custom window parameters. See `(elisp)Window Parameters'.
 
 If any of these are omitted, defaults derived from `+popup-defaults' will be
 used."
@@ -160,9 +169,11 @@ used."
 
 ;;;###autodef
 (defun set-popup-rules! (&rest rulesets)
-  "Like `set-popup-rules!', but defines multiple popup rules. Every entry in RULESETS
+  "Defines multiple popup rules.
-should be a list of lists (each sublist is a popup rule that could be passed to
+
-`set-popup-rule!').
+Every entry in RULESETS should be a list of alists where the CAR is the
+predicate and CDR is a plist. See `set-popup-rule!' for details on the predicate
+and plist.
 
 Example: