bug#51883: 29.0.50; Command to get accidentally deleted frames back

[Gregory Heytings <gregory@heytings.org>]

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]


Thanks again for your comments.

I attach an updated and improved patch.

>>> This is unusual meaning of prefix argument.  Why not use zero for the 
>>> last, 1 for the one before that, etc.?
>
> Usually, commands that use both numeric and raw prefix arg do something 
> very different with the raw argument, which is not the case here.
>

The last version of the patch now does that, too: no argument and a raw 
argument means "most recent", a numeric argument means "Nth".

>> A minor mode is another option, indeed.  My feeling is that this 
>> feature is something about everyone would find useful, and that the 
>> cost you mention is not that high.  And I solved the problem of the 
>> deletion of the terminal frame.  So I made it an opt-out minor-mode.
>
> I'd prefer to make it opt-in.  I see no reason to force on everyone a 
> new feature that doesn't sound like it's urgent or important enough to 
> justify the behavior change.  Even though the memory it uses is not 
> large, it's still memory, and it still increases consing each time a 
> frame is deleted.  For example, some people turn on all kinds of 
> optional features that pop up new frames in many situations, and who 
> knows what this will cause in those usage patterns.  Why risk such 
> unintended consequences on behalf of a minor feature?
>

I hear your arguments, but IMO that would be like making "undo" opt-in. 
I also wouldn't call that a "minor" feature, it's worth an entry in the 
File menu.  Indeed we don't know what this could cause with exotic usage 
patterns, so I suggest, given that the release of Emacs 29 is far away in 
the future, to make it opt-out on the trunk, and if someone protests 
because it breaks their usage pattern before Emacs 29 is released, to make 
it opt-in instead.

>
> How about using "restore" instead of "undelete", here and everywhere 
> else?  I think it's a tad more clear, and also easier to understand, as 
> it doesn't use negative tense.
>

I agree with Juri here.  In another app, I would have named this "Restore 
Window", but Emacs uses "Delete Frame" where other apps would use "Close 
Window", so using "Restore Frame" would be much less clear than "Undelete 
Frame".  In particular, it would not be clear that "Restore" creates a new 
frame and does not do something with the current frame.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Undelete-deleted-frames.patch, Size: 8744 bytes --]

From 35ae03480511c16fb9657e4c661278322d7ccdec Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 19 Nov 2021 08:50:49 +0000
Subject: [PATCH] Undelete deleted frames.

* lisp/frame.el (undelete-frame): New command.
(undelete-frame--handle-delete-frame): New auxiliary function.
(undelete-frame--deleted-frames, undelete-frame--enabled): New
auxiliary variables.
(undelete-frame-mode): New minor mode.
(ctl-x-5-map): Bind the new command.

* src/frame.c (Fdelete_frame): Update docstring, and mention the
minor mode.

* lisp/menu-bar.el (menu-bar-file-menu): Add an entry for the
new command.

* doc/emacs/frames.tex (Frame Commands): Document the new command
and minor mode.

* etc/NEWS: Document the new command and minor mode.

See bug#51883.
---
 doc/emacs/frames.texi | 10 +++++
 etc/NEWS              | 10 +++++
 lisp/frame.el         | 88 +++++++++++++++++++++++++++++++++++++++++++
 lisp/menu-bar.el      |  5 +++
 src/frame.c           |  5 ++-
 5 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..296b66aa56 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -512,6 +512,16 @@ Frame Commands
 Delete the selected frame (@code{delete-frame}).  This signals an
 error if there is only one frame.
 
+@item C-x 5 u
+@kindex C-x 5 u
+@findex undelete-frame
+@findex undelete-frame-mode
+Unless @code{undelete-frame-mode} is disabled, undelete one of the 16
+most recently deleted frames.  Without a prefix argument, the most
+recently deleted frame is undeleted.  With a numerical prefix argument
+between 0 and 15, where 0 is the most recently deleted frame, the
+corresponding deleted frame is undeleted.
+
 @item C-z
 @kindex C-z @r{(X windows)}
 Minimize (or iconify) the selected Emacs frame
diff --git a/etc/NEWS b/etc/NEWS
index 312fc18f4f..51cfe16677 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -130,6 +130,16 @@ For example, a 'display-buffer-alist' entry of
 will make the body of the chosen window 40 columns wide.  For the
 height use 'window-height' in combination with 'body-lines'.
 
+** Frames
+
++++
+*** Deleted frames can now be undeleted.
+The 16 most recently deleted frames can be undeleted with C-x 5 u,
+unless undelete-frame-mode is disabled.  Without a prefix argument,
+undelete the most recently deleted frame.  A numerical prefix argument
+between 0 and 15 undeletes the corresponding deleted frame, where 0 is
+the most recently deleted frame.
+
 ** Better detection of text suspiciously reordered on display.
 The function 'bidi-find-overridden-directionality' has been extended
 to detect reordering effects produced by embeddings and isolates
diff --git a/lisp/frame.el b/lisp/frame.el
index 2c73737a54..563391e7a1 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2484,6 +2484,93 @@ delete-other-frames
         (if iconify (iconify-frame this) (delete-frame this)))
       (setq this next))))
 
+(eval-when-compile (require 'frameset))
+
+(defvar undelete-frame--deleted-frames nil
+  "Internal variable used by `undelete-frame--handle-delete-frame'.")
+
+(defvar undelete-frame--enabled nil
+  "Internal variable used by `undelete-frame--handle-delete-frame'.")
+
+(defun undelete-frame--handle-delete-frame (frame)
+  "Save the configuration of frames deleted with `delete-frame'.
+Only the 16 most recently deleted frames are saved."
+  (when (and
+         ;; Skip the deletion of the terminal frame in
+         ;; `frame-initialize', but do not skip the first deletion of
+         ;; a frame when the daemon is used, nor the first deletion of
+         ;; a frame in a TTY.
+         (or undelete-frame--enabled
+             (daemonp)
+             (and (not (display-graphic-p))
+                  (seq-every-p
+                   (lambda (f) (not (frame-parameter f 'display)))
+                   (frame-list))))
+         (frame-live-p frame))
+    (setq undelete-frame--deleted-frames
+          (cons
+           (cons
+            (display-graphic-p)
+            (frameset-save
+             (list frame)
+             ;; When the daemon is started from a graphical
+             ;; environment, TTY frames have a 'display' parameter set
+             ;; to the value of $DISPLAY (see the note in
+             ;; `server--on-display-p').  Do not store that parameter
+             ;; in the frameset, otherwise `frameset-restore' attempts
+             ;; to restore a graphical frame.
+             :filters (if (display-graphic-p)
+                          frameset-filter-alist
+                        (cons '(display . :never)
+                              frameset-filter-alist))))
+           undelete-frame--deleted-frames))
+    (if (> (length undelete-frame--deleted-frames) 16)
+        (setq undelete-frame--deleted-frames
+              (butlast undelete-frame--deleted-frames))))
+  (setq undelete-frame--enabled t))
+
+(define-minor-mode undelete-frame-mode
+  "Enable the `delete-frame' command."
+  :group 'frames
+  :global t
+  :initialize 'custom-initialize-delay
+  :init-value t
+  (if undelete-frame-mode
+      (add-hook 'delete-frame-functions
+                #'undelete-frame--handle-delete-frame -75)
+    (remove-hook 'delete-frame-functions
+                 #'undelete-frame--handle-delete-frame)
+    (setq undelete-frame--deleted-frames nil)))
+
+(defun undelete-frame (&optional arg)
+  "Undelete a frame deleted with `delete-frame'.
+Without a prefix argument, or with with just \\[universal-argument], \
+undelete the most
+recently deleted frame.
+With a numerical prefix argument ARG between 0 and 15, where 0 is
+most recently deleted frame, undelete the ARGth deleted frame.
+When called from Lisp, returns the new frame."
+  (interactive "P")
+  (if (not undelete-frame-mode)
+      (message "undelete-frame-mode is disabled")
+    (let* ((frames (frame-list))
+           (n (if (listp arg) 0 arg))
+           (frameset (nth n undelete-frame--deleted-frames))
+           (graphic (display-graphic-p)))
+      (if (not frameset)
+          (message "No deleted frame saved at position %d" n)
+        (if (not (eq graphic (car frameset)))
+            (message
+             "Cannot undelete %sgraphic display frame on a %sgraphic display"
+             (if graphic "non-" "")
+             (if graphic "" "non-"))
+          (setq undelete-frame--deleted-frames
+                (delq frameset undelete-frame--deleted-frames))
+          (frameset-restore (cdr frameset))
+          (let ((frame (car (seq-difference (frame-list) frames))))
+            (when frame
+              (select-frame-set-input-focus frame)
+              frame)))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil
@@ -2828,6 +2915,7 @@ ctl-x-5-map
 (define-key ctl-x-5-map "o" #'other-frame)
 (define-key ctl-x-5-map "5" #'other-frame-prefix)
 (define-key ctl-x-5-map "c" #'clone-frame)
+(define-key ctl-x-5-map "u" #'undelete-frame)
 (define-key global-map [f11] #'toggle-frame-fullscreen)
 (define-key global-map [(meta f10)] #'toggle-frame-maximized)
 (define-key esc-map    [f10]        #'toggle-frame-maximized)
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 1a81f1a3d0..a5f7169355 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,11 @@ menu-bar-file-menu
       (bindings--define-key menu [separator-tab]
         menu-bar-separator))
 
+    (bindings--define-key menu [undelete-last-deleted-frame]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :enable undelete-frame-mode
+                  :help "Undelete last deleted frame"))
+
     ;; Don't use delete-frame as event name because that is a special
     ;; event.
     (bindings--define-key menu [delete-this-frame]
diff --git a/src/frame.c b/src/frame.c
index 79a7c89e0d..763ac65717 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2373,9 +2373,12 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 }
 
 DEFUN ("delete-frame", Fdelete_frame, Sdelete_frame, 0, 2, "",
-       doc: /* Delete FRAME, permanently eliminating it from use.
+       doc: /* Delete FRAME, eliminating it from use.
 FRAME must be a live frame and defaults to the selected one.
 
+Unless `undelete-frame-mode' is disabled, the 16 most recently deleted
+frames can be undeleted with `undelete-frame', which see.
+
 A frame may not be deleted if its minibuffer serves as surrogate
 minibuffer for another frame.  Normally, you may not delete a frame if
 all other frames are invisible, but if the second optional argument
-- 
2.33.0