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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 16 Nov 2021 19:40:08 +0200

> Date: Tue, 16 Nov 2021 17:05:07 +0000
> From: Gregory Heytings <[email-masked]>
> Cc: Michael Heerdegen <[email-masked]>
> 
> 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.