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

From: Gregory Heytings <[email-masked]>
Date: Tue, 16 Nov 2021 21:29:35 +0000

Thanks for your comments, and thanks to Juri for his suggestion.

Updated patch attached, which incorporates your comments and Juri's 
suggestion.

>> +(@code{make-frame-command}).  A prefix argument undeletes the last deleted
>> +frame, a numerical prefix argument between 0 and 15 undeletes the
>> +corresponding deleted frame, where 0 is the most recently deleted frame.
>
> This is unusual meaning of prefix argument.  Why not use zero for the 
> last, 1 for the one before that, etc.?
>

You mean: 0 for the least recently deleted one, and 15 for the most 
recently deleted one?  So to recover the frame you just deleted by 
accident, you'd have to type C-u 15 C-x 5 u?  That seems unnatural to me, 
but perhaps it's just me.

>
> +(add-hook 'delete-frame-functions #'undeleted-frame--save-deleted-frame)
>
> I'd rather we didn't do that by default.  Several reasons:
>
> . the startup code deletes the terminal frame, so the above means we will always load frameset, which is not a small package, at startup, even if the user has no use for this functionality
> . using add-hook in Emacs's own code _by_default_ is not a good style; hooks are for customizing the default behavior
> . saving configurations of 16 deleted frames _by_default_ means we again impose on all users something that only some of them will use
>
> So I'd suggest instead making this an opt-in feature or maybe even minor 
> mode.  Only when turned on should we save away the deleted frames.
>

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.

>
> (And did you consider wrapping this into some history-like feature, 
> where users could interactively select which past frame to restore?)
>

Yes, I did consider this, but did not really know if it would be worth 
doing that.  Apparently it is ;-)  It's not yet clear to me how one could 
select one of the frames in a meaningful way.  I'll try to do that later.

>> +The 16 most recently deleted frames can however be undeleted with
>> +`undelete-frame', which see.
>
> The "however" part is "out of the blue" here; I'd drop it.
>

I see what you mean, but it's not out of the blue, it's meant to balance 
the "permanently eliminating" in "Delete FRAME, permanently eliminating it 
from use." two lines above.

From 6bd40494c6a1e6c27a8379146b77027e49048bbf Mon Sep 17 00:00:00 2001
From: Gregory Heytings <[email-masked]>
Date: Tue, 16 Nov 2021 21:19:18 +0000
Subject: [PATCH] Undelete deleted frames.

* lisp/frame.el (undelete-frame): New command.
(undeleted-frame--save-deleted-frame): New auxiliary function.
(undelete-frame--deleted-frames): New auxiliary variable.
(undelete-frame-mode): New minor mode.
(ctl-x-5-map): Bind the new command.

* src/frame.c (Fdelete_frame): Update docstring.

* 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              | 11 ++++++++
 lisp/frame.el         | 60 +++++++++++++++++++++++++++++++++++++++++++
 lisp/menu-bar.el      |  5 ++++
 src/frame.c           |  3 +++
 5 files changed, 89 insertions(+)

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..0217f1fe60 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -130,6 +130,17 @@ 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..e854197008 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2484,6 +2484,65 @@ 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--save-deleted-frame'.")
+
+(defun undeleted-frame--save-deleted-frame (frame)
+  "Save the configuration of frames deleted with `delete-frame'.
+Only the 16 most recently deleted frames are saved."
+  (when (and after-init-time
+             (frame-live-p frame))
+    (setq undelete-frame--deleted-frames
+          (cons (cons
+                 (display-graphic-p)
+                 (frameset-save (list frame)))
+                undelete-frame--deleted-frames))
+    (if (> (length undelete-frame--deleted-frames) 16)
+        (setq undelete-frame--deleted-frames
+              (butlast undelete-frame--deleted-frames)))))
+
+(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
+                #'undeleted-frame--save-deleted-frame)
+    (remove-hook 'delete-frame-functions
+                 #'undeleted-frame--save-deleted-frame)
+    (setq undelete-frame--deleted-frames nil)))
+
+(defun undelete-frame (&optional arg)
+  "Undelete a frame deleted with `delete-frame'.
+With a prefix argument ARG, undelete the most recently deleted
+frame.
+With a numerical prefix argument ARG between 0 and 15, undelete
+the ARGth deleted frame, where 0 is most recently 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)))
+      (if (not frameset)
+          (message "No deleted frame saved at position %d" n)
+        (if (not (eq (display-graphic-p) (car frameset)))
+            (message
+             "Cannot undelete %sgraphic display frame on a %sgraphic display"
+             (if (display-graphic-p) "non-" "")
+             (if (display-graphic-p) "" "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)))))))
 
 ;;; Window dividers.
 (defgroup window-divider nil
@@ -2828,6 +2887,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..9e11bc93ed 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2376,6 +2376,9 @@ DEFUN ("delete-frame", Fdelete_frame, Sdelete_frame, 0, 2, "",
        doc: /* Delete FRAME, permanently eliminating it from use.
 FRAME must be a live frame and defaults to the selected one.
 
+The 16 most recently deleted frames can however 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