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

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

[Michael Heerdegen]

Hello,

seems we don't have a command to restore a frame that the user has
killed by accident (note: we have `restore-frame', but that's something
different [related to maximizing commands]).  This is a feature request
to add such a command.

It seems this feature can't be implemented using window configurations
(AFAIU they die with their frame), and also not using frame
configurations (they don't work per frame) - but using one element
framesets seems to work:

#+begin_src emacs-lisp
(require 'frameset)
(require 'cl-lib)

(defvar my-killed-frame-ring-size 10)
(defvar my-killed-frames (make-ring my-killed-frame-ring-size))

(advice-add 'delete-frame :before #'my-remember-deleted-frame)

(defun my-remember-deleted-frame (&optional frame _force)
  (ring-insert my-killed-frames
               (frameset-save (list (or frame (selected-frame))))))

(defun my-restore-killed-frame (&optional n)
  (interactive "p")
  (let ((frames-before (frame-list)))
    (frameset-restore (ring-ref my-killed-frames (- (or n 1) 1)))
    (let ((restored (cl-set-difference (frame-list) frames-before)))
      (when (and restored (not (cdr restored)))
        (select-frame-set-input-focus (car restored))))))

(global-set-key [?\C-x ?5 ?t] #'my-restore-killed-frame)
#+end_src

Maybe having it in C would be better - I don't know.  A more convenient
access to frames killed earlier than the last one, instead of using the
prefix arg like above, might be appropriate (making the command
repeatable, maybe?)  Apart from these details the above is a start and
could just go to frameset.el.  Opinions?


TIA,

Michael.


In GNU Emacs 29.0.50 (build 25, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-11-15 built on drachen
Repository revision: 6b79b03c81f77ca00adeb80170653d4c0b53d46a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

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

[Juri Linkov]

> Maybe having it in C would be better - I don't know.

To avoid implementing the whole function in C,
'delete-frame' could call a hook with the frame as its arg,
then you can add your code to the hook.

> A more convenient access to frames killed earlier than the last one,
> instead of using the prefix arg like above, might be appropriate (making
> the command repeatable, maybe?)  Apart from these details the above is
> a start and could just go to frameset.el.  Opinions?

Very useful feature.  And like tab-undo is bound to 'C-x t u',
the frame closing undo could be bound to 'C-x 5 u'.

I haven't yet tested whether your implementation restores the tab-bar
as well, or maybe it would require more handling.  I suspect that
instead of framesets you might need to use window-state-get/window-state-put
like it's used in 'clone-frame'.  This means that code could go to frame.el.

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

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> To avoid implementing the whole function in C,
> 'delete-frame' could call a hook with the frame as its arg,
> then you can add your code to the hook.

Yes, that sounds generally useful for other things, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Juri Linkov]

> I haven't yet tested whether your implementation restores the tab-bar
> as well, or maybe it would require more handling.  I suspect that
> instead of framesets you might need to use window-state-get/window-state-put
> like it's used in 'clone-frame'.

Now I tested the recently added 'clone-frame' with the tab-bar,
and indeed cloning a frame with the tab-bar breaks the tabs
in the cloned frame, and breaks the tabs on the original frame.
The fix is pushed now in c25be3e7bb.

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

[Visuwesh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[ Please bear with me here as I try to use the web interface for the
mail archive.  I apologise if I messed something up and the message
doesn't get CC'd to the debbugs.  ]

Hi Juri,

>> Maybe having it in C would be better - I don't know.
> To avoid implementing the whole function in C,
> 'delete-frame' could call a hook with the frame as its arg,
> then you can add your code to the hook.

Doesn't Emacs already provide this?  See `delete-frame-functions'.

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

[Juri Linkov]

>>> Maybe having it in C would be better - I don't know.
>> To avoid implementing the whole function in C,
>> 'delete-frame' could call a hook with the frame as its arg,
>> then you can add your code to the hook.
>
> Doesn't Emacs already provide this?  See `delete-frame-functions'.

Thanks for mentioning `delete-frame-functions', then
we have everything necessary to implement this feature.

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

[Gregory Heytings]

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


Patch attached.

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

From 7d5ffe8af8348bf1b3ff888816f2517ee4aa345e Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 16 Nov 2021 15:10:41 +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.
(make-frame-command): Add a prefix argument, and call the new
command.

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

* doc/emacs/frames.tex (Creating Frames, Frame Commands): Document
the new command.

* etc/NEWS: Document the new command.

See bug#51883.
---
 doc/emacs/frames.texi | 11 ++++++---
 etc/NEWS              | 10 ++++++++
 lisp/frame.el         | 56 +++++++++++++++++++++++++++++++++++++++----
 lisp/menu-bar.el      |  4 ++++
 4 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..f5ede6af5e 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -452,8 +452,11 @@ Creating Frames
 @item C-x 5 2
 @kindex C-x 5 2
 @findex make-frame-command
-Create a new frame using the default frame parameters
-(@code{make-frame-command}).
+Create a new frame using the default frame parameters, or, with a prefix
+argument, undelete one of the 16 most recently deleted frames
+(@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.
 
 @item C-x 5 c
 @kindex C-x 5 c
@@ -510,7 +513,9 @@ Frame Commands
 @kindex C-x 5 0
 @findex delete-frame
 Delete the selected frame (@code{delete-frame}).  This signals an
-error if there is only one frame.
+error if there is only one frame.  The 16 most recently deleted frames
+can be undeleted with the @kbd{C-x 5 2} command, when it is used with
+a prefix argument.
 
 @item C-z
 @kindex C-z @r{(X windows)}
diff --git a/etc/NEWS b/etc/NEWS
index 312fc18f4f..ab47f8cf5b 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
+
++++
+*** With a prefix argument, the key 'C-x 5 2' undeletes deleted frames.
+The 16 most recently deleted frames can be undeleted.  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.
+
 ** 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..2085397b30 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -775,16 +775,23 @@ close-display-connection
       (mapc #'delete-frame frames)
       (x-close-connection display))))
 
-(defun make-frame-command ()
+(defun make-frame-command (&optional arg)
   "Make a new frame, on the same terminal as the selected frame.
 If the terminal is a text-only terminal, this also selects the
 new 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)
-  (if (display-graphic-p)
-      (make-frame)
-    (select-frame (make-frame))))
+  (interactive "P")
+  (if arg
+      (undelete-frame arg)
+    (if (display-graphic-p)
+        (make-frame)
+      (select-frame (make-frame)))))
 
 (defun clone-frame (&optional frame no-windows)
   "Make a new frame with the same parameters and windows as FRAME.
@@ -2484,6 +2491,45 @@ 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 (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)))))
+
+(add-hook 'delete-frame-functions #'undeleted-frame--save-deleted-frame)
+
+(defun undelete-frame (&optional arg)
+  "Undelete a frame deleted with `delete-frame'.
+When ARG is nil or a list, the last deleted frame is undeleted.
+When ARG is a number between 0 and 15, the ARGth deleted frame is
+undeleted.
+When called from Lisp, returns the new frame."
+  (interactive "P")
+  (let* ((frames (frame-list))
+         (n (if (listp arg) 0 arg))
+         (frameset (nth n undelete-frame--deleted-frames)))
+    (when (and frameset
+               (eq (display-graphic-p) (car frameset)))
+      (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
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 1a81f1a3d0..a1e0195bb5 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,10 @@ 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
+                  :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]
-- 
2.33.0

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

[Gregory Heytings]

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


Slightly improved patch attached.

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

From fd68d27dde26788820ebdc3ad816cefb6a5c659b Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Tue, 16 Nov 2021 17:01:37 +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.
(make-frame-command): Add a prefix argument, and call 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 (Creating Frames, Frame Commands): Document
the new command.

* etc/NEWS: Document the new command.

See bug#51883.
---
 doc/emacs/frames.texi | 11 +++++---
 etc/NEWS              | 10 +++++++
 lisp/frame.el         | 61 +++++++++++++++++++++++++++++++++++++++----
 lisp/menu-bar.el      |  4 +++
 src/frame.c           |  3 +++
 5 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..f5ede6af5e 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -452,8 +452,11 @@ Creating Frames
 @item C-x 5 2
 @kindex C-x 5 2
 @findex make-frame-command
-Create a new frame using the default frame parameters
-(@code{make-frame-command}).
+Create a new frame using the default frame parameters, or, with a prefix
+argument, undelete one of the 16 most recently deleted frames
+(@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.
 
 @item C-x 5 c
 @kindex C-x 5 c
@@ -510,7 +513,9 @@ Frame Commands
 @kindex C-x 5 0
 @findex delete-frame
 Delete the selected frame (@code{delete-frame}).  This signals an
-error if there is only one frame.
+error if there is only one frame.  The 16 most recently deleted frames
+can be undeleted with the @kbd{C-x 5 2} command, when it is used with
+a prefix argument.
 
 @item C-z
 @kindex C-z @r{(X windows)}
diff --git a/etc/NEWS b/etc/NEWS
index 312fc18f4f..ab47f8cf5b 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
+
++++
+*** With a prefix argument, the key 'C-x 5 2' undeletes deleted frames.
+The 16 most recently deleted frames can be undeleted.  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.
+
 ** 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..199e9e8005 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -775,16 +775,23 @@ close-display-connection
       (mapc #'delete-frame frames)
       (x-close-connection display))))
 
-(defun make-frame-command ()
+(defun make-frame-command (&optional arg)
   "Make a new frame, on the same terminal as the selected frame.
 If the terminal is a text-only terminal, this also selects the
 new 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)
-  (if (display-graphic-p)
-      (make-frame)
-    (select-frame (make-frame))))
+  (interactive "P")
+  (if arg
+      (undelete-frame arg)
+    (if (display-graphic-p)
+        (make-frame)
+      (select-frame (make-frame)))))
 
 (defun clone-frame (&optional frame no-windows)
   "Make a new frame with the same parameters and windows as FRAME.
@@ -2484,6 +2491,50 @@ 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 (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)))))
+
+(add-hook 'delete-frame-functions #'undeleted-frame--save-deleted-frame)
+
+(defun undelete-frame (&optional arg)
+  "Undelete a frame deleted with `delete-frame'.
+When ARG is nil or a list, the last deleted frame is undeleted.
+When ARG is a number between 0 and 15, the ARGth deleted frame is
+undeleted.
+When called from Lisp, returns the new frame."
+  (interactive "P")
+  (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))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 1a81f1a3d0..a1e0195bb5 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,10 @@ 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
+                  :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

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

[Eli Zaretskii]

> Date: Tue, 16 Nov 2021 17:05:07 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: Michael Heerdegen <michael_heerdegen@web.de>
> 
> Slightly improved patch attached.

Thanks, I have some comments below.

> +(@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.?

> +---
> +** Frames
> +
> ++++
> +*** With a prefix argument, the key 'C-x 5 2' undeletes deleted frames.

I would make the heading shorter and more to the point:

  *** Deleted frames can now be undeleted.

> +(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 (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)))))
> +
> +(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.

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

> +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.

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

[Gregory Heytings]

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


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.

[-- Attachment #2: Type: text/x-diff, Size: 7256 bytes --]

From 6bd40494c6a1e6c27a8379146b77027e49048bbf Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
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)))))))
 \f
 ;;; 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

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

[Gregory Heytings]

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


>
> And I solved the problem of the deletion of the terminal frame.
>

That solution wasn't robust enough, it would have failed if someone for 
some reason did a (setq after-init-time nil).  So I implemented something 
that shouldn't fail.

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

From 2fc329b63b64f2e53c3040c8f5cfd0b1f4667e07 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 17 Nov 2021 10:01:15 +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.

* 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         | 68 +++++++++++++++++++++++++++++++++++++++++++
 lisp/menu-bar.el      |  5 ++++
 src/frame.c           |  3 ++
 5 files changed, 97 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..1601ed1c44 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2484,6 +2484,73 @@ 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 (or undelete-frame--enabled
+                 (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)))
+                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)
+    (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 a non-numerical prefix 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)))
+      (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)))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil
@@ -2828,6 +2895,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

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

[Eli Zaretskii]

> Date: Tue, 16 Nov 2021 21:29:35 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 51883@debbugs.gnu.org, Juri Linkov <juri@linkov.net>, 
>     michael_heerdegen@web.de
> 
> Updated patch attached, which incorporates your comments and Juri's 
> suggestion.
> 
> > 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.

No, I mean "last" as in "the most recently deleted one".  Sorry for
being unclear.

Usually, commands that use both numeric and raw prefix arg do
something very different with the raw argument, which is not the case
here.

> 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?

As a nice bonus, making it opt-in will also allow to make the
implementation cleaner: no need for special handling of the initial
frame etc.

> > 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.

> +                 (and (not (display-graphic-p))
> +                      (seq-every-p
> +                       (lambda (f) (not (frame-parameter f 'display)))
> +                       (frame-list))))

This should have a comment explaining what it does and why.  (And I
hope we will be able to avoid doing that in the first place.)

Also, what happens with the daemon frame if this function is invoked
from a GUI frame?

> +Without a prefix argument, or with a non-numerical prefix argument,

This is better rephrased as

  ... or with just \\[universal-argument], ...

since "non-numerical prefix argument" will not necessarily be clear to
everyone quickly enough.

> +    (bindings--define-key menu [undelete-last-deleted-frame]
> +      '(menu-item "Undelete Frame" undelete-frame
> +                  :enable undelete-frame-mode
> +                  :help "Undelete last deleted frame"))

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.

>         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.

If "however" is because of "permanently", I'd rather we lost both.  It
makes little sense to say something and then contradict ourselves 2
sentences later.

The doc string should also mention the minor mode, because without it
the added sentence is inaccurate.

Thanks.

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

[Juri Linkov]

>> +    (bindings--define-key menu [undelete-last-deleted-frame]
>> +      '(menu-item "Undelete Frame" undelete-frame
>> +                  :enable undelete-frame-mode
>> +                  :help "Undelete last deleted frame"))
>
> 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.

Michael pointed out that "restore" is a too confusing name
where "restore" is opposite to making a frame fullscreen.
OTOH, "undelete" is similar to "undo", so "undelete" better explains
what it does.  And the main advantage of the name "undelete-frame"
is that it's immediately clear that it's opposite to "delete-frame".

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Gregory Heytings <gregory@heytings.org>,  51883@debbugs.gnu.org,
>   michael_heerdegen@web.de
> Date: Wed, 17 Nov 2021 19:06:06 +0200
> 
> >> +    (bindings--define-key menu [undelete-last-deleted-frame]
> >> +      '(menu-item "Undelete Frame" undelete-frame
> >> +                  :enable undelete-frame-mode
> >> +                  :help "Undelete last deleted frame"))
> >
> > 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.
> 
> Michael pointed out that "restore" is a too confusing name
> where "restore" is opposite to making a frame fullscreen.

And I raised a brow when reading that.  "Restore" is a very far cry
from "fullscreen".  OTOH, browsers that keep history use "restore", so
I thought it will be a better terminology here.

> OTOH, "undelete" is similar to "undo", so "undelete" better explains
> what it does.

The difference is that "undo" is widely accepted terminology for what
that does.

> And the main advantage of the name "undelete-frame" is that it's
> immediately clear that it's opposite to "delete-frame".

But undelete-frame (the command) is NOT the opposite of delete-frame,
because it doesn't just delete the last frame you deleted.

Anyway, I don't intend to argue more; if you are unhappy with my
proposal, so be it.

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

[Gregory Heytings]

[-- 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

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

[Eli Zaretskii]

> Date: Fri, 19 Nov 2021 09:00:39 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 51883@debbugs.gnu.org, juri@linkov.net, michael_heerdegen@web.de
> 
> > 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".

That's not "very different" in my book.  So I think only numeric
arguments should do that, with 1 being the default and meaning the
most-recently deleted one.  Yes, we will lose one frame this way, but
I don't think it's important enough to justify such strange usage of
the prefix arg.

> > 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.

No, please make it opt-in from the get-go, which will also remove the
need for some of the code which messes with the initial frame.  If
many users will request it be on by default, we will then reconsider.

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

[Gregory Heytings]

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


Updated patch attached.

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

From 1d969cd2850226baf967eb58619074b8d6e01ac4 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Wed, 24 Nov 2021 00:38:33 +0000
Subject: [PATCH] Undelete deleted frames.

* lisp/frame.el (undelete-frame): New command.
(undelete-last-deleted-frame): New auxiliary 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 |  9 ++++++
 etc/NEWS              |  9 ++++++
 lisp/frame.el         | 74 +++++++++++++++++++++++++++++++++++++++++++
 lisp/menu-bar.el      |  9 ++++++
 src/frame.c           |  5 ++-
 5 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..2f6e0f8300 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -512,6 +512,15 @@ 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
+When @code{undelete-frame-mode} is enabled, undelete one of the 16
+most recently deleted frames.  A numerical prefix argument between 1
+and 16, where 1 is the most recently deleted frame, specifies which
+deleted frame should be 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 bfea4da8b9..953c7ebdff 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -178,6 +178,15 @@ 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 when
+undelete-frame-mode is enabled.  A numerical prefix argument between 1
+and 16, where 1 is the most recently deleted frame, specifies which
+deleted frame should be undeleted.
+
 ** 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 1319759e74..a44ac4afaa 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2499,6 +2499,79 @@ 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'.")
+
+(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 (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)))))
+
+(define-minor-mode undelete-frame-mode
+  "Enable the `undelete-frame' command."
+  :group 'frames
+  :global 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-last-deleted-frame ()
+  "Undeleted the last frame deleted with `delete-frame'."
+  (interactive)
+  (undelete-frame 1))
+
+(defun undelete-frame (&optional arg)
+  "Undelete a frame deleted with `delete-frame'.
+With a numerical prefix argument ARG between 1 and 16, where 1 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")
+    (if (listp arg)
+        (message "Missing deleted frame number argument")
+      (let* ((frames (frame-list))
+             (frameset (nth (1- arg) undelete-frame--deleted-frames))
+             (graphic (display-graphic-p)))
+        (if (not frameset)
+            (message "No deleted frame with number %d" arg)
+          (if (not (eq graphic (car frameset)))
+              (message
+               "Cannot undelete a %s display frame on a %s display"
+               (if graphic "non-graphic" "graphic")
+               (if graphic "graphic" "non-graphic"))
+            (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
@@ -2843,6 +2916,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 274f594f69..e696e6c4c0 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,15 @@ menu-bar-file-menu
       (bindings--define-key menu [separator-tab]
         menu-bar-separator))
 
+    (bindings--define-key menu [enable-undelete-frame-mode]
+      '(menu-item "Enable Frame Undeletion" undelete-frame-mode
+                  :visible (null undelete-frame-mode)
+                  :help "Enable frame undeletion for this session"))
+    (bindings--define-key menu [undelete-last-deleted-frame]
+      '(menu-item "Undelete Frame" undelete-last-deleted-frame
+                  :visible 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 a21dd0d927..544fa0d02b 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2376,9 +2376,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.
 
+When `undelete-frame-mode' is enabled, 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

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> Updated patch attached.

Thanks for your work so far.

I tested your patch quickly.  It works!

But I noticed that C-x 5 u without prefix arg errors.  This is probably
not intended.  I think you rather want to test the raw prefix arg with
`consp', not with `listp'.  But why not just use (interactive "p")?

Regards,

Michael.

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

[Gregory Heytings]

>
> Thanks for your work so far.
>
> I tested your patch quickly.  It works!
>

Thanks for your feedback!

>
> But I noticed that C-x 5 u without prefix arg errors.  This is probably 
> not intended.  I think you rather want to test the raw prefix arg with 
> `consp', not with `listp'.  But why not just use (interactive "p")?
>

This is intended, it's what Eli wanted, unless I misunderstood what he 
meant.  In a previous version of the patch C-x 5 u without prefix arg 
undeleted the last deleted frame.  Now an explicit numerical prefix 
argument between 1 and 16 is required.

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

[Eli Zaretskii]

> Date: Sat, 27 Nov 2021 11:53:10 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, 51883@debbugs.gnu.org, juri@linkov.net
> 
> > But I noticed that C-x 5 u without prefix arg errors.  This is probably 
> > not intended.  I think you rather want to test the raw prefix arg with 
> > `consp', not with `listp'.  But why not just use (interactive "p")?
> >
> 
> This is intended, it's what Eli wanted, unless I misunderstood what he 
> meant.  In a previous version of the patch C-x 5 u without prefix arg 
> undeleted the last deleted frame.  Now an explicit numerical prefix 
> argument between 1 and 16 is required.

No, that's a misunderstanding, sorry.  I meant "C-x 5 u" to undelete
the most recently deleted frame, and 1 to 16 to mean undelete the
penultimate frame etc.  Basically ARG of N means undelete the N+1st
previously deleted frame.  I just didn't want the ARG of zero to be
handled specially.

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

[Gregory Heytings]

>
> I meant "C-x 5 u" to undelete the most recently deleted frame, and 1 to 
> 16 to mean undelete the penultimate frame etc.  Basically ARG of N means 
> undelete the N+1st previously deleted frame.  I just didn't want the ARG 
> of zero to be handled specially.
>

Okay, I'll do that.

So, just to make this crystal clear:

C-x 5 u -> undelete most recently deleted frame
C-u 0 C-x 5 u -> undelete most recently deleted frame
C-u 1 C-x 5 u -> undelete second most recently deleted frame
...

and

C-u C-x 5 u -> error

?

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

[Andreas Schwab]

On Nov 27 2021, Gregory Heytings wrote:

> C-u C-x 5 u -> error

Often, a lone C-u stands for 4 (if only a numeric argument is expected).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Eli Zaretskii]

> Date: Sat, 27 Nov 2021 12:12:15 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: michael_heerdegen@web.de, 51883@debbugs.gnu.org, juri@linkov.net
> 
> So, just to make this crystal clear:
> 
> C-x 5 u -> undelete most recently deleted frame
> C-u 0 C-x 5 u -> undelete most recently deleted frame
> C-u 1 C-x 5 u -> undelete second most recently deleted frame
> ...

Yes.

> and
> 
> C-u C-x 5 u -> error
> 
> ?

You could treat that as "C-u 4 C-x 5 u", perhaps, like C-f does.

Thanks, and again apologies for the misunderstanding I caused.

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> No, that's a misunderstanding, sorry.  I meant "C-x 5 u" to undelete
> the most recently deleted frame, and 1 to 16 to mean undelete the
> penultimate frame etc.  Basically ARG of N means undelete the N+1st
> previously deleted frame.

1 should undelete the second previously deleted frame?  I think it's
already complicated to guess the correct N, and then I would also have
to remember that I have to add or subtract 1... so why N+1?

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Gregory Heytings <gregory@heytings.org>,  51883@debbugs.gnu.org,
>   juri@linkov.net
> Date: Sat, 27 Nov 2021 13:23:58 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No, that's a misunderstanding, sorry.  I meant "C-x 5 u" to undelete
> > the most recently deleted frame, and 1 to 16 to mean undelete the
> > penultimate frame etc.  Basically ARG of N means undelete the N+1st
> > previously deleted frame.
> 
> 1 should undelete the second previously deleted frame?  I think it's
> already complicated to guess the correct N, and then I would also have
> to remember that I have to add or subtract 1... so why N+1?

You want to make "C-x 5 u" and "C-u 1 C-x 5 u" mean the same?  It's
possible, but we also have commands/functions that work like above,
no?

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> You want to make "C-x 5 u" and "C-u 1 C-x 5 u" mean the same?

Yes, I think that would be the simplest solution.

>  It's possible, but we also have commands/functions that work like
> above, no?

I'm not sure.  I didn't find one quickly.

In my opinion in this case it would not make much sense conceptionally,
numbering the last killed frames starting with 1 is more natural than
counting zero based.  Making it zero based would be an annoying detail
one has to remember without much gain.  Just my personal opinion without
having meditated long over the matter, of course.

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: gregory@heytings.org,  51883@debbugs.gnu.org,  juri@linkov.net
> Date: Sat, 27 Nov 2021 14:22:11 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You want to make "C-x 5 u" and "C-u 1 C-x 5 u" mean the same?
> 
> Yes, I think that would be the simplest solution.
> 
> >  It's possible, but we also have commands/functions that work like
> > above, no?
> 
> I'm not sure.  I didn't find one quickly.
> 
> In my opinion in this case it would not make much sense conceptionally,
> numbering the last killed frames starting with 1 is more natural than
> counting zero based.  Making it zero based would be an annoying detail
> one has to remember without much gain.  Just my personal opinion without
> having meditated long over the matter, of course.

Fine with me.

Gregory, please do it this way, to avoid future rework due to my own
idiosyncrasies.

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Fine with me.
>
> Gregory, please do it this way, to avoid future rework due to my own
> idiosyncrasies.

Let's help him and be clear: this means using just (interactive "p")
without any distinction of cases, right?

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: gregory@heytings.org,  51883@debbugs.gnu.org,  juri@linkov.net
> Date: Sat, 27 Nov 2021 14:34:56 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Fine with me.
> >
> > Gregory, please do it this way, to avoid future rework due to my own
> > idiosyncrasies.
> 
> Let's help him and be clear: this means using just (interactive "p")
> without any distinction of cases, right?

Probably.  But that doesn't tell what to do with "C-u 0", does it?

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Probably.  But that doesn't tell what to do with "C-u 0", does it?

I would want C-u 1 be like M-1 or no prefix arg and C-u 0 produce an
error because number 0 is invalid.

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: gregory@heytings.org,  51883@debbugs.gnu.org,  juri@linkov.net
> Date: Sat, 27 Nov 2021 14:59:43 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Probably.  But that doesn't tell what to do with "C-u 0", does it?
> 
> I would want C-u 1 be like M-1 or no prefix arg and C-u 0 produce an
> error because number 0 is invalid.

It is maybe better to ignore an argument of zero, i.e. behave as if
there was no argument.

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

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> It is maybe better to ignore an argument of zero, i.e. behave as if
> there was no argument.

If we do that (and I don't object) - what should negative arguments do?
Should -N be equivalent to +N?

Michael.

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

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: gregory@heytings.org,  51883@debbugs.gnu.org,  juri@linkov.net
> Date: Sat, 27 Nov 2021 15:08:34 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It is maybe better to ignore an argument of zero, i.e. behave as if
> > there was no argument.
> 
> If we do that (and I don't object) - what should negative arguments do?
> Should -N be equivalent to +N?

Either that or ignored.

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

[Gregory Heytings]

>
> Let's help him and be clear: this means using just (interactive "p") 
> without any distinction of cases, right?
>

No, because I do not want to see C-u C-x 5 u undelete the fourth most 
recently deleted frame.

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> No, because I do not want to see C-u C-x 5 u undelete the fourth most
> recently deleted frame.

What do you want in this case?

Michael.

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

[Gregory Heytings]

>>> Let's help him and be clear: this means using just (interactive "p") 
>>> without any distinction of cases, right?
>>
>> No, because I do not want to see C-u C-x 5 u undelete the fourth most 
>> recently deleted frame.
>
> What do you want in this case?
>

An error, something like "You didn't specify a frame number."

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> An error, something like "You didn't specify a frame number."

What's different in this case, compared to commands that do use
(interactive "p")?  Why is raising an error here better than doing what
at least some people expect?

Michael.

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

[Gregory Heytings]

>> An error, something like "You didn't specify a frame number."
>
> What's different in this case, compared to commands that do use 
> (interactive "p")?  Why is raising an error here better than doing what 
> at least some people expect?
>

They will not expect it if the docstring (and the error message) mentions 
that an explicit numerical argument must be given.  And I don't think that 
"undelete the fourth most recently deleted frame with just C-u" is 
something anyone would expect, in fact I think it would be more confusing 
than helpful.

Moreover I want to keep the C-u prefix free for later use.

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> Moreover I want to keep the C-u prefix free for later use.

I accept that argument.  Still, do we raise an error in similar cases?
But I won't argue about that detail...

Michael.

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

[Gregory Heytings]

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


Updated patch attached.

(A note for Michael: I know about the ring.el library, but it would have 
been necessary to require it in frame.el, which is not necessary, given 
that it's easy to get the same effect with standard functions.)

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

From 69657897097c986efcc859a070fbd850989c8343 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sat, 27 Nov 2021 16:59:51 +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): New auxiliary variables.
(undelete-frame-mode): New minor mode.
(ctl-x-5-map): Bind the new command.

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

* 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.

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

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..b0fa22660f 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
+When @code{undelete-frame-mode} is enabled, undelete one of the 16
+most recently deleted frames.  Without a prefix argument, undelete the
+most recently deleted frame.  With a numerical prefix argument between
+1 and 16, where 1 is the most recently deleted frame, undelete the
+corresponding deleted frame.
+
 @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 8b7c2f7850..45628b6a65 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -189,6 +189,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 when
+undelete-frame-mode is enabled.  Without a prefix argument, undelete
+the most recently deleted frame.  With a numerical prefix argument
+between 1 and 16, where 1 is the most recently deleted frame, undelete
+the corresponding deleted frame.
+
 ** Tab Bars and Tab Lines
 
 ---
diff --git a/lisp/frame.el b/lisp/frame.el
index 1319759e74..22e373fa0e 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2499,6 +2499,80 @@ 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'.")
+
+(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 (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)))))
+
+(define-minor-mode undelete-frame-mode
+  "Enable the `undelete-frame' command."
+  :group 'frames
+  :global 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, undelete the most recently deleted
+frame.
+With a numerical prefix argument ARG between 1 and 16, where 1 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)
+      (error "Undelete-Frame mode is disabled")
+    (if (consp arg)
+        (error "Missing deleted frame number argument")
+      (let* ((number (or arg 1))
+             (frames (frame-list))
+             (frameset (nth (1- number) undelete-frame--deleted-frames))
+             (graphic (display-graphic-p)))
+        (if (not (<= 1 number 16))
+            (error "%d is not a valid deleted frame number argument"
+                   number)
+          (if (not frameset)
+              (error "No deleted frame with number %d" number)
+            (if (not (eq graphic (car frameset)))
+                (error
+                 "Cannot undelete a %s display frame on a %s display"
+                 (if graphic "non-graphic" "graphic")
+                 (if graphic "graphic" "non-graphic"))
+              (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
@@ -2843,6 +2917,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 274f594f69..9500f577bc 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,15 @@ menu-bar-file-menu
       (bindings--define-key menu [separator-tab]
         menu-bar-separator))
 
+    (bindings--define-key menu [enable-undelete-frame-mode]
+      '(menu-item "Enable Frame Undeletion" undelete-frame-mode
+                  :visible (null undelete-frame-mode)
+                  :help "Enable frame undeletion for this session"))
+    (bindings--define-key menu [undelete-last-deleted-frame]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :visible undelete-frame-mode
+                  :help "Undelete the most recently 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 33e9606e41..04dd6e3550 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2376,9 +2376,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.
 
+When `undelete-frame-mode' is enabled, 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

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> Updated patch attached.

Thanks.  Hmm - one kind of raw prefix arg you don't handle yet is `-`
which is synonymous for -1.  M-- C-x 5 u gives the not so nice error
message:

  Wrong type argument: number-or-marker-p, -

Oh, and, I think all explicit `error' calls in your patch could be made
`user-error's instead, WDYT?

> (A note for Michael: I know about the ring.el library, but it would
> have been necessary to require it in frame.el, which is not necessary,
> given that it's easy to get the same effect with standard functions.)

Ok, fine with me.


Regards,

Michael.

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

[Gregory Heytings]

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


Thanks for your feedback.

>
> Thanks.  Hmm - one kind of raw prefix arg you don't handle yet is `-` 
> which is synonymous for -1.  M-- C-x 5 u gives the not so nice error 
> message:
>
> Wrong type argument: number-or-marker-p, -
>

Indeed, I forgot to handle that case.

>
> Oh, and, I think all explicit `error' calls in your patch could be made 
> `user-error's instead, WDYT?
>

That makes sense, indeed.  Done.

Updated patch attached.

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

From b84ef1881834fb0b25eb29005d158cf10712d8f3 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 29 Nov 2021 10:25:50 +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): New auxiliary variables.
(undelete-frame-mode): New minor mode.
(ctl-x-5-map): Bind the new command.

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

* 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.

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

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index c14ada2957..b0fa22660f 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
+When @code{undelete-frame-mode} is enabled, undelete one of the 16
+most recently deleted frames.  Without a prefix argument, undelete the
+most recently deleted frame.  With a numerical prefix argument between
+1 and 16, where 1 is the most recently deleted frame, undelete the
+corresponding deleted frame.
+
 @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 8b7c2f7850..45628b6a65 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -189,6 +189,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 when
+undelete-frame-mode is enabled.  Without a prefix argument, undelete
+the most recently deleted frame.  With a numerical prefix argument
+between 1 and 16, where 1 is the most recently deleted frame, undelete
+the corresponding deleted frame.
+
 ** Tab Bars and Tab Lines
 
 ---
diff --git a/lisp/frame.el b/lisp/frame.el
index 1319759e74..45dade7410 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2499,6 +2499,80 @@ 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'.")
+
+(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 (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)))))
+
+(define-minor-mode undelete-frame-mode
+  "Enable the `undelete-frame' command."
+  :group 'frames
+  :global 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, undelete the most recently deleted
+frame.
+With a numerical prefix argument ARG between 1 and 16, where 1 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)
+      (user-error "Undelete-Frame mode is disabled")
+    (if (consp arg)
+        (user-error "Missing deleted frame number argument")
+      (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
+             (frames (frame-list))
+             (frameset (nth (1- number) undelete-frame--deleted-frames))
+             (graphic (display-graphic-p)))
+        (if (not (<= 1 number 16))
+            (user-error "%d is not a valid deleted frame number argument"
+                        number)
+          (if (not frameset)
+              (user-error "No deleted frame with number %d" number)
+            (if (not (eq graphic (car frameset)))
+                (user-error
+                 "Cannot undelete a %s display frame on a %s display"
+                 (if graphic "non-graphic" "graphic")
+                 (if graphic "graphic" "non-graphic"))
+              (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
@@ -2843,6 +2917,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 274f594f69..9500f577bc 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -109,6 +109,15 @@ menu-bar-file-menu
       (bindings--define-key menu [separator-tab]
         menu-bar-separator))
 
+    (bindings--define-key menu [enable-undelete-frame-mode]
+      '(menu-item "Enable Frame Undeletion" undelete-frame-mode
+                  :visible (null undelete-frame-mode)
+                  :help "Enable frame undeletion for this session"))
+    (bindings--define-key menu [undelete-last-deleted-frame]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :visible undelete-frame-mode
+                  :help "Undelete the most recently 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 33e9606e41..04dd6e3550 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2376,9 +2376,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.
 
+When `undelete-frame-mode' is enabled, 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

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

[Michael Heerdegen]

Gregory Heytings <gregory@heytings.org> writes:

> Thanks for your feedback.

Thanks for your work and patience!

Ok, then we are done with comments from my side.  I think you can wait
another two days or so for other comments, and then it's really time to
install the patch to master.

Thanks!

Michael.

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thanks for your work and patience!

Oh - and the tab-bar - Juri had mentioned it - do we have to do anything
special about it?  In my tests your patch restored the tab-bar correctly
I think.

Michael.

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

[Juri Linkov]

>> Thanks for your work and patience!
>
> Oh - and the tab-bar - Juri had mentioned it - do we have to do anything
> special about it?  In my tests your patch restored the tab-bar correctly
> I think.

I'm waiting when the Gregory's patch will be pushed to master
to start testing the tab-bar with it :-)

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

[Juri Linkov]

>>> Thanks for your work and patience!
>>
>> Oh - and the tab-bar - Juri had mentioned it - do we have to do anything
>> special about it?  In my tests your patch restored the tab-bar correctly
>> I think.
>
> I'm waiting when the Gregory's patch will be pushed to master
> to start testing the tab-bar with it :-)

I guess Gregory has no access to the repo, so I've tested that
restoring the tab-bar works correctly, and pushed to master.

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

[Juri Linkov]

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

>>> Oh - and the tab-bar - Juri had mentioned it - do we have to do anything
>>> special about it?  In my tests your patch restored the tab-bar correctly
>>> I think.
>>
>> I'm waiting when the Gregory's patch will be pushed to master
>> to start testing the tab-bar with it :-)
>
> I guess Gregory has no access to the repo, so I've tested that
> restoring the tab-bar works correctly, and pushed to master.

I tried to use this, but undelete-frame-mode in the File menu
makes no sense: when you mistakenly deleted a frame, you want
to undelete it immediately, so you open the File menu, and
see the message "No way, you can't undelete the deleted frame,
because you were careless and not enabled a special mode".

So the most useful case for this feature is to get the
accidentally deleted frame back, and it fails to do this.

Instead, it allows undeleting 16 frames in a special mode.
Is there really a human that can delete 16 frames, and then
remember what was on the 16th frame back?

Rereading this thread indicates that the only concern about
enabling this by default was the memory footprint for remembering
16 frames.  OTOH, this feature is really useful for remembering
1 frame.  So this is what should be enabled by default:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: undelete-frame-max.patch --]
[-- Type: text/x-diff, Size: 2672 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 599ffe591a..b82a4ae26f 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2529,6 +2529,12 @@ delete-other-frames
         (if iconify (iconify-frame this) (delete-frame this)))
       (setq this next))))
 
+(defcustom undelete-frame-max 1
+  "Maximum number of frames deleted with `delete-frame'."
+  :type 'integer
+  :group 'frames
+  :version "29.1")
+
 (eval-when-compile (require 'frameset))
 
 (defvar undelete-frame--deleted-frames nil
@@ -2536,7 +2542,7 @@ undelete-frame--deleted-frames
 
 (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."
+Only the `undelete-frame-max' most recently deleted frames are saved."
   (when (frame-live-p frame)
     (setq undelete-frame--deleted-frames
           (cons
@@ -2555,26 +2561,18 @@ undelete-frame--handle-delete-frame
                         (cons '(display . :never)
                               frameset-filter-alist))))
            undelete-frame--deleted-frames))
-    (if (> (length undelete-frame--deleted-frames) 16)
+    (if (> (length undelete-frame--deleted-frames) undelete-frame-max)
         (setq undelete-frame--deleted-frames
               (butlast undelete-frame--deleted-frames)))))
 
-(define-minor-mode undelete-frame-mode
-  "Enable the `undelete-frame' command."
-  :group 'frames
-  :global 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)))
+(add-hook 'delete-frame-functions
+          #'undelete-frame--handle-delete-frame -75)
 
 (defun undelete-frame (&optional arg)
   "Undelete a frame deleted with `delete-frame'.
 Without a prefix argument, undelete the most recently deleted
 frame.
-With a numerical prefix argument ARG between 1 and 16, where 1 is
+With a numerical prefix argument ARG between 1 and `undelete-frame-max', where 1 is
 most recently deleted frame, undelete the ARGth deleted frame.
 When called from Lisp, returns the new frame."
   (interactive "P")
@@ -2586,7 +2584,7 @@ undelete-frame
              (frames (frame-list))
              (frameset (nth (1- number) undelete-frame--deleted-frames))
              (graphic (display-graphic-p)))
-        (if (not (<= 1 number 16))
+        (if (not (<= 1 number undelete-frame-max))
             (user-error "%d is not a valid deleted frame number argument"
                         number)
           (if (not frameset)

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

[Juri Linkov]

close 51883 29.0.50
quit

> I tried to use this, but undelete-frame-mode in the File menu
> makes no sense: when you mistakenly deleted a frame, you want
> to undelete it immediately, so you open the File menu, and
> see the message "No way, you can't undelete the deleted frame,
> because you were careless and not enabled a special mode".
>
> So the most useful case for this feature is to get the
> accidentally deleted frame back, and it fails to do this.
>
> Instead, it allows undeleting 16 frames in a special mode.
> Is there really a human that can delete 16 frames, and then
> remember what was on the 16th frame back?
>
> Rereading this thread indicates that the only concern about
> enabling this by default was the memory footprint for remembering
> 16 frames.  OTOH, this feature is really useful for remembering
> 1 frame.  So this is what should be enabled by default:

It seems this is the right thing to do, so now pushed to master.

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

[Michael Heerdegen]

Juri Linkov <juri@linkov.net> writes:

> It seems this is the right thing to do, so now pushed to master.

Thanks.

A detail: I find this:

| modified   etc/NEWS
| +The most recently deleted frame can be undeleted with 'C-x 5 u' when
| +the new user option 'undelete-frame-max' has its default value 1.

slightly confusing: any positive value will suffice to undelete the most
recently deleted frame.  Maybe say instead "...if the new user option
'undelete-frame-max' is enabled.  The option specifies the maximum
number of closed frames to remember.  The default is 1." or something
like that?

Michael.

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

[Juri Linkov]

> A detail: I find this:
>
> | modified   etc/NEWS
> | +The most recently deleted frame can be undeleted with 'C-x 5 u' when
> | +the new user option 'undelete-frame-max' has its default value 1.
>
> slightly confusing: any positive value will suffice to undelete the most
> recently deleted frame.  Maybe say instead "...if the new user option
> 'undelete-frame-max' is enabled.  The option specifies the maximum
> number of closed frames to remember.  The default is 1." or something
> like that?

Thanks for the suggestion, now changed with a better wording.

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 16 Jan 2022 22:59:20 +0200
> Cc: Gregory Heytings <gregory@heytings.org>, 51883@debbugs.gnu.org
> 
> close 51883 29.0.50
> quit
> 
> > I tried to use this, but undelete-frame-mode in the File menu
> > makes no sense: when you mistakenly deleted a frame, you want
> > to undelete it immediately, so you open the File menu, and
> > see the message "No way, you can't undelete the deleted frame,
> > because you were careless and not enabled a special mode".
> >
> > So the most useful case for this feature is to get the
> > accidentally deleted frame back, and it fails to do this.
> >
> > Instead, it allows undeleting 16 frames in a special mode.
> > Is there really a human that can delete 16 frames, and then
> > remember what was on the 16th frame back?
> >
> > Rereading this thread indicates that the only concern about
> > enabling this by default was the memory footprint for remembering
> > 16 frames.  OTOH, this feature is really useful for remembering
> > 1 frame.  So this is what should be enabled by default:
> 
> It seems this is the right thing to do, so now pushed to master.

I'm sorry, you cannot do that.  We discussed this at some length and
reached certain conclusions.  Then you come and in effect say those
considerations and discussions make no sense, and you know better?
Let's please respect our discussions and decisions more than that.
And if you want others to respect your opinions, please respect
theirs, even if you disagree.  The feature as installed allows you to
customize it to have that mode turned on by default, so you could
easily fix your problem by doing that.

Specifically to your main argument: it is no different from deleting a
file: unless the user took steps to configure the system to allow
undeleting deleted files, deleted files are lost forever.  Moreover,
in the case of an Emacs frame, nothing of terrible importance is
actually lost: the buffers displayed in that frame are still there,
and restoring the deleted frame by hand shouldn't take more than a few
moments.

So I reverted this changeset.  Please in the future don't make such
changes unilaterally.

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

[Juri Linkov]

>> > I tried to use this, but undelete-frame-mode in the File menu
>> > makes no sense: when you mistakenly deleted a frame, you want
>> > to undelete it immediately, so you open the File menu, and
>> > see the message "No way, you can't undelete the deleted frame,
>> > because you were careless and not enabled a special mode".
>> >
>> > So the most useful case for this feature is to get the
>> > accidentally deleted frame back, and it fails to do this.
>> >
>> > Instead, it allows undeleting 16 frames in a special mode.
>> > Is there really a human that can delete 16 frames, and then
>> > remember what was on the 16th frame back?
>> >
>> > Rereading this thread indicates that the only concern about
>> > enabling this by default was the memory footprint for remembering
>> > 16 frames.  OTOH, this feature is really useful for remembering
>> > 1 frame.  So this is what should be enabled by default:
>>
>> It seems this is the right thing to do, so now pushed to master.
>
> I'm sorry, you cannot do that.  We discussed this at some length and
> reached certain conclusions.  Then you come and in effect say those
> considerations and discussions make no sense, and you know better?
> Let's please respect our discussions and decisions more than that.
> And if you want others to respect your opinions, please respect
> theirs, even if you disagree.  The feature as installed allows you to
> customize it to have that mode turned on by default, so you could
> easily fix your problem by doing that.
>
> Specifically to your main argument: it is no different from deleting a
> file: unless the user took steps to configure the system to allow
> undeleting deleted files, deleted files are lost forever.  Moreover,
> in the case of an Emacs frame, nothing of terrible importance is
> actually lost: the buffers displayed in that frame are still there,
> and restoring the deleted frame by hand shouldn't take more than a few
> moments.
>
> So I reverted this changeset.  Please in the future don't make such
> changes unilaterally.

This is not true.  This is not a unilateral change.  I posted a patch,
then waited for comments 3 days, and when no one commented this means
that everyone agreed that it's a more reasonable change, then pushed to
master.

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: michael_heerdegen@web.de,  gregory@heytings.org,  51883@debbugs.gnu.org
> Date: Mon, 17 Jan 2022 20:41:26 +0200
> 
> >> > I tried to use this, but undelete-frame-mode in the File menu
> >> > makes no sense: when you mistakenly deleted a frame, you want
> >> > to undelete it immediately, so you open the File menu, and
> >> > see the message "No way, you can't undelete the deleted frame,
> >> > because you were careless and not enabled a special mode".
> >> >
> >> > So the most useful case for this feature is to get the
> >> > accidentally deleted frame back, and it fails to do this.
> >> >
> >> > Instead, it allows undeleting 16 frames in a special mode.
> >> > Is there really a human that can delete 16 frames, and then
> >> > remember what was on the 16th frame back?
> >> >
> >> > Rereading this thread indicates that the only concern about
> >> > enabling this by default was the memory footprint for remembering
> >> > 16 frames.  OTOH, this feature is really useful for remembering
> >> > 1 frame.  So this is what should be enabled by default:
> >>
> >> It seems this is the right thing to do, so now pushed to master.
> >
> > I'm sorry, you cannot do that.  We discussed this at some length and
> > reached certain conclusions.  Then you come and in effect say those
> > considerations and discussions make no sense, and you know better?
> > Let's please respect our discussions and decisions more than that.
> > And if you want others to respect your opinions, please respect
> > theirs, even if you disagree.  The feature as installed allows you to
> > customize it to have that mode turned on by default, so you could
> > easily fix your problem by doing that.
> >
> > Specifically to your main argument: it is no different from deleting a
> > file: unless the user took steps to configure the system to allow
> > undeleting deleted files, deleted files are lost forever.  Moreover,
> > in the case of an Emacs frame, nothing of terrible importance is
> > actually lost: the buffers displayed in that frame are still there,
> > and restoring the deleted frame by hand shouldn't take more than a few
> > moments.
> >
> > So I reverted this changeset.  Please in the future don't make such
> > changes unilaterally.
> 
> This is not true.  This is not a unilateral change.  I posted a patch,
> then waited for comments 3 days, and when no one commented this means
> that everyone agreed that it's a more reasonable change, then pushed to
> master.

I guess 3 days is not enough, especially in this time of year.  My
rule of thumb is to wait at least a week, possibly two.

But in any case, the amount of time you waited is not the main issue
here.  The main issue is that we decided to implement this the way we
did, and you were even part of that discussion.  It makes no sense to
undo all that because you suddenly don't like the results.

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

[Juri Linkov]

>> >> > I tried to use this, but undelete-frame-mode in the File menu
>> >> > makes no sense: when you mistakenly deleted a frame, you want
>> >> > to undelete it immediately, so you open the File menu, and
>> >> > see the message "No way, you can't undelete the deleted frame,
>> >> > because you were careless and not enabled a special mode".
>> >> >
>> >> > So the most useful case for this feature is to get the
>> >> > accidentally deleted frame back, and it fails to do this.
>> >> >
>> >> > Instead, it allows undeleting 16 frames in a special mode.
>> >> > Is there really a human that can delete 16 frames, and then
>> >> > remember what was on the 16th frame back?
>> >> >
>> >> > Rereading this thread indicates that the only concern about
>> >> > enabling this by default was the memory footprint for remembering
>> >> > 16 frames.  OTOH, this feature is really useful for remembering
>> >> > 1 frame.  So this is what should be enabled by default:
>> >>
>> >> It seems this is the right thing to do, so now pushed to master.
>> >
>> > I'm sorry, you cannot do that.  We discussed this at some length and
>> > reached certain conclusions.  Then you come and in effect say those
>> > considerations and discussions make no sense, and you know better?
>> > Let's please respect our discussions and decisions more than that.
>> > And if you want others to respect your opinions, please respect
>> > theirs, even if you disagree.  The feature as installed allows you to
>> > customize it to have that mode turned on by default, so you could
>> > easily fix your problem by doing that.
>> >
>> > Specifically to your main argument: it is no different from deleting a
>> > file: unless the user took steps to configure the system to allow
>> > undeleting deleted files, deleted files are lost forever.  Moreover,
>> > in the case of an Emacs frame, nothing of terrible importance is
>> > actually lost: the buffers displayed in that frame are still there,
>> > and restoring the deleted frame by hand shouldn't take more than a few
>> > moments.
>> >
>> > So I reverted this changeset.  Please in the future don't make such
>> > changes unilaterally.
>>
>> This is not true.  This is not a unilateral change.  I posted a patch,
>> then waited for comments 3 days, and when no one commented this means
>> that everyone agreed that it's a more reasonable change, then pushed to
>> master.
>
> I guess 3 days is not enough, especially in this time of year.  My
> rule of thumb is to wait at least a week, possibly two.
>
> But in any case, the amount of time you waited is not the main issue
> here.  The main issue is that we decided to implement this the way we
> did, and you were even part of that discussion.  It makes no sense to
> undo all that because you suddenly don't like the results.

After applying the patch to test whether it correctly restores the tab-bar,
I discovered that the weirdest thing was added to the main menu.

No other app has such unusual menu item in the File menu
because this feature is useful for everyone.
Just imagine trying to undelete a tab in a web browser,
then failing to do this when the browser requires
to enable a special mode to undelete tabs.  We can't afford
to make Emacs more bizarre than it currently is, that
would scare away users.

Then I recalled that the main argument in the discussion
against enabling this by default was the memory occupied
by 16 deleted frames.

So to simplify this I proposed a new option with the default value 1
instead of the hard-coded number 16.  Then waited for 3 days
that is standard practice.  And no comments means that everyone agreed
with this improvement.

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

[Juri Linkov]

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

>>> >> > I tried to use this, but undelete-frame-mode in the File menu
>>> >> > makes no sense: when you mistakenly deleted a frame, you want
>>> >> > to undelete it immediately, so you open the File menu, and
>>> >> > see the message "No way, you can't undelete the deleted frame,
>>> >> > because you were careless and not enabled a special mode".
>>> >> >
>>> >> > So the most useful case for this feature is to get the
>>> >> > accidentally deleted frame back, and it fails to do this.
>>> >> >
>>> >> > Instead, it allows undeleting 16 frames in a special mode.
>>> >> > Is there really a human that can delete 16 frames, and then
>>> >> > remember what was on the 16th frame back?
>>> >> >
>>> >> > Rereading this thread indicates that the only concern about
>>> >> > enabling this by default was the memory footprint for remembering
>>> >> > 16 frames.  OTOH, this feature is really useful for remembering
>>> >> > 1 frame.  So this is what should be enabled by default:
>>> >>
>>> >> It seems this is the right thing to do, so now pushed to master.
>>> >
>>> > I'm sorry, you cannot do that.  We discussed this at some length and
>>> > reached certain conclusions.  Then you come and in effect say those
>>> > considerations and discussions make no sense, and you know better?
>>> > Let's please respect our discussions and decisions more than that.
>>> > And if you want others to respect your opinions, please respect
>>> > theirs, even if you disagree.  The feature as installed allows you to
>>> > customize it to have that mode turned on by default, so you could
>>> > easily fix your problem by doing that.
>>> >
>>> > Specifically to your main argument: it is no different from deleting a
>>> > file: unless the user took steps to configure the system to allow
>>> > undeleting deleted files, deleted files are lost forever.  Moreover,
>>> > in the case of an Emacs frame, nothing of terrible importance is
>>> > actually lost: the buffers displayed in that frame are still there,
>>> > and restoring the deleted frame by hand shouldn't take more than a few
>>> > moments.
>>> >
>>> > So I reverted this changeset.  Please in the future don't make such
>>> > changes unilaterally.
>>>
>>> This is not true.  This is not a unilateral change.  I posted a patch,
>>> then waited for comments 3 days, and when no one commented this means
>>> that everyone agreed that it's a more reasonable change, then pushed to
>>> master.
>>
>> I guess 3 days is not enough, especially in this time of year.  My
>> rule of thumb is to wait at least a week, possibly two.
>>
>> But in any case, the amount of time you waited is not the main issue
>> here.  The main issue is that we decided to implement this the way we
>> did, and you were even part of that discussion.  It makes no sense to
>> undo all that because you suddenly don't like the results.
>
> After applying the patch to test whether it correctly restores the tab-bar,
> I discovered that the weirdest thing was added to the main menu.
>
> No other app has such unusual menu item in the File menu
> because this feature is useful for everyone.
> Just imagine trying to undelete a tab in a web browser,
> then failing to do this when the browser requires
> to enable a special mode to undelete tabs.  We can't afford
> to make Emacs more bizarre than it currently is, that
> would scare away users.
>
> Then I recalled that the main argument in the discussion
> against enabling this by default was the memory occupied
> by 16 deleted frames.
>
> So to simplify this I proposed a new option with the default value 1
> instead of the hard-coded number 16.  Then waited for 3 days
> that is standard practice.  And no comments means that everyone agreed
> with this improvement.

Here is the patch that fixes all these problems:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: undelete-frame-max.patch --]
[-- Type: text/x-diff, Size: 8228 bytes --]

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index ba58f70caf..c641b8ccb1 100644
--- b/doc/emacs/frames.texi
+++ a/doc/emacs/frames.texi
@@ -515,12 +515,14 @@
 @item C-x 5 u
 @kindex C-x 5 u
 @findex undelete-frame
-@findex undelete-frame-mode
-When @code{undelete-frame-mode} is enabled, undelete one of the 16
-most recently deleted frames.  Without a prefix argument, undelete the
-most recently deleted frame.  With a numerical prefix argument between
-1 and 16, where 1 is the most recently deleted frame, undelete the
-corresponding deleted frame.
+@findex undelete-frame-max
+Undelete one of the recently deleted frames.  The user option
+@code{undelete-frame-max} specifies the maximum number of deleted
+frames to keep (the default is 1).  Without a prefix argument,
+undelete the most recently deleted frame.  With a numerical prefix
+argument between 1 and the number specified by @code{undelete-frame-max},
+where 1 is the most recently deleted frame, undelete the corresponding
+deleted frame.
 
 @item C-z
 @kindex C-z @r{(X windows)}
diff --git a/etc/NEWS b/etc/NEWS
index fdbfd9b1be..2e748ce7c5 100644
--- b/etc/NEWS
+++ a/etc/NEWS
@@ -287,11 +287,12 @@
 
 +++
 *** Deleted frames can now be undeleted.
-The 16 most recently deleted frames can be undeleted with 'C-x 5 u' when
-'undelete-frame-mode' is enabled.  Without a prefix argument, undelete
-the most recently deleted frame.  With a numerical prefix argument
-between 1 and 16, where 1 is the most recently deleted frame, undelete
-the corresponding deleted frame.
+The most recently deleted frame can be undeleted with 'C-x 5 u' when
+the new user option 'undelete-frame-max' has its default value 1.
+Without a prefix argument, undelete the most recently deleted frame.
+With a numerical prefix argument between 1 and 'undelete-frame-max',
+where 1 is the most recently deleted frame, undelete the corresponding
+deleted frame.
 
 ** Tab Bars and Tab Lines
 
diff --git a/lisp/frame.el b/lisp/frame.el
index 5926a4d748..599ffe591a 100644
--- b/lisp/frame.el
+++ a/lisp/frame.el
@@ -2529,6 +2529,13 @@
         (if iconify (iconify-frame this) (delete-frame this)))
       (setq this next))))
 
+\f
+(defcustom undelete-frame-max 1
+  "Maximum number of deleted frames before oldest are thrown away."
+  :type 'integer
+  :group 'frames
+  :version "29.1")
+
 (eval-when-compile (require 'frameset))
 
 (defvar undelete-frame--deleted-frames nil
@@ -2536,7 +2543,7 @@
 
 (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."
+Only the `undelete-frame-max' most recently deleted frames are saved."
   (when (frame-live-p frame)
     (setq undelete-frame--deleted-frames
           (cons
@@ -2555,54 +2562,45 @@
                         (cons '(display . :never)
                               frameset-filter-alist))))
            undelete-frame--deleted-frames))
-    (if (> (length undelete-frame--deleted-frames) 16)
+    (if (> (length undelete-frame--deleted-frames) undelete-frame-max)
         (setq undelete-frame--deleted-frames
               (butlast undelete-frame--deleted-frames)))))
 
-(define-minor-mode undelete-frame-mode
-  "Enable the `undelete-frame' command."
-  :group 'frames
-  :global 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)))
+(add-hook 'after-init-hook
+          (lambda ()
+            (add-hook 'delete-frame-functions
+                      #'undelete-frame--handle-delete-frame -75)))
 
 (defun undelete-frame (&optional arg)
   "Undelete a frame deleted with `delete-frame'.
-Without a prefix argument, undelete the most recently deleted
-frame.
-With a numerical prefix argument ARG between 1 and 16, where 1 is
-most recently deleted frame, undelete the ARGth deleted frame.
+Without a prefix argument, undelete the most recently deleted frame.
+With a numerical prefix argument ARG between 1 and `undelete-frame-max',
+where 1 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)
-      (user-error "Undelete-Frame mode is disabled")
-    (if (consp arg)
-        (user-error "Missing deleted frame number argument")
-      (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
-             (frames (frame-list))
-             (frameset (nth (1- number) undelete-frame--deleted-frames))
-             (graphic (display-graphic-p)))
-        (if (not (<= 1 number 16))
-            (user-error "%d is not a valid deleted frame number argument"
-                        number)
-          (if (not frameset)
-              (user-error "No deleted frame with number %d" number)
-            (if (not (eq graphic (car frameset)))
-                (user-error
-                 "Cannot undelete a %s display frame on a %s display"
-                 (if graphic "non-graphic" "graphic")
-                 (if graphic "graphic" "non-graphic"))
-              (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)))))))))
+  (if (consp arg)
+      (user-error "Missing deleted frame number argument")
+    (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
+           (frames (frame-list))
+           (frameset (nth (1- number) undelete-frame--deleted-frames))
+           (graphic (display-graphic-p)))
+      (if (not (<= 1 number undelete-frame-max))
+          (user-error "%d is not a valid deleted frame number argument"
+                      number)
+        (if (not frameset)
+            (user-error "No deleted frame with number %d" number)
+          (if (not (eq graphic (car frameset)))
+              (user-error
+               "Cannot undelete a %s display frame on a %s display"
+               (if graphic "non-graphic" "graphic")
+               (if graphic "graphic" "non-graphic"))
+            (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
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index e5a070b24a..36cbd6a9c5 100644
--- b/lisp/menu-bar.el
+++ a/lisp/menu-bar.el
@@ -109,14 +109,9 @@
       (bindings--define-key menu [separator-tab]
         menu-bar-separator))
 
-    (bindings--define-key menu [enable-undelete-frame-mode]
-      '(menu-item "Enable Undeleting Frames" undelete-frame-mode
-                  :visible (null undelete-frame-mode)
-                  :help "Enable undeleting frames in this session"))
     (bindings--define-key menu [undelete-last-deleted-frame]
       '(menu-item "Undelete Frame" undelete-frame
-                  :visible (and undelete-frame-mode
-                                (car undelete-frame--deleted-frames))
+                  :visible (car undelete-frame--deleted-frames)
                   :help "Undelete the most recently deleted frame"))
 
     ;; Don't use delete-frame as event name because that is a special
diff --git a/src/frame.c b/src/frame.c
index 959f0c9c14..e5d74edc16 100644
--- b/src/frame.c
+++ a/src/frame.c
@@ -2385,7 +2385,7 @@
        doc: /* Delete FRAME, eliminating it from use.
 FRAME must be a live frame and defaults to the selected one.
 
-When `undelete-frame-mode' is enabled, the 16 most recently deleted
+When `undelete-frame-max' is more than 0, the most recently deleted
 frames can be undeleted with `undelete-frame', which see.
 
 A frame may not be deleted if its minibuffer serves as surrogate

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: michael_heerdegen@web.de,  gregory@heytings.org,  51883@debbugs.gnu.org
> Date: Wed, 19 Jan 2022 20:37:37 +0200
> 
> Here is the patch that fixes all these problems:

Which problems are those?

And how is this different from the changeset I reverted?

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Ok, then we are done with comments from my side.  I think you can wait
> another two days or so for other comments, and then it's really time to
> install the patch to master.

Can we install it now?

TIA,

Michael.

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
> > Ok, then we are done with comments from my side.  I think you can wait
> > another two days or so for other comments, and then it's really time to
> > install the patch to master.
>
> Can we install it now?

Gregory, are there any problems left, or do you need help with
installing?

Regards,

Michael.

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

[Eli Zaretskii]

> Date: Sat, 27 Nov 2021 14:12:56 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, 51883@debbugs.gnu.org, juri@linkov.net
> 
> > Let's help him and be clear: this means using just (interactive "p") 
> > without any distinction of cases, right?
> 
> No, because I do not want to see C-u C-x 5 u undelete the fourth most 
> recently deleted frame.

Why not?  It's the natural meaning of a bare C-u.  E.g., C-f behaves
like that.

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> I tested your patch quickly.  It works!

One thing you also might want to consider: if you use a ring instead of
a list to store the frames:

  (info "(elisp) Rings")

you don't have to care about removing old elements, it will be done
silently.  Apart from that you only have to exchange the insertion and
access functions, that's it.  We can also think about this detail
after installing your patch to master, though.

Apart from that detail, no more comments from my side, only a "thank
you".

Michael.

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

[Juri Linkov]

>> 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.
>
> No, please make it opt-in from the get-go, which will also remove the
> need for some of the code which messes with the initial frame.  If
> many users will request it be on by default, we will then reconsider.

Many users already requested it be on by default.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2021-11-16 19:40:08] wrote:
>> +(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

Clearly we should avoid putting the special initial-frame in the undo
log since we don't want (and can't) recreate it later anyway.

But as for loading the package, the problem is more general in the sense
that maybe we should refrain from loading the package just because
a frame is deleted.

So maybe we should change the code so the hook's function doesn't
require loading `frameset.el`.  AFAICT fundamentally, the only function
from `frameset.el` that it needs is `frameset-filter-params`, so maybe
we should move some of that code to `frame.el`.

To make up for it, maybe we can move some of the `undelete-frame` code
to `frameset.el` since it can't be used without `frameset.el` anyway?

>   . saving configurations of 16 deleted frames _by_default_ means we
>     again impose on all users something that only some of them will use

IIUC framesets are designed to be serializable so they shouldn't hold on
to external data like buffers and windows, so such 16 elements should
cost very little in terms of heap use.


        Stefan

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

[Juri Linkov]

> But as for loading the package, the problem is more general in the sense
> that maybe we should refrain from loading the package just because
> a frame is deleted.
>
> So maybe we should change the code so the hook's function doesn't
> require loading `frameset.el`.  AFAICT fundamentally, the only function
> from `frameset.el` that it needs is `frameset-filter-params`, so maybe
> we should move some of that code to `frame.el`.
>
> To make up for it, maybe we can move some of the `undelete-frame` code
> to `frameset.el` since it can't be used without `frameset.el` anyway?
>
>>   . saving configurations of 16 deleted frames _by_default_ means we
>>     again impose on all users something that only some of them will use
>
> IIUC framesets are designed to be serializable so they shouldn't hold on
> to external data like buffers and windows, so such 16 elements should
> cost very little in terms of heap use.

OTOH, since framesets are designed to be serializable, isn't it
overkill to use framesets in the same session?  For example,
`clone-frame` doesn't use framesets, and the effect of `clone-frame`
should be the same as what `undelete-frame` does.  The difference
between them is that with `clone-frame` the original frame is not deleted.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> IIUC framesets are designed to be serializable so they shouldn't hold on
>> to external data like buffers and windows, so such 16 elements should
>> cost very little in terms of heap use.
> OTOH, since framesets are designed to be serializable, isn't it
> overkill to use framesets in the same session?

Not sure what you mean by "overkill".  AFAIK it makes them more lightweight.

> For example, `clone-frame` doesn't use framesets, and the effect of
> `clone-frame` should be the same as what `undelete-frame` does.

Except it actually creates a frame, so it requires a lot more resources.


        Stefan

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

[Juri Linkov]

>>> IIUC framesets are designed to be serializable so they shouldn't hold on
>>> to external data like buffers and windows, so such 16 elements should
>>> cost very little in terms of heap use.
>> OTOH, since framesets are designed to be serializable, isn't it
>> overkill to use framesets in the same session?
>
> Not sure what you mean by "overkill".  AFAIK it makes them more lightweight.

Without framesets it doesn't require loading frameset.el.

>> For example, `clone-frame` doesn't use framesets, and the effect of
>> `clone-frame` should be the same as what `undelete-frame` does.
>
> Except it actually creates a frame, so it requires a lot more resources.

I don't understand: `clone-frame` creates a new frame with `make-frame`,
and `undelete-frame` creates a new frame with `make-frame-on-display`.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>>> IIUC framesets are designed to be serializable so they shouldn't hold on
>>>> to external data like buffers and windows, so such 16 elements should
>>>> cost very little in terms of heap use.
>>> OTOH, since framesets are designed to be serializable, isn't it
>>> overkill to use framesets in the same session?
>> Not sure what you mean by "overkill".  AFAIK it makes them more lightweight.
> Without framesets it doesn't require loading frameset.el.

The alternative to framesets, AFAIK is to keep actual frames around,
which are more expansive than framesets.

Or what do you suggest we use instead?

>>> For example, `clone-frame` doesn't use framesets, and the effect of
>>> `clone-frame` should be the same as what `undelete-frame` does.
>> Except it actually creates a frame, so it requires a lot more resources.
> I don't understand: `clone-frame` creates a new frame with `make-frame`,
> and `undelete-frame` creates a new frame with `make-frame-on-display`.

I was confused.  I still haven't understood you correctly, but at
least now I'm aware of it.

I'm talking about the cost of the representation of the frames we
deleted until the moment we undelete them.  I can't see how
`clone-frame` helps in this respect.


        Stefan

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

[Juri Linkov]

>>>>> IIUC framesets are designed to be serializable so they shouldn't hold on
>>>>> to external data like buffers and windows, so such 16 elements should
>>>>> cost very little in terms of heap use.
>>>> OTOH, since framesets are designed to be serializable, isn't it
>>>> overkill to use framesets in the same session?
>>> Not sure what you mean by "overkill".  AFAIK it makes them more lightweight.
>> Without framesets it doesn't require loading frameset.el.
>
> The alternative to framesets, AFAIK is to keep actual frames around,
> which are more expansive than framesets.
>
> Or what do you suggest we use instead?

I didn't mean to keep actual frames around.  I thought about keeping
only the same data that is used by `clone-frame` to make a new frame
identical to the original frame.  In case of `undelete-frame` this data
can be used to make a frame identical to the deleted frame.
I.e. a lightweight version of framesets, that avoids loading frameset.el.
This mostly means that after deleting the frame, only frame parameters
are kept from garbage collection, that later can be reused when making
a new frame on undeletion.

> I'm talking about the cost of the representation of the frames we
> deleted until the moment we undelete them.  I can't see how
> `clone-frame` helps in this respect.

Then the question is what takes more memory: loading frameset.el,
or keeping frame parameters of the deleted frame?

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I didn't mean to keep actual frames around.  I thought about keeping
> only the same data that is used by `clone-frame` to make a new frame
> identical to the original frame.  In case of `undelete-frame` this data
> can be used to make a frame identical to the deleted frame.
> I.e. a lightweight version of framesets, that avoids loading frameset.el.
> This mostly means that after deleting the frame, only frame parameters
> are kept from garbage collection, that later can be reused when making
> a new frame on undeletion.

Ah, got it, thanks.  Then fully agreed.  I was heading in the same
direction when I suggested to try and use as little as possible of
`frameset.el` and move the resulting code to `frame.el`.


        Stefan

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

[Juri Linkov]

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

>> I didn't mean to keep actual frames around.  I thought about keeping
>> only the same data that is used by `clone-frame` to make a new frame
>> identical to the original frame.  In case of `undelete-frame` this data
>> can be used to make a frame identical to the deleted frame.
>> I.e. a lightweight version of framesets, that avoids loading frameset.el.
>> This mostly means that after deleting the frame, only frame parameters
>> are kept from garbage collection, that later can be reused when making
>> a new frame on undeletion.
>
> Ah, got it, thanks.  Then fully agreed.  I was heading in the same
> direction when I suggested to try and use as little as possible of
> `frameset.el` and move the resulting code to `frame.el`.

While copying some code from `clone-frame` to `undelete-frame`,
I noticed that `clone-frame` doesn't work on non-GTK builds,
because window-id frame parameters keep frame-specific X-windows,
so `clone-frame` is broken in emacs-28.

Eli, is it ok to push the first patch to the release branch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: clone-frame-fix.patch --]
[-- Type: text/x-diff, Size: 703 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 599ffe591a..5de554eee6 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -798,8 +798,9 @@ clone-frame
          (windows (unless no-windows
                     (window-state-get (frame-root-window frame))))
          (default-frame-alist
-           (seq-remove (lambda (elem) (eq (car elem) 'name))
-                       (frame-parameters frame)))
+          (seq-remove (lambda (elem)
+                        (memq (car elem) '(name window-id outer-window-id parent-id)))
+                      (frame-parameters frame)))
          (new-frame (make-frame)))
     (when windows
       (window-state-put windows (frame-root-window new-frame) 'safe))

[-- Attachment #3: Type: text/plain, Size: 74 bytes --]


Then the second patch for master removes the dependency on frameset.el:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: undelete-frame-without-frameset.patch --]
[-- Type: text/x-diff, Size: 3431 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 599ffe591a..5de554eee6 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2529,8 +2530,6 @@ 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'.")
 
@@ -2540,20 +2539,12 @@ undelete-frame--handle-delete-frame
   (when (frame-live-p frame)
     (setq undelete-frame--deleted-frames
           (cons
-           (cons
+           (list
             (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))))
+            (seq-remove (lambda (elem)
+                          (memq (car elem) '(name window-id outer-window-id parent-id)))
+                       (frame-parameters frame))
+            (window-state-get (frame-root-window frame)))
            undelete-frame--deleted-frames))
     (if (> (length undelete-frame--deleted-frames) 16)
         (setq undelete-frame--deleted-frames
@@ -2583,26 +2574,25 @@ undelete-frame
     (if (consp arg)
         (user-error "Missing deleted frame number argument")
       (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
-             (frames (frame-list))
-             (frameset (nth (1- number) undelete-frame--deleted-frames))
+             (frame-data (nth (1- number) undelete-frame--deleted-frames))
              (graphic (display-graphic-p)))
         (if (not (<= 1 number 16))
             (user-error "%d is not a valid deleted frame number argument"
                         number)
-          (if (not frameset)
+          (if (not frame-data)
               (user-error "No deleted frame with number %d" number)
-            (if (not (eq graphic (car frameset)))
+            (if (not (eq graphic (nth 0 frame-data)))
                 (user-error
                  "Cannot undelete a %s display frame on a %s display"
                  (if graphic "non-graphic" "graphic")
                  (if graphic "graphic" "non-graphic"))
               (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)))))))))
+                    (delq frame-data undelete-frame--deleted-frames))
+              (let* ((default-frame-alist (nth 1 frame-data))
+                     (frame (make-frame)))
+                (window-state-put (nth 2 frame-data) (frame-root-window frame) 'safe)
+                (select-frame-set-input-focus frame)
+                frame))))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  michael_heerdegen@web.de,  Gregory
>  Heytings <gregory@heytings.org>,  51883@debbugs.gnu.org
> Date: Mon, 24 Jan 2022 20:12:48 +0200
> 
> While copying some code from `clone-frame` to `undelete-frame`,
> I noticed that `clone-frame` doesn't work on non-GTK builds,
> because window-id frame parameters keep frame-specific X-windows,
> so `clone-frame` is broken in emacs-28.
> 
> Eli, is it ok to push the first patch to the release branch?

I don't yet understand the change.  I could understand why window-id
should be removed, but why parent-id, for example?

Martin, any comments on this change?

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

[martin rudalics]

 > I don't yet understand the change.  I could understand why window-id
 > should be removed, but why parent-id, for example?
 >
 > Martin, any comments on this change?

The problematic part _is_ parent-id which makes the clone an "embedded"
frame, one for which FRAME_X_EMBEDDED_P (f) holds and one which Emacs
doesn't try to make visible in 'x-create-frame' because

      However, with explicit parent, Emacs
      cannot control visibility, so don't try.

Subsequent attempts to make that frame visible fail here in xterm.c

#ifdef USE_GTK
       gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
       gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
#else
       if (FRAME_X_EMBEDDED_P (f))
	xembed_set_info (f, XEMBED_MAPPED);
       else
	XMapRaised (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f));
#endif /* not USE_GTK */

which also explains why on GTK the frame does become visible.

So it suffices to remove only parent-id from the parameters passed to
'make-frame' (and I won't comment the idea to bind 'default-frame-alist'
when making the frame and the whole idea of 'clone-frame' - the value
returned by 'frame-parameters' is IMHO strictly not intended for feeding
it into 'make-frame' calls "just like that").

martin

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

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, michael_heerdegen@web.de, gregory@heytings.org,
>  51883@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 25 Jan 2022 10:28:29 +0100
> 
> So it suffices to remove only parent-id from the parameters passed to
> 'make-frame'

Thanks.

> (and I won't comment the idea to bind 'default-frame-alist'
> when making the frame and the whole idea of 'clone-frame' - the value
> returned by 'frame-parameters' is IMHO strictly not intended for feeding
> it into 'make-frame' calls "just like that").

Which other parameters would you suggest to remove (on master)?

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

[martin rudalics]

 > Which other parameters would you suggest to remove (on master)?

It would be pretentious to answer that question.  The comment starting
at line 261 of frameset.el tells that

;; - `window-id', `outer-window-id', `parent-id': They are assigned
;;   automatically and cannot be set, so keeping them is harmless, but they
;;   add clutter.  `window-system' is similar: it's assigned at frame
;;   creation, and does not serve any useful purpose later.

so according to that comment, leaving 'parent-id' in the list should not
have caused any problems.  Thanks to Juri, we know better now.

For anyone who wants to peruse the parameters of an existing frame,
however, reading the ";; Filtering" comment in frameset.el should be an
indispensable prerequisite.  Juanma (and maybe others) have invested a
lot of work there - consider entries on 'frameset--text-pixel-height' or
'minibuffer'.  Disregarding that text when implementing a function that
pretends to clone a frame is just recklessness.

martin

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

[Juri Linkov]

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

>> Which other parameters would you suggest to remove (on master)?
>
> It would be pretentious to answer that question.  The comment starting
> at line 261 of frameset.el tells that
>
> ;; - `window-id', `outer-window-id', `parent-id': They are assigned
> ;;   automatically and cannot be set, so keeping them is harmless, but they
> ;;   add clutter.  `window-system' is similar: it's assigned at frame
> ;;   creation, and does not serve any useful purpose later.
>
> so according to that comment, leaving 'parent-id' in the list should not
> have caused any problems.  Thanks to Juri, we know better now.

I confirm that removing 'parent-id' is sufficient to fix the bug in Emacs 28.
So here is the minimal patch for the release branch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: clone-frame.patch --]
[-- Type: text/x-diff, Size: 677 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 86c52dc438..69119b4c24 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -798,8 +798,9 @@ clone-frame
          (windows (unless no-windows
                     (window-state-get (frame-root-window frame))))
          (default-frame-alist
-           (seq-remove (lambda (elem) (eq (car elem) 'name))
-                       (frame-parameters frame)))
+          (seq-remove (lambda (elem)
+                        (memq (car elem) '(name parent-id)))
+                      (frame-parameters frame)))
          (new-frame (make-frame)))
     (when windows
       (window-state-put windows (frame-root-window new-frame) 'safe))

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  monnier@iro.umontreal.ca,
>   michael_heerdegen@web.de,  gregory@heytings.org,  51883@debbugs.gnu.org
> Date: Thu, 27 Jan 2022 19:21:24 +0200
> 
> I confirm that removing 'parent-id' is sufficient to fix the bug in Emacs 28.
> So here is the minimal patch for the release branch:

Thanks, please install this on the release branch.

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

[Juri Linkov]

>> I confirm that removing 'parent-id' is sufficient to fix the bug in Emacs 28.
>> So here is the minimal patch for the release branch:
>
> Thanks, please install this on the release branch.

Done.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> -           (seq-remove (lambda (elem) (eq (car elem) 'name))
> -                       (frame-parameters frame)))
> +          (seq-remove (lambda (elem)
> +                        (memq (car elem) '(name window-id outer-window-id parent-id)))
> +                      (frame-parameters frame)))

Could you move this list into a variable and then use it in
`frameset.el` when computing `frameset-persistent-filter-alist` (tho
I suspect they should be in `frameset-session-filter-alist` even they
currently aren't in there, which might be a bug in `frameset.el`).


        Stefan

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

[Juri Linkov]

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

>> -           (seq-remove (lambda (elem) (eq (car elem) 'name))
>> -                       (frame-parameters frame)))
>> +          (seq-remove (lambda (elem)
>> +                        (memq (car elem) '(name window-id outer-window-id parent-id)))
>> +                      (frame-parameters frame)))
>
> Could you move this list into a variable and then use it in
> `frameset.el` when computing `frameset-persistent-filter-alist` (tho
> I suspect they should be in `frameset-session-filter-alist` even they
> currently aren't in there, which might be a bug in `frameset.el`).

Done in this patch (for master):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: frame-internal-parameters.patch --]
[-- Type: text/x-diff, Size: 6303 bytes --]

diff --git a/src/frame.c b/src/frame.c
index 8aaff949ba..b0b9d1184e 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -6495,6 +6495,14 @@ focus (where a frame immediately loses focus when it's left by the mouse
 iconify the top level frame instead.  */);
   iconify_child_frame = Qiconify_top_level;
 
+  DEFVAR_LISP ("frame-internal-parameters", frame_internal_parameters,
+	       doc: /* Frame parameters specific to every frame.  */);
+#ifdef HAVE_X_WINDOWS
+  frame_internal_parameters = list4 (Qname, Qparent_id, Qwindow_id, Qouter_window_id);
+#else
+  frame_internal_parameters = list3 (Qname, Qparent_id, Qwindow_id);
+#endif
+
   defsubr (&Sframep);
   defsubr (&Sframe_live_p);
   defsubr (&Swindow_system);
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 10714af1fa..05884eed3a 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -436,10 +436,11 @@ frameset-prop
 
 ;;;###autoload
 (defvar frameset-session-filter-alist
-  '((name            . :never)
-    (left            . frameset-filter-iconified)
-    (minibuffer      . frameset-filter-minibuffer)
-    (top             . frameset-filter-iconified))
+  (append
+   '((left            . frameset-filter-iconified)
+     (minibuffer      . frameset-filter-minibuffer)
+     (top             . frameset-filter-iconified))
+   (mapcar (lambda (p) (cons p :never)) frame-internal-parameters))
   "Minimum set of parameters to filter for live (on-session) framesets.
 DO NOT MODIFY.  See `frameset-filter-alist' for a full description.")
 
@@ -468,14 +469,11 @@ frameset-persistent-filter-alist
      (GUI:height                  . frameset-filter-unshelve-param)
      (GUI:width                   . frameset-filter-unshelve-param)
      (height                      . frameset-filter-shelve-param)
-     (outer-window-id             . :never)
      (parent-frame                . :never)
-     (parent-id                   . :never)
      (mouse-wheel-frame           . :never)
      (tty                         . frameset-filter-tty-to-GUI)
      (tty-type                    . frameset-filter-tty-to-GUI)
      (width                       . frameset-filter-shelve-param)
-     (window-id                   . :never)
      (window-system               . :never))
    frameset-session-filter-alist)
   "Parameters to filter for persistent framesets.
diff --git a/lisp/frame.el b/lisp/frame.el
index 599ffe591a..d9b21783b9 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -798,8 +798,9 @@ clone-frame
          (windows (unless no-windows
                     (window-state-get (frame-root-window frame))))
          (default-frame-alist
-           (seq-remove (lambda (elem) (eq (car elem) 'name))
-                       (frame-parameters frame)))
+          (seq-remove (lambda (elem)
+                        (memq (car elem) frame-internal-parameters))
+                      (frame-parameters frame)))
          (new-frame (make-frame)))
     (when windows
       (window-state-put windows (frame-root-window new-frame) 'safe))
@@ -2529,8 +2530,6 @@ 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'.")
 
@@ -2540,20 +2539,12 @@ undelete-frame--handle-delete-frame
   (when (frame-live-p frame)
     (setq undelete-frame--deleted-frames
           (cons
-           (cons
+           (list
             (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))))
+            (seq-remove (lambda (elem)
+                          (memq (car elem) frame-internal-parameters))
+                       (frame-parameters frame))
+            (window-state-get (frame-root-window frame)))
            undelete-frame--deleted-frames))
     (if (> (length undelete-frame--deleted-frames) 16)
         (setq undelete-frame--deleted-frames
@@ -2583,26 +2574,25 @@ undelete-frame
     (if (consp arg)
         (user-error "Missing deleted frame number argument")
       (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
-             (frames (frame-list))
-             (frameset (nth (1- number) undelete-frame--deleted-frames))
+             (frame-data (nth (1- number) undelete-frame--deleted-frames))
              (graphic (display-graphic-p)))
         (if (not (<= 1 number 16))
             (user-error "%d is not a valid deleted frame number argument"
                         number)
-          (if (not frameset)
+          (if (not frame-data)
               (user-error "No deleted frame with number %d" number)
-            (if (not (eq graphic (car frameset)))
+            (if (not (eq graphic (nth 0 frame-data)))
                 (user-error
                  "Cannot undelete a %s display frame on a %s display"
                  (if graphic "non-graphic" "graphic")
                  (if graphic "graphic" "non-graphic"))
               (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)))))))))
+                    (delq frame-data undelete-frame--deleted-frames))
+              (let* ((default-frame-alist (nth 1 frame-data))
+                     (frame (make-frame)))
+                (window-state-put (nth 2 frame-data) (frame-root-window frame) 'safe)
+                (select-frame-set-input-focus frame)
+                frame))))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil

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

[Juri Linkov]

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

> Eli Zaretskii [2021-11-16 19:40:08] wrote:
>>> +(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
>
> Clearly we should avoid putting the special initial-frame in the undo
> log since we don't want (and can't) recreate it later anyway.

This is fixed now.

> But as for loading the package, the problem is more general in the sense
> that maybe we should refrain from loading the package just because
> a frame is deleted.
>
> So maybe we should change the code so the hook's function doesn't
> require loading `frameset.el`.  AFAICT fundamentally, the only function
> from `frameset.el` that it needs is `frameset-filter-params`, so maybe
> we should move some of that code to `frame.el`.

Now the undelete-frame feature doesn't load frameset.el anymore.

So all concerns raised by Eli are addressed now
that make possible to enable it by default
as was discussed on emacs-devel:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: undelete-frame-max.patch --]
[-- Type: text/x-diff, Size: 8489 bytes --]

diff --git a/doc/emacs/frames.texi b/doc/emacs/frames.texi
index 29edbe9863..39b38a95d3 100644
--- a/doc/emacs/frames.texi
+++ b/doc/emacs/frames.texi
@@ -515,12 +515,14 @@ Frame Commands
 @item C-x 5 u
 @kindex C-x 5 u
 @findex undelete-frame
-@findex undelete-frame-mode
-When @code{undelete-frame-mode} is enabled, undelete one of the 16
-most recently deleted frames.  Without a prefix argument, undelete the
-most recently deleted frame.  With a numerical prefix argument between
-1 and 16, where 1 is the most recently deleted frame, undelete the
-corresponding deleted frame.
+@findex undelete-frame-max
+Undelete one of the recently deleted frames.  The user option
+@code{undelete-frame-max} specifies the maximum number of deleted
+frames to keep (the default is 1).  Without a prefix argument,
+undelete the most recently deleted frame.  With a numerical prefix
+argument between 1 and the number specified by @code{undelete-frame-max},
+where 1 is the most recently deleted frame, undelete the corresponding
+deleted frame.
 
 @item C-z
 @kindex C-z @r{(X windows)}
diff --git a/etc/NEWS b/etc/NEWS
index 19d23e8943..385ddb0262 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -327,11 +327,13 @@ height use 'window-height' in combination with 'body-lines'.
 
 +++
 *** Deleted frames can now be undeleted.
-The 16 most recently deleted frames can be undeleted with 'C-x 5 u' when
-'undelete-frame-mode' is enabled.  Without a prefix argument, undelete
-the most recently deleted frame.  With a numerical prefix argument
-between 1 and 16, where 1 is the most recently deleted frame, undelete
-the corresponding deleted frame.
+The most recently deleted frame can be undeleted with 'C-x 5 u'
+if the new user option 'undelete-frame-max' is a positive number.
+This option specifies the maximum number of closed frames to remember.
+The default is 1.  Without a prefix argument, undelete the most
+recently deleted frame.  With a numerical prefix argument between 1
+and the value of 'undelete-frame-max', where 1 is the most recently
+deleted frame, undelete the corresponding deleted frame.
 
 ** Tab Bars and Tab Lines
 
diff --git a/lisp/frame.el b/lisp/frame.el
index d39597d0af..143c51b98c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2530,12 +2530,19 @@ delete-other-frames
         (if iconify (iconify-frame this) (delete-frame this)))
       (setq this next))))
 
+\f
+(defcustom undelete-frame-max 1
+  "Maximum number of deleted frames before oldest are thrown away."
+  :type 'integer
+  :group 'frames
+  :version "29.1")
+
 (defvar undelete-frame--deleted-frames nil
   "Internal variable used by `undelete-frame--save-deleted-frame'.")
 
 (defun undelete-frame--save-deleted-frame (frame)
   "Save the configuration of frames deleted with `delete-frame'.
-Only the 16 most recently deleted frames are saved."
+Only the `undelete-frame-max' most recently deleted frames are saved."
   (when (and after-init-time (frame-live-p frame))
     (setq undelete-frame--deleted-frames
           (cons
@@ -2554,53 +2561,41 @@ undelete-frame--save-deleted-frame
              (frame-parameters frame))
             (window-state-get (frame-root-window frame)))
            undelete-frame--deleted-frames))
-    (if (> (length undelete-frame--deleted-frames) 16)
+    (if (> (length undelete-frame--deleted-frames) undelete-frame-max)
         (setq undelete-frame--deleted-frames
               (butlast undelete-frame--deleted-frames)))))
 
-(define-minor-mode undelete-frame-mode
-  "Enable the `undelete-frame' command."
-  :group 'frames
-  :global t
-  (if undelete-frame-mode
-      (add-hook 'delete-frame-functions
-                #'undelete-frame--save-deleted-frame -75)
-    (remove-hook 'delete-frame-functions
-                 #'undelete-frame--save-deleted-frame)
-    (setq undelete-frame--deleted-frames nil)))
+(add-hook 'delete-frame-functions #'undelete-frame--save-deleted-frame -75)
 
 (defun undelete-frame (&optional arg)
   "Undelete a frame deleted with `delete-frame'.
-Without a prefix argument, undelete the most recently deleted
-frame.
-With a numerical prefix argument ARG between 1 and 16, where 1 is
-most recently deleted frame, undelete the ARGth deleted frame.
+Without a prefix argument, undelete the most recently deleted frame.
+With a numerical prefix argument ARG between 1 and `undelete-frame-max',
+where 1 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)
-      (user-error "Undelete-Frame mode is disabled")
-    (if (consp arg)
-        (user-error "Missing deleted frame number argument")
-      (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
-             (frame-data (nth (1- number) undelete-frame--deleted-frames))
-             (graphic (display-graphic-p)))
-        (if (not (<= 1 number 16))
-            (user-error "%d is not a valid deleted frame number argument"
-                        number)
-          (if (not frame-data)
-              (user-error "No deleted frame with number %d" number)
-            (if (not (eq graphic (nth 0 frame-data)))
-                (user-error
-                 "Cannot undelete a %s display frame on a %s display"
-                 (if graphic "non-graphic" "graphic")
-                 (if graphic "graphic" "non-graphic"))
-              (setq undelete-frame--deleted-frames
-                    (delq frame-data undelete-frame--deleted-frames))
-              (let* ((default-frame-alist (nth 1 frame-data))
-                     (frame (make-frame)))
-                (window-state-put (nth 2 frame-data) (frame-root-window frame) 'safe)
-                (select-frame-set-input-focus frame)
-                frame))))))))
+  (if (consp arg)
+      (user-error "Missing deleted frame number argument")
+    (let* ((number (pcase arg ('nil 1) ('- -1) (_ arg)))
+           (frame-data (nth (1- number) undelete-frame--deleted-frames))
+           (graphic (display-graphic-p)))
+      (if (not (<= 1 number undelete-frame-max))
+          (user-error "%d is not a valid deleted frame number argument"
+                      number)
+        (if (not frame-data)
+            (user-error "No deleted frame with number %d" number)
+          (if (not (eq graphic (nth 0 frame-data)))
+              (user-error
+               "Cannot undelete a %s display frame on a %s display"
+               (if graphic "non-graphic" "graphic")
+               (if graphic "graphic" "non-graphic"))
+            (setq undelete-frame--deleted-frames
+                  (delq frame-data undelete-frame--deleted-frames))
+            (let* ((default-frame-alist (nth 1 frame-data))
+                   (frame (make-frame)))
+              (window-state-put (nth 2 frame-data) (frame-root-window frame) 'safe)
+              (select-frame-set-input-focus frame)
+              frame)))))))
 \f
 ;;; Window dividers.
 (defgroup window-divider nil
diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index b6dbf209ec..df55b3be89 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,15 +108,9 @@ menu-bar-file-menu
     (bindings--define-key menu [separator-tab]
       menu-bar-separator)
 
-    (bindings--define-key menu [undelete-frame-mode]
-      '(menu-item "Allow Undeleting Frames" undelete-frame-mode
-                  :help "Allow frames to be restored after deletion"
-                  :button (:toggle . undelete-frame-mode)))
-
     (bindings--define-key menu [undelete-last-deleted-frame]
       '(menu-item "Undelete Frame" undelete-frame
-                  :enable (and undelete-frame-mode
-                                (car undelete-frame--deleted-frames))
+                  :visible (car undelete-frame--deleted-frames)
                   :help "Undelete the most recently deleted frame"))
 
     ;; Don't use delete-frame as event name because that is a special
diff --git a/src/frame.c b/src/frame.c
index f94dff0a60..1d0dcf6278 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2385,7 +2385,7 @@ DEFUN ("delete-frame", Fdelete_frame, Sdelete_frame, 0, 2, "",
        doc: /* Delete FRAME, eliminating it from use.
 FRAME must be a live frame and defaults to the selected one.
 
-When `undelete-frame-mode' is enabled, the 16 most recently deleted
+When `undelete-frame-max' is more than 0, the most recently deleted
 frames can be undeleted with `undelete-frame', which see.
 
 A frame may not be deleted if its minibuffer serves as surrogate

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  michael_heerdegen@web.de,  Gregory Heytings
>  <gregory@heytings.org>,  51883@debbugs.gnu.org
> Date: Sun, 30 Jan 2022 18:39:16 +0200
> 
> So all concerns raised by Eli are addressed now
> that make possible to enable it by default
> as was discussed on emacs-devel:

Once again, please do NOT enable this by default, I didn't agree to
that.  This feature should remain disabled by default for some time,
until we have enough information to discuss whether it should be
turned on by default.  As discussed and agreed during its development.

>  @item C-x 5 u
>  @kindex C-x 5 u
>  @findex undelete-frame
> -@findex undelete-frame-mode
> -When @code{undelete-frame-mode} is enabled, undelete one of the 16
> -most recently deleted frames.  Without a prefix argument, undelete the
> -most recently deleted frame.  With a numerical prefix argument between
> -1 and 16, where 1 is the most recently deleted frame, undelete the
> -corresponding deleted frame.
> +@findex undelete-frame-max
> +Undelete one of the recently deleted frames.  The user option

"One of the recently deleted frames" sounds like Emacs arbitrarily
decides which one.  Which is not  what you wanted to say, surely.

> +(defcustom undelete-frame-max 1
> +  "Maximum number of deleted frames before oldest are thrown away."

This doesn't say how to disable the feature.

>      (bindings--define-key menu [undelete-last-deleted-frame]
>        '(menu-item "Undelete Frame" undelete-frame
> -                  :enable (and undelete-frame-mode
> -                                (car undelete-frame--deleted-frames))
> +                  :visible (car undelete-frame--deleted-frames)

I think Gregory didn't like the disappearing item, and preferred it to
be visible at all times, even if disabled.

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

[Juri Linkov]

>> So all concerns raised by Eli are addressed now
>> that make possible to enable it by default
>> as was discussed on emacs-devel:
>
> Once again, please do NOT enable this by default, I didn't agree to
> that.  This feature should remain disabled by default for some time,
> until we have enough information to discuss whether it should be
> turned on by default.  As discussed and agreed during its development.

But after the recent discussion it was agreed to enable it by default
after fixing raised concerns.  Everything is fixed now.
Should we restart the same discussion again?

>>  @item C-x 5 u
>>  @kindex C-x 5 u
>>  @findex undelete-frame
>> -@findex undelete-frame-mode
>> -When @code{undelete-frame-mode} is enabled, undelete one of the 16
>> -most recently deleted frames.  Without a prefix argument, undelete the
>> -most recently deleted frame.  With a numerical prefix argument between
>> -1 and 16, where 1 is the most recently deleted frame, undelete the
>> -corresponding deleted frame.
>> +@findex undelete-frame-max
>> +Undelete one of the recently deleted frames.  The user option
>
> "One of the recently deleted frames" sounds like Emacs arbitrarily
> decides which one.  Which is not  what you wanted to say, surely.

This was copied from old text: "undelete one of the 16 most recently deleted frames".

>> +(defcustom undelete-frame-max 1
>> +  "Maximum number of deleted frames before oldest are thrown away."
>
> This doesn't say how to disable the feature.

This feature can be disabled by customizing it to 0.

>>      (bindings--define-key menu [undelete-last-deleted-frame]
>>        '(menu-item "Undelete Frame" undelete-frame
>> -                  :enable (and undelete-frame-mode
>> -                                (car undelete-frame--deleted-frames))
>> +                  :visible (car undelete-frame--deleted-frames)
>
> I think Gregory didn't like the disappearing item, and preferred it to
> be visible at all times, even if disabled.

The invisible item makes the already overly long menu shorter,
especially for users who don't use frames.

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>   gregory@heytings.org,  51883@debbugs.gnu.org
> Date: Sun, 30 Jan 2022 19:17:30 +0200
> 
> >> So all concerns raised by Eli are addressed now
> >> that make possible to enable it by default
> >> as was discussed on emacs-devel:
> >
> > Once again, please do NOT enable this by default, I didn't agree to
> > that.  This feature should remain disabled by default for some time,
> > until we have enough information to discuss whether it should be
> > turned on by default.  As discussed and agreed during its development.
> 
> But after the recent discussion it was agreed to enable it by default
> after fixing raised concerns.  Everything is fixed now.
> Should we restart the same discussion again?

I see no need to restart that discussion, since we won't hear anything
new in it.  We made a decision back then, and there's no burning
reason to change that decision, except that some people would like to
have that feature turned on -- they can tun it on in their
customizations.

We discuss stuff and make decisions, and we should stick to our
decisions unless they turn out to be grave mistakes, which this one
isn't.

> >>  @item C-x 5 u
> >>  @kindex C-x 5 u
> >>  @findex undelete-frame
> >> -@findex undelete-frame-mode
> >> -When @code{undelete-frame-mode} is enabled, undelete one of the 16
> >> -most recently deleted frames.  Without a prefix argument, undelete the
> >> -most recently deleted frame.  With a numerical prefix argument between
> >> -1 and 16, where 1 is the most recently deleted frame, undelete the
> >> -corresponding deleted frame.
> >> +@findex undelete-frame-max
> >> +Undelete one of the recently deleted frames.  The user option
> >
> > "One of the recently deleted frames" sounds like Emacs arbitrarily
> > decides which one.  Which is not  what you wanted to say, surely.
> 
> This was copied from old text: "undelete one of the 16 most recently deleted frames".

Does that mean we must blindly copy it, even if it is confusing?

> >> +(defcustom undelete-frame-max 1
> >> +  "Maximum number of deleted frames before oldest are thrown away."
> >
> > This doesn't say how to disable the feature.
> 
> This feature can be disabled by customizing it to 0.

I know.  But the doc string keeps silent on that.

> >>      (bindings--define-key menu [undelete-last-deleted-frame]
> >>        '(menu-item "Undelete Frame" undelete-frame
> >> -                  :enable (and undelete-frame-mode
> >> -                                (car undelete-frame--deleted-frames))
> >> +                  :visible (car undelete-frame--deleted-frames)
> >
> > I think Gregory didn't like the disappearing item, and preferred it to
> > be visible at all times, even if disabled.
> 
> The invisible item makes the already overly long menu shorter,
> especially for users who don't use frames.

So you basically say we should disregard Gregory's opinion and prefer
yours?

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

[Juri Linkov]

>> >> So all concerns raised by Eli are addressed now
>> >> that make possible to enable it by default
>> >> as was discussed on emacs-devel:
>> >
>> > Once again, please do NOT enable this by default, I didn't agree to
>> > that.  This feature should remain disabled by default for some time,
>> > until we have enough information to discuss whether it should be
>> > turned on by default.  As discussed and agreed during its development.
>>
>> But after the recent discussion it was agreed to enable it by default
>> after fixing raised concerns.  Everything is fixed now.
>> Should we restart the same discussion again?
>
> I see no need to restart that discussion, since we won't hear anything
> new in it.  We made a decision back then, and there's no burning
> reason to change that decision, except that some people would like to
> have that feature turned on -- they can tun it on in their
> customizations.
>
> We discuss stuff and make decisions, and we should stick to our
> decisions unless they turn out to be grave mistakes, which this one
> isn't.

Unfortunately, this is a grave mistake, so it needs more discussion
on emacs-devel.

>> >>  @item C-x 5 u
>> >>  @kindex C-x 5 u
>> >>  @findex undelete-frame
>> >> -@findex undelete-frame-mode
>> >> -When @code{undelete-frame-mode} is enabled, undelete one of the 16
>> >> -most recently deleted frames.  Without a prefix argument, undelete the
>> >> -most recently deleted frame.  With a numerical prefix argument between
>> >> -1 and 16, where 1 is the most recently deleted frame, undelete the
>> >> -corresponding deleted frame.
>> >> +@findex undelete-frame-max
>> >> +Undelete one of the recently deleted frames.  The user option
>> >
>> > "One of the recently deleted frames" sounds like Emacs arbitrarily
>> > decides which one.  Which is not  what you wanted to say, surely.
>>
>> This was copied from old text: "undelete one of the 16 most recently deleted frames".
>
> Does that mean we must blindly copy it, even if it is confusing?

I will fix this, once the patch is accepted.

>> >> +(defcustom undelete-frame-max 1
>> >> +  "Maximum number of deleted frames before oldest are thrown away."
>> >
>> > This doesn't say how to disable the feature.
>>
>> This feature can be disabled by customizing it to 0.
>
> I know.  But the doc string keeps silent on that.

Will fix this as well.

>> >>      (bindings--define-key menu [undelete-last-deleted-frame]
>> >>        '(menu-item "Undelete Frame" undelete-frame
>> >> -                  :enable (and undelete-frame-mode
>> >> -                                (car undelete-frame--deleted-frames))
>> >> +                  :visible (car undelete-frame--deleted-frames)
>> >
>> > I think Gregory didn't like the disappearing item, and preferred it to
>> > be visible at all times, even if disabled.
>>
>> The invisible item makes the already overly long menu shorter,
>> especially for users who don't use frames.
>
> So you basically say we should disregard Gregory's opinion and prefer
> yours?

This definitely needs more discussion, so I'll post the problem's description
on emacs-devel.

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

[Juri Linkov]

> +*** With a prefix argument, the key 'C-x 5 2' undeletes deleted frames.

Recently we had a dispute whether the prefix argument of 'C-x 5 2'
should be used to clone the frame, and now you propose another
meaning of its argument :)

But to avoid conflicts, like 'clone-frame' is bound to 'C-x 5 c',
'undelete-frame' could be bound to 'C-x 5 u'.

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

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

The command sounds useful, but I worry about one possible problem:
how much garbage will the to-be-restored frame hold onto?
Please investigate this and see whether it is a significant issue or not.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

[Gregory Heytings]

>
> The command sounds useful, but I worry about one possible problem: how 
> much garbage will the to-be-restored frame hold onto? Please investigate 
> this and see whether it is a significant issue or not.
>

It's not significant.  The frames are not kept in memory for later 
possible reuse, only a description of their state (size, windows, which 
buffer is displayed in which window) is kept in memory, which occupies a 
few kilobytes for each deleted frame.  That's much less than the memory 
used by undo, for example.  Moreover that state would not be kept for all 
deleted frames, but only for a the most recently deleted ones.

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

[Drew Adams]

> The command sounds useful, but I worry about one possible problem:
> how much garbage will the to-be-restored frame hold onto?
> Please investigate this and see whether it is a significant issue or not.

That was my first thought too.  Some people use
few frames over an Emacs session.  Some others
use many.

At least, IIUC, a user can set the ring size,
to control how many deleted frames to save.

That should be sufficient (including the
possibility of setting the size to zero).