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.