Dissociate frame creation and deletion handlers from the actual configuration of frames as workspaces

* exwm-workspace.el (exwm-workspace--add-frame-as-workspace)
(exwm-workspace--remove-frame-as-workspace): Limit functionality
to the configuration of frames as workspaces.
(exwm-workspace--on-after-make-frame)
(exwm-workspace--on-delete-frame): Callbacks run on frame creation
and deletion that may use or stop them from being used as
workspaces.

exwm-workspace.el

@@ -1257,27 +1257,6 @@ Please check `exwm-workspace--minibuffer-own-frame-p' first."
 
 (defun exwm-workspace--add-frame-as-workspace (frame)
   "Configure frame FRAME to be treated as a workspace."
-  (cond
-   ((exwm-workspace--workspace-p frame)
-    (exwm--log "Frame `%s' is already a workspace" frame))
-   ((not (display-graphic-p frame))
-    (exwm--log "Frame `%s' is not graphical" frame))
-   ((not (string-equal
-          (replace-regexp-in-string "\\.0$" ""
-                                    (slot-value exwm--connection 'display))
-          (replace-regexp-in-string "\\.0$" ""
-                                    (frame-parameter frame 'display))))
-    (exwm--log "Frame `%s' is on a different DISPLAY (%S instead of %S)"
-               frame
-               (frame-parameter frame 'display)
-               (slot-value exwm--connection 'display)))
-   ((frame-parameter frame 'unsplittable)
-    ;; We create floating frames with the "unsplittable" parameter set.
-    ;; Though it may not be a floating frame, we won't treat an
-    ;; unsplittable frame as a workspace anyway.
-    (exwm--log "Frame `%s' is floating" frame))
-   (t
-    (exwm--log "Adding frame `%s' as workspace" frame)
   (setq exwm-workspace--list (nconc exwm-workspace--list (list frame)))
   (let ((outer-id (string-to-number (frame-parameter frame
                                                      'outer-window-id)))
@@ -1337,20 +1316,13 @@ Please check `exwm-workspace--minibuffer-own-frame-p' first."
   (if exwm-workspace--create-silently
       (setq exwm-workspace--switch-history-outdated t)
     (exwm-workspace-switch frame t)
-      (run-hooks 'exwm-workspace-list-change-hook)))))
+    (run-hooks 'exwm-workspace-list-change-hook)))
 
 (defun exwm-workspace--remove-frame-as-workspace (frame)
   "Stop treating frame FRAME as a workspace."
-  (cond
+  ;; TODO: restore all frame parameters (e.g. exwm-workspace, buffer-predicate,
-   ((not (exwm-workspace--workspace-p frame))
+  ;; etc)
-    (exwm--log "Frame `%s' is not a workspace" frame))
-   (t
   (exwm--log "Removing frame `%s' as workspace" frame)
-    (when (= 1 (exwm-workspace--count))
-      ;; The user managed to delete the last workspace, so create a new one.
-      (exwm--log "Last workspace deleted; create a new one")
-      (let ((exwm-workspace--create-silently t))
-        (make-frame)))
   (let* ((index (exwm-workspace--position frame))
          (lastp (= index (1- (exwm-workspace--count))))
          (nextw (elt exwm-workspace--list (+ index (if lastp -1 +1)))))
@@ -1373,21 +1345,75 @@ Please check `exwm-workspace--minibuffer-own-frame-p' first."
     (when (eq frame exwm-workspace--current)
       (exwm-workspace-switch nextw)))
   ;; Reparent out the frame.
+  (let ((outer-id (frame-parameter frame 'exwm-outer-id)))
+    (xcb:+request exwm--connection
+        (make-instance 'xcb:UnmapWindow
+                       :window outer-id))
     (xcb:+request exwm--connection
         (make-instance 'xcb:ReparentWindow
-                       :window (frame-parameter frame 'exwm-outer-id)
+                       :window outer-id
                        :parent exwm--root
                        :x 0
                        :y 0))
+    ;; Reset the override-redirect.
+    (xcb:+request exwm--connection
+        (make-instance 'xcb:ChangeWindowAttributes
+                       :window outer-id
+                       :value-mask xcb:CW:OverrideRedirect
+                       :override-redirect 0))
+    (xcb:+request exwm--connection
+        (make-instance 'xcb:MapWindow
+                       :window outer-id)))
   ;; Destroy the container.
   (xcb:+request exwm--connection
       (make-instance 'xcb:DestroyWindow
                      :window (frame-parameter frame 'exwm-container)))
+  (xcb:flush exwm--connection)
   ;; Update EWMH properties.
   (exwm-workspace--update-ewmh-props)
   ;; Update switch history.
   (setq exwm-workspace--switch-history-outdated t)
-    (run-hooks 'exwm-workspace-list-change-hook))))
+  (run-hooks 'exwm-workspace-list-change-hook))
+
+(defun exwm-workspace--on-delete-frame (frame)
+  "Hook run upon `delete-frame' that tears down FRAME's configuration as a workspace."
+  (cond
+   ((not (exwm-workspace--workspace-p frame))
+    (exwm--log "Frame `%s' is not a workspace" frame))
+   (t
+    (when (= 1 (exwm-workspace--count))
+      ;; The user managed to delete the last workspace, so create a new one.
+      (exwm--log "Last workspace deleted; create a new one")
+      ;; TODO: this makes sense in the hook.  But we need a function that takes
+      ;; care of converting a workspace into a regular unmanaged frame.
+      (let ((exwm-workspace--create-silently t))
+        (make-frame)))
+    (exwm-workspace--remove-frame-as-workspace frame))))
+
+(defun exwm-workspace--on-after-make-frame (frame)
+  "Hook run upon `delete-frame' that configures FRAME as a workspace."
+  (cond
+   ((exwm-workspace--workspace-p frame)
+    (exwm--log "Frame `%s' is already a workspace" frame))
+   ((not (display-graphic-p frame))
+    (exwm--log "Frame `%s' is not graphical" frame))
+   ((not (string-equal
+          (replace-regexp-in-string "\\.0$" ""
+                                    (slot-value exwm--connection 'display))
+          (replace-regexp-in-string "\\.0$" ""
+                                    (frame-parameter frame 'display))))
+    (exwm--log "Frame `%s' is on a different DISPLAY (%S instead of %S)"
+               frame
+               (frame-parameter frame 'display)
+               (slot-value exwm--connection 'display)))
+   ((frame-parameter frame 'unsplittable)
+    ;; We create floating frames with the "unsplittable" parameter set.
+    ;; Though it may not be a floating frame, we won't treat an
+    ;; unsplittable frame as a workspace anyway.
+    (exwm--log "Frame `%s' is floating" frame))
+   (t
+    (exwm--log "Adding frame `%s' as workspace" frame)
+    (exwm-workspace--add-frame-as-workspace frame))))
 
 (defun exwm-workspace--update-ewmh-props ()
   "Update EWMH properties to match the workspace list."
@@ -1546,7 +1572,7 @@ applied to all subsequently created X frames."
   (advice-add 'handle-focus-out :around #'exwm-workspace--handle-focus-out)
   ;; Make new frames create new workspaces.
   (add-hook 'after-make-frame-functions
-            #'exwm-workspace--add-frame-as-workspace)
+            #'exwm-workspace--on-after-make-frame)
   (add-hook 'delete-frame-functions
             #'exwm-workspace--remove-frame-as-workspace)
   ;; Switch to the first workspace
@@ -1579,9 +1605,9 @@ applied to all subsequently created X frames."
   (advice-remove 'handle-focus-in #'exwm-workspace--handle-focus-in)
   (advice-remove 'handle-focus-out #'exwm-workspace--handle-focus-out)
   (remove-hook 'after-make-frame-functions
-               #'exwm-workspace--add-frame-as-workspace)
+               #'exwm-workspace--on-after-make-frame)
   (remove-hook 'delete-frame-functions
-               #'exwm-workspace--remove-frame-as-workspace))
+               #'exwm-workspace--on-delete-frame))
 
 (defun exwm-workspace--post-init ()
   "The second stage in the initialization of the workspace module."