all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Undo mode
@ 2022-01-20 20:45 Juri Linkov
  2022-01-21  0:42 ` Stefan Monnier
                   ` (4 more replies)
  0 siblings, 5 replies; 62+ messages in thread
From: Juri Linkov @ 2022-01-20 20:45 UTC (permalink / raw)
  To: emacs-devel

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

It would be more convenient to have a special mode `undo-mode'
that is disabled by default.

To enable undo-mode, it will possible to select from the main menu:

  Edit >
   Enable Undo Mode

Then after enabling it, more menu items will appear on the menu:

  Edit >
   Disable Undo Mode
   Undo
   Redo

Also the current number of undo-limit will be hard-coded
for more convenience:

diff --git a/src/undo.c b/src/undo.c
index 5d705945c4..510d6e8692 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -386,7 +386,7 @@ truncate_undo_list (struct buffer *b)
 	  if (size_so_far > undo_strong_limit)
 	    break;
 	  last_boundary = prev;
-	  if (size_so_far > undo_limit)
+	  if (size_so_far > 160000)
 	    break;
 	}

Does this make sense?

If not, wouldn't be more preferable the following patch
that does the opposite?


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

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

^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-20 20:45 Undo mode Juri Linkov
@ 2022-01-21  0:42 ` Stefan Monnier
  2022-01-21  8:16   ` Juri Linkov
  2022-01-21  1:57 ` Po Lu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 62+ messages in thread
From: Stefan Monnier @ 2022-01-21  0:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

> Does this make sense?

Sounds like Antinews alright ;-)

> If not, wouldn't be more preferable the following patch
> that does the opposite?

I generally agree, yes.

> +(defcustom undelete-frame-max 1
> +  "Maximum number of deleted frames before oldest are thrown away."
> +  :type 'integer
> +  :group 'frames
> +  :version "29.1")

Any reason why this default is so low?
Is it really that expensive to keep old frames's info?

>  (defun undelete-frame--handle-delete-frame (frame)

Could you find a better name for that function?
I mean a name that describes what the function does, rather than
where/when it's used?

> +(add-hook 'after-init-hook
> +          (lambda ()
> +            (add-hook 'delete-frame-functions
> +                      #'undelete-frame--handle-delete-frame -75)))

Why do we need to postpone it via `after-init-hook`?


        Stefan




^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-20 20:45 Undo mode Juri Linkov
  2022-01-21  0:42 ` Stefan Monnier
@ 2022-01-21  1:57 ` Po Lu
  2022-01-21  3:11   ` [External] : " Drew Adams
                     ` (2 more replies)
  2022-01-21  7:17 ` Eli Zaretskii
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 62+ messages in thread
From: Po Lu @ 2022-01-21  1:57 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

Juri Linkov <juri@linkov.net> writes:

> If not, wouldn't be more preferable the following patch
> that does the opposite?

IMO, consistency doesn't matter at all when the ship has already sailed.
`undelete-frame-mode' has been here for nearly a week, so removing it
should be out of the question.

Introducing an option to allow customizing the amount of frames that are
kept is okay, however.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [External] : Re: Undo mode
  2022-01-21  1:57 ` Po Lu
@ 2022-01-21  3:11   ` Drew Adams
  2022-01-21  3:40     ` Po Lu
  2022-01-21 12:50     ` [External] : Re: Undo mode Stefan Monnier
  2022-01-21  8:18   ` Juri Linkov
  2022-01-21  8:56   ` Lars Ingebrigtsen
  2 siblings, 2 replies; 62+ messages in thread
From: Drew Adams @ 2022-01-21  3:11 UTC (permalink / raw)
  To: Po Lu, Juri Linkov; +Cc: emacs-devel@gnu.org

> IMO, consistency doesn't matter at all when the ship has already sailed.
> `undelete-frame-mode' has been here for nearly a week, so removing it
> should be out of the question.

I'm not saying anything about this feature or any
ship on which someone thinks something has already
sailed.

But if something isn't in an Emacs release (the
latest is 27.2) then I'd say it hasn't sailed.

Something that someone threw into master or
whatever "nearly a week" ago hasn't even been
loaded onto a ship yet, IMHO.

It might be, but it isn't necessarily, sitting
on some dock somewhere.

And yes, it should be possible to remove it
from any dock or any ship it's sitting on.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [External] : Re: Undo mode
  2022-01-21  3:11   ` [External] : " Drew Adams
@ 2022-01-21  3:40     ` Po Lu
  2022-01-21  5:35       ` Drew Adams
  2022-01-21  7:56       ` Tracking master (was: [External] : Re: Undo mode) Kévin Le Gouguec
  2022-01-21 12:50     ` [External] : Re: Undo mode Stefan Monnier
  1 sibling, 2 replies; 62+ messages in thread
From: Po Lu @ 2022-01-21  3:40 UTC (permalink / raw)
  To: Drew Adams; +Cc: Juri Linkov, emacs-devel@gnu.org

Drew Adams <drew.adams@oracle.com> writes:

> I'm not saying anything about this feature or any
> ship on which someone thinks something has already
> sailed.
>
> But if something isn't in an Emacs release (the
> latest is 27.2) then I'd say it hasn't sailed.

What if someone tracking master (and there are enough of them to make a
difference, I think) has customized `undelete-frame-mode' to t?

That's the specific case I was thinking of.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [External] : Re: Undo mode
  2022-01-21  3:40     ` Po Lu
@ 2022-01-21  5:35       ` Drew Adams
  2022-01-21  7:56       ` Tracking master (was: [External] : Re: Undo mode) Kévin Le Gouguec
  1 sibling, 0 replies; 62+ messages in thread
From: Drew Adams @ 2022-01-21  5:35 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel@gnu.org, Juri Linkov

> > I'm not saying anything about this feature or any
> > ship on which someone thinks something has already
> > sailed.
> >
> > But if something isn't in an Emacs release (the
> > latest is 27.2) then I'd say it hasn't sailed.
> 
> What if someone tracking master (and there are enough of them to make a
> difference, I think) has customized `undelete-frame-mode' to t?
> 
> That's the specific case I was thinking of.

Bleeding edge.  Not a sailing ship.

(And again, I'm not saying anything
about this feature.)



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-20 20:45 Undo mode Juri Linkov
  2022-01-21  0:42 ` Stefan Monnier
  2022-01-21  1:57 ` Po Lu
@ 2022-01-21  7:17 ` Eli Zaretskii
  2022-01-21 10:09   ` Stefan Kangas
  2022-01-21  8:41 ` Gregory Heytings
  2022-01-21  8:55 ` Lars Ingebrigtsen
  4 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21  7:17 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 20 Jan 2022 22:45:00 +0200
> 
> If not, wouldn't be more preferable the following patch
> that does the opposite?

We are not going to enable undelete-frame-mode by default.  People who
do want it enabled by default can do so in their customizations, so I
see no problem here that we need to solve.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Tracking master (was: [External] : Re: Undo mode)
  2022-01-21  3:40     ` Po Lu
  2022-01-21  5:35       ` Drew Adams
@ 2022-01-21  7:56       ` Kévin Le Gouguec
  1 sibling, 0 replies; 62+ messages in thread
From: Kévin Le Gouguec @ 2022-01-21  7:56 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel@gnu.org, Drew Adams, Juri Linkov

Po Lu <luangruo@yahoo.com> writes:

> Drew Adams <drew.adams@oracle.com> writes:
>
>> I'm not saying anything about this feature or any
>> ship on which someone thinks something has already
>> sailed.
>>
>> But if something isn't in an Emacs release (the
>> latest is 27.2) then I'd say it hasn't sailed.
>
> What if someone tracking master (and there are enough of them to make a
> difference, I think) has customized `undelete-frame-mode' to t?
>
> That's the specific case I was thinking of.

As someone using master as my daily driver for years: I have zero
expectation that features introduced on this development branch will
still be around next week, in their current shape, or in any shape at
all.

My only hope (and still, not an expectation) is that good features will
be discussed and reworked into their best possible iteration by the time
a release is cut.

(Luckily, since collectively, Emacs hackers are as considerate as they
are innovative, I can generally start sitting on newly-installed rugs
without worrying that they'll be pulled from under me.  As a
bleeding-edge user, this brings me a level of comfort that is well worth
the occasional .emacs adjusment)

While patches can and are discussed on the bugtracker before being
applied to master, and feature branches can and are discussed before
being merged, they don't get nearly as much exposure as they do once
they land on master.  For that reason alone, I think it'd be a waste to
miss the chance of using master to gather further feedback and iterate
more before committing to the maintenance burden.

(Although take all that with a grain of salt: I can't remember if the
ship departure schedule has been formally spelled out.  My expectations
(or lack thereof) regarding master might not reflect the maintainers'
policy)



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  0:42 ` Stefan Monnier
@ 2022-01-21  8:16   ` Juri Linkov
  2022-01-21 11:44     ` Eli Zaretskii
  2022-01-21 13:50     ` Stefan Monnier
  0 siblings, 2 replies; 62+ messages in thread
From: Juri Linkov @ 2022-01-21  8:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>> Does this make sense?
>
> Sounds like Antinews alright ;-)
>
>> If not, wouldn't be more preferable the following patch
>> that does the opposite?
>
> I generally agree, yes.
>
>> +(defcustom undelete-frame-max 1
>> +  "Maximum number of deleted frames before oldest are thrown away."
>> +  :type 'integer
>> +  :group 'frames
>> +  :version "29.1")
>
> Any reason why this default is so low?
> Is it really that expensive to keep old frames's info?

If the memory consumption is not a problem, it could be increased.
But 1 is the minimum value that it useful for all users:
when the user accidentally deletes a frame, it will be possible
to undelete it immediately, without enabling some special mode.

>>  (defun undelete-frame--handle-delete-frame (frame)
>
> Could you find a better name for that function?
> I mean a name that describes what the function does, rather than
> where/when it's used?

Maybe `undelete-frame--save-deleted-frame', I will change this
in the next version of the patch.

>> +(add-hook 'after-init-hook
>> +          (lambda ()
>> +            (add-hook 'delete-frame-functions
>> +                      #'undelete-frame--handle-delete-frame -75)))
>
> Why do we need to postpone it via `after-init-hook`?

The problem is that during Emacs GUI startup, some temporary TTY frame
is created and deleted.  Maybe this is a bug?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  1:57 ` Po Lu
  2022-01-21  3:11   ` [External] : " Drew Adams
@ 2022-01-21  8:18   ` Juri Linkov
  2022-01-21  8:56   ` Lars Ingebrigtsen
  2 siblings, 0 replies; 62+ messages in thread
From: Juri Linkov @ 2022-01-21  8:18 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> `undelete-frame-mode' has been here for nearly a week, so removing it
> should be out of the question.

If the entire history of Emacs was compressed into a single 24-hour day,
then a week will last only 30 seconds!



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-20 20:45 Undo mode Juri Linkov
                   ` (2 preceding siblings ...)
  2022-01-21  7:17 ` Eli Zaretskii
@ 2022-01-21  8:41 ` Gregory Heytings
  2022-01-21  9:40   ` Po Lu
                     ` (3 more replies)
  2022-01-21  8:55 ` Lars Ingebrigtsen
  4 siblings, 4 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21  8:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel


It is deeply saddening to see that the "Undelete Frame" feature has become 
the subject of a quarrel.

The original design added a single "Undelete Frame" item in the File menu, 
under the "Delete Frame" item.  The conclusion of the discussion in 
bug#51883 was that this feature shouldn't be enabled by default, which 
means that this "Undelete Frame" menu item would have remained inactive, 
unlike other items in the menus whose state change from inactive to active 
when their actions become feasible.

Users could have reasonably expected to see this menu item become active 
after they used the "Delete Frame" menu item, just like the "Remove Other 
Windows" item becomes active after using one of the "New Window" items.

Therefore, to help users discover that this menu item required an explicit 
activation in their init files to become usable, I added a "Enable Frame 
Undeletion" item in the File menu, with a tooltip which said "Enable frame 
undeletion for this session", and which replaced the "Undelete Frame" item 
when undelete-frame-mode was not active.  IOW, this feature always used 
one and only one item in the File menu: either "Enable Frame Undeletion", 
or "Undelete Frame".

The hard-coded limit of 16 deleted frames was also a conscious design 
choice, to avoid adding yet another defcustom that nobody had asked (yet). 
A user might want to undelete a few frames, so recording a single deleted 
frame is not enough, and 16 seemed like a reasonable upper limit that 
should cover all needs.

I respectfully suggest to go back to that original design.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-20 20:45 Undo mode Juri Linkov
                   ` (3 preceding siblings ...)
  2022-01-21  8:41 ` Gregory Heytings
@ 2022-01-21  8:55 ` Lars Ingebrigtsen
  4 siblings, 0 replies; 62+ messages in thread
From: Lars Ingebrigtsen @ 2022-01-21  8:55 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

Juri Linkov <juri@linkov.net> writes:

> If not, wouldn't be more preferable the following patch
> that does the opposite?

Yes, I'd prefer your patch.

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



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  1:57 ` Po Lu
  2022-01-21  3:11   ` [External] : " Drew Adams
  2022-01-21  8:18   ` Juri Linkov
@ 2022-01-21  8:56   ` Lars Ingebrigtsen
  2022-01-21  9:15     ` Po Lu
  2022-01-21 11:41     ` Eli Zaretskii
  2 siblings, 2 replies; 62+ messages in thread
From: Lars Ingebrigtsen @ 2022-01-21  8:56 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Juri Linkov

Po Lu <luangruo@yahoo.com> writes:

> IMO, consistency doesn't matter at all when the ship has already sailed.
> `undelete-frame-mode' has been here for nearly a week, so removing it
> should be out of the question.

It's not -- we do development on the master branch, and we add and
remove things at will.  And certainly if it's been on the branch for
such a short period as a week.  `undelete-frame-mode' makes no sense to
me.

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



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:56   ` Lars Ingebrigtsen
@ 2022-01-21  9:15     ` Po Lu
  2022-01-21  9:17       ` Lars Ingebrigtsen
  2022-01-21 11:41     ` Eli Zaretskii
  1 sibling, 1 reply; 62+ messages in thread
From: Po Lu @ 2022-01-21  9:15 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel, Juri Linkov

Lars Ingebrigtsen <larsi@gnus.org> writes:

> It's not -- we do development on the master branch, and we add and
> remove things at will.  And certainly if it's been on the branch for
> such a short period as a week.

Duly noted, but I think the period between when releases are made is
still long enough to warrant some guarantee of stability.  Maybe a month
or so would make more sense?

> `undelete-frame-mode' makes no sense to me.

I thought there was a discussion around its implementation that led to
the creation of `undelete-frame-mode'.

I did not follow it, so I don't really have an opinion as to whether or
not it's a good idea, but it seems that a decision was made there with
input from many people that we shouldn't reject only a week later as a
bad idea.

Thanks.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  9:15     ` Po Lu
@ 2022-01-21  9:17       ` Lars Ingebrigtsen
  2022-01-21 12:52         ` Stefan Monnier
  0 siblings, 1 reply; 62+ messages in thread
From: Lars Ingebrigtsen @ 2022-01-21  9:17 UTC (permalink / raw)
  To: Po Lu; +Cc: Juri Linkov, emacs-devel

Po Lu <luangruo@yahoo.com> writes:

> Duly noted, but I think the period between when releases are made is
> still long enough to warrant some guarantee of stability.  Maybe a month
> or so would make more sense?

I don't think we need to have any specified rules in this area.  Of
course we don't want to be needlessly disruptive for the people that
follow the master branch, so nothing is changed wilfully, but we're not
ruling out changing anything on that branch -- it's up to the maintainers.

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



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:41 ` Gregory Heytings
@ 2022-01-21  9:40   ` Po Lu
  2022-01-21  9:57     ` Gregory Heytings
  2022-01-21 11:48   ` Eli Zaretskii
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 62+ messages in thread
From: Po Lu @ 2022-01-21  9:40 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Juri Linkov, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> The original design added a single "Undelete Frame" item in the File
> menu, under the "Delete Frame" item.  The conclusion of the discussion
> in bug#51883 was that this feature shouldn't be enabled by default,
> which means that this "Undelete Frame" menu item would have remained
> inactive, unlike other items in the menus whose state change from
> inactive to active when their actions become feasible.
>
> Users could have reasonably expected to see this menu item become
> active after they used the "Delete Frame" menu item, just like the
> "Remove Other Windows" item becomes active after using one of the "New
> Window" items.
>
> Therefore, to help users discover that this menu item required an
> explicit activation in their init files to become usable, I added a
> "Enable Frame Undeletion" item in the File menu, with a tooltip which
> said "Enable frame undeletion for this session", and which replaced
> the "Undelete Frame" item when undelete-frame-mode was not active.
> IOW, this feature always used one and only one item in the File menu:
> either "Enable Frame Undeletion", or "Undelete Frame".
>
> The hard-coded limit of 16 deleted frames was also a conscious design
> choice, to avoid adding yet another defcustom that nobody had asked
> (yet). A user might want to undelete a few frames, so recording a
> single deleted frame is not enough, and 16 seemed like a reasonable
> upper limit that should cover all needs.
>
> I respectfully suggest to go back to that original design.

We haven't yet decided to change the design (aside from me turning the
"Enable Frame Undeletion" menu item into a toggle, so someone can figure
out how to turn it off should he want to.)

At least, I think we haven't.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  9:40   ` Po Lu
@ 2022-01-21  9:57     ` Gregory Heytings
  2022-01-21 10:19       ` Po Lu
  2022-01-21 11:58       ` Eli Zaretskii
  0 siblings, 2 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21  9:57 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Juri Linkov

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


>> I respectfully suggest to go back to that original design.
>
> We haven't yet decided to change the design
>

Going back to the original design is not changing the design, it's 
restoring the menu item as it was originally meant to be.  See attached 
patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix-Undelete-Frame-menu-item.patch --]
[-- Type: text/x-diff; name=Fix-Undelete-Frame-menu-item.patch, Size: 1732 bytes --]

From 9b025659fdbca5a0fe9ae38a05dbc13ec05a1c8a Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 21 Jan 2022 09:47:02 +0000
Subject: [PATCH] Fix Undelete Frame menu item

* lisp/menu-bar.el (menu-bar-file-menu): Re-add the "Enable Frame Undeletion"
menu item, and restore the "Undelete Frame" menu item.
---
 lisp/menu-bar.el | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d1ca16dbf6..3a630912a9 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,16 +108,15 @@ 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
-                  :visible (and undelete-frame-mode
-                                (car undelete-frame--deleted-frames))
+                  :visible undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
+    (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"))
 
     ;; Don't use delete-frame as event name because that is a special
     ;; event.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  7:17 ` Eli Zaretskii
@ 2022-01-21 10:09   ` Stefan Kangas
  2022-01-21 10:19     ` Gregory Heytings
  2022-01-21 11:56     ` Eli Zaretskii
  0 siblings, 2 replies; 62+ messages in thread
From: Stefan Kangas @ 2022-01-21 10:09 UTC (permalink / raw)
  To: Eli Zaretskii, Juri Linkov; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> We are not going to enable undelete-frame-mode by default.

I don't think I understand the benefit of hiding this behind a mode.
It seems to me that the major difference it will make is that the
`undelete-frame' command won't work by default.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  9:57     ` Gregory Heytings
@ 2022-01-21 10:19       ` Po Lu
  2022-01-21 10:26         ` Gregory Heytings
  2022-01-21 11:58       ` Eli Zaretskii
  1 sibling, 1 reply; 62+ messages in thread
From: Po Lu @ 2022-01-21 10:19 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Juri Linkov, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> Going back to the original design is not changing the design, it's
> restoring the menu item as it was originally meant to be.  See
> attached patch.

Thanks, but I don't see how that's any different, aside from not telling
the user how to turn it off.

Either way, this change was discussed in bug#53382, so I suggest to look
there.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:09   ` Stefan Kangas
@ 2022-01-21 10:19     ` Gregory Heytings
  2022-01-21 16:45       ` [External] : " Drew Adams
  2022-01-21 11:56     ` Eli Zaretskii
  1 sibling, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 10:19 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov


>> We are not going to enable undelete-frame-mode by default.
>
> I don't think I understand the benefit of hiding this behind a mode. It 
> seems to me that the major difference it will make is that the 
> `undelete-frame' command won't work by default.
>

See bug#51883.  The conclusion was that undeleting frames is not a feature 
that all users need, and that enabling it by default uses some memory.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:19       ` Po Lu
@ 2022-01-21 10:26         ` Gregory Heytings
  2022-01-21 10:34           ` Po Lu
  2022-01-21 12:10           ` Eli Zaretskii
  0 siblings, 2 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 10:26 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Juri Linkov


>> Going back to the original design is not changing the design, it's 
>> restoring the menu item as it was originally meant to be.  See attached 
>> patch.
>
> Thanks, but I don't see how that's any different, aside from not telling 
> the user how to turn it off.
>
> Either way, this change was discussed in bug#53382, so I suggest to look 
> there.
>

Discussed?  You filed a bug report, Eli replied "I don't see that as a 
problem", you did what you wanted to do two hours later, and closed the 
bug report.

There's no point in telling the user "how to turn it off", as I said 
earlier the "Enable Frame Undeletion" menu item was there as a hint to 
tell users that it's a feature that needs to be turned on in their init 
files.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:26         ` Gregory Heytings
@ 2022-01-21 10:34           ` Po Lu
  2022-01-21 10:45             ` Gregory Heytings
  2022-01-21 12:10           ` Eli Zaretskii
  1 sibling, 1 reply; 62+ messages in thread
From: Po Lu @ 2022-01-21 10:34 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Juri Linkov, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> Discussed?  You filed a bug report, Eli replied "I don't see that as a
> problem", you did what you wanted to do two hours later, and closed
> the bug report.

Eli also added an option first to disable the mode, and I turned it into
a toggle.  And yes, that counts as a discussion, since it involved more
than one person.

> There's no point in telling the user "how to turn it off", as I said
> earlier the "Enable Frame Undeletion" menu item was there as a hint to
> tell users that it's a feature that needs to be turned on in their
> init files.

Let's say a hypothetical user with limited memory turns frame undeletion
on, realizes that it uses too much memory to be useful to him, and then
wants to turn it off.

By the time he realizes he wants it off, the output of view-lossage is
no longer likely to be helpful to determine exactly how the feature was
enabled, so naturally he will look in the place where he turned it on,
which is the File menu, and get very confused if all that is there is
"Undelete Frame", and not "Disable Frame Undeletion" or some variant
thereof.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:34           ` Po Lu
@ 2022-01-21 10:45             ` Gregory Heytings
  2022-01-21 10:51               ` Po Lu
  0 siblings, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 10:45 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel, Juri Linkov


>> There's no point in telling the user "how to turn it off", as I said 
>> earlier the "Enable Frame Undeletion" menu item was there as a hint to 
>> tell users that it's a feature that needs to be turned on in their init 
>> files.
>
> Let's say a hypothetical user with limited memory turns frame undeletion 
> on, realizes that it uses too much memory to be useful to him, and then 
> wants to turn it off.
>

I suggest to wait before such an (unlikely) hypothetical situation happens 
before concluding that something which disfigures the File menu must be 
done.

And your hypothetical user can always do M-x undelete-frame TAB to see 
that there is an undelete-frame-mode.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:45             ` Gregory Heytings
@ 2022-01-21 10:51               ` Po Lu
  0 siblings, 0 replies; 62+ messages in thread
From: Po Lu @ 2022-01-21 10:51 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: emacs-devel, Juri Linkov

Gregory Heytings <gregory@heytings.org> writes:

> I suggest to wait before such an (unlikely) hypothetical situation
> happens before concluding that something which disfigures the File
> menu must be done.

While the scenario I presented is hypothetical, the problem itself is
not.  I spent a minute searching for a way to turn frame undeletion off
after enabling it, to debug a input focus related problem on X.

It turned out to be a bug in either GNOME Shell or XWayland, but at
first it only manifested after I turned on frame undeletion.

> And your hypothetical user can always do M-x undelete-frame TAB to see
> that there is an undelete-frame-mode.

What if he only knows to use `undelete-frame' from the menu bar?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:56   ` Lars Ingebrigtsen
  2022-01-21  9:15     ` Po Lu
@ 2022-01-21 11:41     ` Eli Zaretskii
  1 sibling, 0 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 11:41 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: luangruo, juri, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 21 Jan 2022 09:56:50 +0100
> Cc: emacs-devel@gnu.org, Juri Linkov <juri@linkov.net>
> 
> `undelete-frame-mode' makes no sense to me.

It does to me, and it was discussed at length as part of bug#51883.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:16   ` Juri Linkov
@ 2022-01-21 11:44     ` Eli Zaretskii
  2022-01-21 13:50     ` Stefan Monnier
  1 sibling, 0 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 11:44 UTC (permalink / raw)
  To: Juri Linkov; +Cc: monnier, emacs-devel

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 21 Jan 2022 10:16:45 +0200
> Cc: emacs-devel@gnu.org
> 
> > Why do we need to postpone it via `after-init-hook`?
> 
> The problem is that during Emacs GUI startup, some temporary TTY frame
> is created and deleted.  Maybe this is a bug?

No, this is normal.  temacs uses a surrogate TTY frame, and that frame
is dumped when the Emacs executable is built.  It is also used by the
daemon.




^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:41 ` Gregory Heytings
  2022-01-21  9:40   ` Po Lu
@ 2022-01-21 11:48   ` Eli Zaretskii
  2022-01-21 13:18   ` Yuri Khan
  2022-01-22 18:04   ` Juri Linkov
  3 siblings, 0 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 11:48 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: emacs-devel, juri

> Date: Fri, 21 Jan 2022 08:41:42 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: emacs-devel@gnu.org
> 
> I respectfully suggest to go back to that original design.

Which part(s) of the original design changed in ways that you disagree
with?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:09   ` Stefan Kangas
  2022-01-21 10:19     ` Gregory Heytings
@ 2022-01-21 11:56     ` Eli Zaretskii
  2022-01-22  9:51       ` Stefan Kangas
  1 sibling, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 11:56 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel, juri

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 21 Jan 2022 02:09:41 -0800
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > We are not going to enable undelete-frame-mode by default.
> 
> I don't think I understand the benefit of hiding this behind a mode.

That was the conclusion of our discussion of the original issue.
Please re-read it if you want to understand the reasons.

> It seems to me that the major difference it will make is that the
> `undelete-frame' command won't work by default.

That's right, we concluded that it would be best to have that disabled
by default.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  9:57     ` Gregory Heytings
  2022-01-21 10:19       ` Po Lu
@ 2022-01-21 11:58       ` Eli Zaretskii
  2022-01-21 13:50         ` Gregory Heytings
  1 sibling, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 11:58 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, juri, emacs-devel

> Date: Fri, 21 Jan 2022 09:57:18 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: emacs-devel@gnu.org, Juri Linkov <juri@linkov.net>
> 
> Going back to the original design is not changing the design, it's 
> restoring the menu item as it was originally meant to be.  See attached 
> patch.

Why is it a problem to allow disabling undeletion of frames once that
was enabled?


Or if that is not the problem you are trying to fix, what is that
problem?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 10:26         ` Gregory Heytings
  2022-01-21 10:34           ` Po Lu
@ 2022-01-21 12:10           ` Eli Zaretskii
  1 sibling, 0 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 12:10 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, juri, emacs-devel

> Date: Fri, 21 Jan 2022 10:26:54 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: emacs-devel@gnu.org, Juri Linkov <juri@linkov.net>
> 
> There's no point in telling the user "how to turn it off", as I said 
> earlier the "Enable Frame Undeletion" menu item was there as a hint to 
> tell users that it's a feature that needs to be turned on in their init 
> files.

It is unusual to have a setting that can be turned on via the menu,
but not turned off via the same menu.  The mode can be turned off via
M-x, so what would be our reason not to allow the same from the menu
bar?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [External] : Re: Undo mode
  2022-01-21  3:11   ` [External] : " Drew Adams
  2022-01-21  3:40     ` Po Lu
@ 2022-01-21 12:50     ` Stefan Monnier
  2022-01-21 16:45       ` Drew Adams
  1 sibling, 1 reply; 62+ messages in thread
From: Stefan Monnier @ 2022-01-21 12:50 UTC (permalink / raw)
  To: Drew Adams; +Cc: Po Lu, Juri Linkov, emacs-devel@gnu.org

> But if something isn't in an Emacs release (the
> latest is 27.2) then I'd say it hasn't sailed.

100% agreed.

> What if someone tracking master (and there are enough of them to make a
> difference, I think) has customized `undelete-frame-mode' to t?

What about that?


        Stefan




^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  9:17       ` Lars Ingebrigtsen
@ 2022-01-21 12:52         ` Stefan Monnier
  0 siblings, 0 replies; 62+ messages in thread
From: Stefan Monnier @ 2022-01-21 12:52 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Po Lu, Juri Linkov, emacs-devel

Lars Ingebrigtsen [2022-01-21 10:17:45] wrote:
> Po Lu <luangruo@yahoo.com> writes:
>> Duly noted, but I think the period between when releases are made is
>> still long enough to warrant some guarantee of stability.  Maybe a month
>> or so would make more sense?
> I don't think we need to have any specified rules in this area.

Especially since changes cost efforts, so there's a natural resistance
to change, without needing any extra rules to enforce it.


        Stefan




^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:41 ` Gregory Heytings
  2022-01-21  9:40   ` Po Lu
  2022-01-21 11:48   ` Eli Zaretskii
@ 2022-01-21 13:18   ` Yuri Khan
  2022-01-21 14:29     ` Eli Zaretskii
  2022-01-22 18:04   ` Juri Linkov
  3 siblings, 1 reply; 62+ messages in thread
From: Yuri Khan @ 2022-01-21 13:18 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Emacs developers, Juri Linkov

On Fri, 21 Jan 2022 at 16:38, Gregory Heytings <gregory@heytings.org> wrote:

> Therefore, to help users discover that this menu item required an explicit
> activation in their init files to become usable, I added a "Enable Frame
> Undeletion" item in the File menu, with a tooltip which said "Enable frame
> undeletion for this session"

Why would it be on the File menu rather than Options?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 11:58       ` Eli Zaretskii
@ 2022-01-21 13:50         ` Gregory Heytings
  2022-01-21 14:32           ` Eli Zaretskii
  0 siblings, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 13:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, emacs-devel, juri

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


>> Going back to the original design is not changing the design, it's 
>> restoring the menu item as it was originally meant to be.  See attached 
>> patch.
>
> Why is it a problem to allow disabling undeletion of frames once that 
> was enabled?
>

It's not a problem at all, but it's not what the "Enable Frame Undeletion" 
menu item was meant to do.  It was meant to tell users that it is 
necessary to add somethig their init files to enable that feature.

I attach three possible solutions, in order of personal preference:

1. The original design: the "Enable Frame Undeletion" item disappears once 
it has been used, and is replaced by the "Undelete Frame" item.  Users 
have to use M-x undelete-frame-mode to turn that feature off, and it is 
turned on only during the current session.

2. The only menu item is "Undelete Frame", but it has a "Undelete-Frame 
mode is disabled" tooltip when undelete-frame-mode is off, and a "Undelete 
the most recently deleted frame" tooltip when it is on.

3. The "Enable Frame Undeletion" item disappears, and a "Disable Frame 
Undeletion" item is added after the "Undelete Frame" item.  It uses one or 
two menu items, but a least it doesn't shift the whole File menu to the 
right because of a checkbox.

[-- Attachment #2: Fix-Undelete-Frame-menu-item-Option-1.patch --]
[-- Type: text/x-diff, Size: 1743 bytes --]

From ac56be4475b773e08f33f9d1e91eb8191865ebf8 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 21 Jan 2022 13:08:50 +0000
Subject: [PATCH] Fix Undelete Frame menu item (Option 1)

* lisp/menu-bar.el (menu-bar-file-menu): Re-add the "Enable Frame Undeletion"
menu item, and restore the "Undelete Frame" menu item.
---
 lisp/menu-bar.el | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d1ca16dbf6..3a630912a9 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,16 +108,15 @@ 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
-                  :visible (and undelete-frame-mode
-                                (car undelete-frame--deleted-frames))
+                  :visible undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
+    (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"))
 
     ;; Don't use delete-frame as event name because that is a special
     ;; event.
-- 
2.34.1


[-- Attachment #3: Fix-Undelete-Frame-menu-item-Option-2.patch --]
[-- Type: text/x-diff, Size: 1751 bytes --]

From 298fb61779e652016dda3fc4b4af2c0a23c0f5cc Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 21 Jan 2022 13:16:04 +0000
Subject: [PATCH] Fix Undelete Frame menu item (Option 2)

* lisp/menu-bar.el (menu-bar-file-menu): Restore the "Undelete Frame"
menu item, and add a disabled item with a tooltip explaining how to
enable it.
---
 lisp/menu-bar.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d1ca16dbf6..d2ed6dc3b7 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,15 +108,15 @@ 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 [disabled-undelete-frame]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :visible (null undelete-frame-mode)
+                  :enable nil
+                  :help "Undelete-Frame mode is disabled"))
     (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 undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
 
     ;; Don't use delete-frame as event name because that is a special
-- 
2.34.1


[-- Attachment #4: Fix-Undelete-Frame-menu-item-Option-3.patch --]
[-- Type: text/x-diff, Size: 2044 bytes --]

From 6b5f070ac30cef7c61418a399023d2258e642930 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 21 Jan 2022 13:20:16 +0000
Subject: [PATCH] Fix Undelete Frame menu item (Option 3)

* lisp/menu-bar.el (menu-bar-file-menu): Re-add the "Enable Frame Undeletion"
menu item, restore the "Undelete Frame" menu item, and add a "Disable Frame
Undeletion" menu item.
---
 lisp/menu-bar.el | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d1ca16dbf6..a7b6dfc995 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,16 +108,19 @@ 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 [disable-undelete-frame-mode]
+      '(menu-item "Disable Frame Undeletion" undelete-frame-mode
+                  :visible undelete-frame-mode
+                  :help "Disable frame undeletion for 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 undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
+    (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"))
 
     ;; Don't use delete-frame as event name because that is a special
     ;; event.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:16   ` Juri Linkov
  2022-01-21 11:44     ` Eli Zaretskii
@ 2022-01-21 13:50     ` Stefan Monnier
  2022-01-21 15:07       ` Gregory Heytings
  1 sibling, 1 reply; 62+ messages in thread
From: Stefan Monnier @ 2022-01-21 13:50 UTC (permalink / raw)
  To: Juri Linkov; +Cc: emacs-devel

>>> +(add-hook 'after-init-hook
>>> +          (lambda ()
>>> +            (add-hook 'delete-frame-functions
>>> +                      #'undelete-frame--handle-delete-frame -75)))
>> Why do we need to postpone it via `after-init-hook`?
> The problem is that during Emacs GUI startup, some temporary TTY frame
> is created and deleted.  Maybe this is a bug?

I think you're talking about the initial frame (which is actually not
a TTY frame either, it's the pseudo frame used for example during batch
processing, which sends messages to stdout/stderr and reads "minibuffer"
input from stdin).

It's kind of a pain to recognize in ELisp.  In C it's recognized by:

    #define FRAME_INITIAL_P(f) ((f)->output_method == output_initial)

But in ELisp, the only simple&convenient way I know is
(eq frame frame-initial-frame), but I'm not sure if that var is still
non-nil when that code is run.

I think it's better to add a specific workaround in
`undelete-frame--handle-delete-frame` such as:

    (unless (eq frame frame-initial-frame) ...)

And if that specific one doesn't work, we'll come up with
a different test.


        Stefan




^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 13:18   ` Yuri Khan
@ 2022-01-21 14:29     ` Eli Zaretskii
  2022-01-21 15:07       ` Yuri Khan
  2022-01-21 16:46       ` [External] : " Drew Adams
  0 siblings, 2 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 14:29 UTC (permalink / raw)
  To: Yuri Khan; +Cc: gregory, juri, emacs-devel

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Fri, 21 Jan 2022 20:18:03 +0700
> Cc: Emacs developers <emacs-devel@gnu.org>, Juri Linkov <juri@linkov.net>
> 
> On Fri, 21 Jan 2022 at 16:38, Gregory Heytings <gregory@heytings.org> wrote:
> 
> > Therefore, to help users discover that this menu item required an explicit
> > activation in their init files to become usable, I added a "Enable Frame
> > Undeletion" item in the File menu, with a tooltip which said "Enable frame
> > undeletion for this session"
> 
> Why would it be on the File menu rather than Options?

Because that's where Delete Frame is.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 13:50         ` Gregory Heytings
@ 2022-01-21 14:32           ` Eli Zaretskii
  2022-01-21 14:48             ` Gregory Heytings
  0 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 14:32 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, emacs-devel, juri

> Date: Fri, 21 Jan 2022 13:50:03 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, juri@linkov.net, emacs-devel@gnu.org
> 
> > Why is it a problem to allow disabling undeletion of frames once that 
> > was enabled?
> 
> It's not a problem at all, but it's not what the "Enable Frame Undeletion" 
> menu item was meant to do.  It was meant to tell users that it is 
> necessary to add somethig their init files to enable that feature.

So now we have that indication, plus the ability of turning the mdoe
back off.  Why is that a problem?  Your original feature is still
there (albeit under a slightly different name), and there's an
_additional_ feature as well.  How is that a problem?

> I attach three possible solutions, in order of personal preference:

I'm sorry, I'm unable to discuss solutions to a problem I don't
understand.

Once again: what is the problem that needs to be fixed here, and why?
That Emacs behaves slightly differently from your original code is not
a problem in itself.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 14:32           ` Eli Zaretskii
@ 2022-01-21 14:48             ` Gregory Heytings
  2022-01-21 19:41               ` Eli Zaretskii
  0 siblings, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 14:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, emacs-devel, juri


>> I attach three possible solutions, in order of personal preference:
>
> I'm sorry, I'm unable to discuss solutions to a problem I don't 
> understand.
>
> Once again: what is the problem that needs to be fixed here, and why? 
> That Emacs behaves slightly differently from your original code is not a 
> problem in itself.
>

The three problems of the current state are:

1. All File menu items are shifted to the right because of a single 
checkbox.

2. The "Undelete Frame" item appears and disappears, instead of being 
disabled and enabled.

3. The frame undeletion items uses two entries in the File menu, IMO 
that's one too much, and with the proposed options 1 and 2 it uses only 
one.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 13:50     ` Stefan Monnier
@ 2022-01-21 15:07       ` Gregory Heytings
  0 siblings, 0 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 15:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Juri Linkov


>
> I think it's better to add a specific workaround in 
> `undelete-frame--handle-delete-frame` such as:
>
>    (unless (eq frame frame-initial-frame) ...)
>
> And if that specific one doesn't work, we'll come up with a different 
> test.
>

That problem was already identified and solved (with a boolean state 
variable) in bug#51883, before the current solution (using a minor mode 
that users can turn on in their init file) was adopted.  With a minor mode 
this test becomes unnecessary.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 14:29     ` Eli Zaretskii
@ 2022-01-21 15:07       ` Yuri Khan
  2022-01-21 19:22         ` Eli Zaretskii
  2022-01-21 16:46       ` [External] : " Drew Adams
  1 sibling, 1 reply; 62+ messages in thread
From: Yuri Khan @ 2022-01-21 15:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Gregory Heytings, Juri Linkov, Emacs developers

On Fri, 21 Jan 2022 at 21:29, Eli Zaretskii <eliz@gnu.org> wrote:

> > Why would it be on the File menu rather than Options?
>
> Because that's where Delete Frame is.

That does not explain much. ‘Cut’, ‘Paste’ and ‘Search’ are on Edit
but ‘Use CUA Keys’ and ‘Default Search Options’ are on Options. Buffer
names are on Buffers, but ‘Use Directory Names in Buffer Names’ is on
Options.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [External] : Re: Undo mode
  2022-01-21 10:19     ` Gregory Heytings
@ 2022-01-21 16:45       ` Drew Adams
  0 siblings, 0 replies; 62+ messages in thread
From: Drew Adams @ 2022-01-21 16:45 UTC (permalink / raw)
  To: Gregory Heytings, Stefan Kangas
  Cc: Eli Zaretskii, Juri Linkov, emacs-devel@gnu.org

> See bug#51883.  The conclusion was that undeleting frames is not a feature
> that all users need, and that enabling it by default uses some memory.

I haven't read that bug thread, but a
quick look at its status suggests that
there hasn't yet been _any_ conclusion.

Is this incorrect?



^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [External] : Re: Undo mode
  2022-01-21 12:50     ` [External] : Re: Undo mode Stefan Monnier
@ 2022-01-21 16:45       ` Drew Adams
  0 siblings, 0 replies; 62+ messages in thread
From: Drew Adams @ 2022-01-21 16:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Po Lu, emacs-devel@gnu.org, Juri Linkov

You quoted two different participants, using
the same quote level.  Please don't do that -
it can confuse or mislead.  (Thx.)

Here's the text you quoted, but clarified to
indicate who said what.

>da> But if something isn't in an Emacs
>da> release (the latest is 27.2) then
>da> I'd say it hasn't sailed.
> 
> 100% agreed.
> 
>pl> What if someone tracking master (and
>pl> there are enough of them to make a
>pl> difference, I think) has customized
>pl> `undelete-frame-mode' to t?
> 
> What about that?




^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [External] : Re: Undo mode
  2022-01-21 14:29     ` Eli Zaretskii
  2022-01-21 15:07       ` Yuri Khan
@ 2022-01-21 16:46       ` Drew Adams
  1 sibling, 0 replies; 62+ messages in thread
From: Drew Adams @ 2022-01-21 16:46 UTC (permalink / raw)
  To: Eli Zaretskii, Yuri Khan
  Cc: gregory@heytings.org, emacs-devel@gnu.org, juri@linkov.net

> > Why would it be on the File menu rather than Options?
> 
> Because that's where Delete Frame is.

FWIW, a `Frames' menu or submenu might
be useful, for grouping frame commands.
___

My library menu-bar+.el adds a top-level
`Frames' menu, if library frame-cmds.el
or fit-frame.el has been loaded.

Menu `Frames' has these menu items:

 _________
 Hide Frames / Show Buffers        C-M-S-z
 Iconify All Frames
 _________
 Maximize Frame
 Maximize Frame Horizontally
 Maximize Frame Vertically
 Toggle Max Frame
 Toggle Max Frame Horizontally
 Toggle Max Frame Vertically
 _________
 Tile Frames Horizontally...
 Tile Frames Vertically...
 _________
 Set Frame Parameter from Frame...
 Set All Frame Parameters from Frame...
 Fit This Frame                    C-x C-_
 Delete Frame                      C-x 5 0
 New Frame                         C-x 5 2
 New Frame on Display...
 _________

Vanilla Emacs has its own set of commands
that could be appropriate for a `Frames'
(sub)menu.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 15:07       ` Yuri Khan
@ 2022-01-21 19:22         ` Eli Zaretskii
  0 siblings, 0 replies; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 19:22 UTC (permalink / raw)
  To: Yuri Khan; +Cc: gregory, juri, emacs-devel

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Fri, 21 Jan 2022 22:07:44 +0700
> Cc: Gregory Heytings <gregory@heytings.org>, Emacs developers <emacs-devel@gnu.org>, 
> 	Juri Linkov <juri@linkov.net>
> 
> On Fri, 21 Jan 2022 at 21:29, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > > Why would it be on the File menu rather than Options?
> >
> > Because that's where Delete Frame is.
> 
> That does not explain much. ‘Cut’, ‘Paste’ and ‘Search’ are on Edit
> but ‘Use CUA Keys’ and ‘Default Search Options’ are on Options. Buffer
> names are on Buffers, but ‘Use Directory Names in Buffer Names’ is on
> Options.

The options in your examples are much more distant from the
corresponding commands, so I see no analogy and no need to do
something similar in this case.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 14:48             ` Gregory Heytings
@ 2022-01-21 19:41               ` Eli Zaretskii
  2022-01-21 20:58                 ` Gregory Heytings
  0 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-21 19:41 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, emacs-devel, juri

> Date: Fri, 21 Jan 2022 14:48:44 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, juri@linkov.net, emacs-devel@gnu.org
> 
> The three problems of the current state are:
> 
> 1. All File menu items are shifted to the right because of a single 
> checkbox.

Doesn't sound too bad to me.  We have the same in Text menu, for
example.

> 2. The "Undelete Frame" item appears and disappears, instead of being 
> disabled and enabled.

That's easy to change, but why is it better to show it disabled
instead of not showing it at all?

> 3. The frame undeletion items uses two entries in the File menu, IMO 
> that's one too much, and with the proposed options 1 and 2 it uses only 
> one.

That's not a separate issue, so I don't think it counts.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 19:41               ` Eli Zaretskii
@ 2022-01-21 20:58                 ` Gregory Heytings
  2022-01-22  0:47                   ` Po Lu
  2022-01-22  7:11                   ` Eli Zaretskii
  0 siblings, 2 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-21 20:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, emacs-devel, juri

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


>> 1. All File menu items are shifted to the right because of a single 
>> checkbox.
>
> Doesn't sound too bad to me.
>

It depends on the toolkit you use I guess.  With Lucid the effect is 
awful, see attached screenshots.

>
> We have the same in Text menu, for example.
>

Perhaps we're looking at different Text menus, but I see five entries in 
that menu, two of which have a checkbox.

>> 2. The "Undelete Frame" item appears and disappears, instead of being 
>> disabled and enabled.
>
> That's easy to change, but why is it better to show it disabled instead 
> of not showing it at all?
>

It's what happens elsewhere in that menu and in others, some items are 
disabled or enabled depending on whether their corresponding action can be 
performed, but none disappear when their corresponding action cannot be 
performed, at least not in the standard menus.

>> 3. The frame undeletion items uses two entries in the File menu, IMO 
>> that's one too much, and with the proposed options 1 and 2 it uses only 
>> one.
>
> That's not a separate issue, so I don't think it counts.
>

What do you mean?  Problem 1 above is the appearance of the checkbox, 
problem 2 about the fact that a menu item appears and disappears, problem 
3 about the number of menu entries.  These are three separate things.

My conclusion so far is that the most reasonable compromise would be to 
use the second patch (attached again).

[-- Attachment #2: with-checkbox.png --]
[-- Type: image/png, Size: 33372 bytes --]

[-- Attachment #3: without-checkbox.png --]
[-- Type: image/png, Size: 31754 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: Fix-Undelete-Frame-menu-item-Option-2.patch --]
[-- Type: text/x-diff; name=Fix-Undelete-Frame-menu-item-Option-2.patch, Size: 1751 bytes --]

From 298fb61779e652016dda3fc4b4af2c0a23c0f5cc Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 21 Jan 2022 13:16:04 +0000
Subject: [PATCH] Fix Undelete Frame menu item (Option 2)

* lisp/menu-bar.el (menu-bar-file-menu): Restore the "Undelete Frame"
menu item, and add a disabled item with a tooltip explaining how to
enable it.
---
 lisp/menu-bar.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index d1ca16dbf6..d2ed6dc3b7 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,15 +108,15 @@ 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 [disabled-undelete-frame]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :visible (null undelete-frame-mode)
+                  :enable nil
+                  :help "Undelete-Frame mode is disabled"))
     (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 undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
 
     ;; Don't use delete-frame as event name because that is a special
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 20:58                 ` Gregory Heytings
@ 2022-01-22  0:47                   ` Po Lu
  2022-01-22  7:11                   ` Eli Zaretskii
  1 sibling, 0 replies; 62+ messages in thread
From: Po Lu @ 2022-01-22  0:47 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Eli Zaretskii, emacs-devel, juri

Gregory Heytings <gregory@heytings.org> writes:

> It depends on the toolkit you use I guess.  With Lucid the effect is
> awful, see attached screenshots.

We should fix the toolkit, then.  And perhaps provide an option for the
user to customize the behaviour.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 20:58                 ` Gregory Heytings
  2022-01-22  0:47                   ` Po Lu
@ 2022-01-22  7:11                   ` Eli Zaretskii
  2022-01-22  8:32                     ` Gregory Heytings
  1 sibling, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-22  7:11 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, emacs-devel, juri

> Date: Fri, 21 Jan 2022 20:58:54 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, juri@linkov.net, emacs-devel@gnu.org
> 
> >> 1. All File menu items are shifted to the right because of a single 
> >> checkbox.
> >
> > Doesn't sound too bad to me.
> 
> It depends on the toolkit you use I guess.  With Lucid the effect is 
> awful, see attached screenshots.
> 
> > We have the same in Text menu, for example.
> 
> Perhaps we're looking at different Text menus, but I see five entries in 
> that menu, two of which have a checkbox.

Yes, and how is this different from File?  Some items have the
checkbox, others don't, and all of them are shifted to the right as
result.

> >> 2. The "Undelete Frame" item appears and disappears, instead of being 
> >> disabled and enabled.
> >
> > That's easy to change, but why is it better to show it disabled instead 
> > of not showing it at all?
> 
> It's what happens elsewhere in that menu and in others, some items are 
> disabled or enabled depending on whether their corresponding action can be 
> performed, but none disappear when their corresponding action cannot be 
> performed, at least not in the standard menus.

That's factually untrue: there are several menu items that disappear
when they are not relevant.  Examples: "New Frame on Monitor...",
"Close Tab", and a few others.  But okay, I've now changed this
particular entry to be always present.

> >> 3. The frame undeletion items uses two entries in the File menu, IMO 
> >> that's one too much, and with the proposed options 1 and 2 it uses only 
> >> one.
> >
> > That's not a separate issue, so I don't think it counts.
> 
> What do you mean?

I mean if we add an item, there's one more item as a consequence.  So
the number of items is not a separate issue.

> My conclusion so far is that the most reasonable compromise would be to 
> use the second patch (attached again).

Given that I made the change above, I see no reason to change anything
else at this time.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  7:11                   ` Eli Zaretskii
@ 2022-01-22  8:32                     ` Gregory Heytings
  2022-01-22  8:52                       ` Eli Zaretskii
  0 siblings, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-22  8:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, juri, emacs-devel


>>>> The frame undeletion items uses two entries in the File menu, IMO 
>>>> that's one too much, and with the proposed options 1 and 2 it uses 
>>>> only one.
>>>
>>> That's not a separate issue, so I don't think it counts.
>>
>> What do you mean?
>
> I mean if we add an item, there's one more item as a consequence.  So 
> the number of items is not a separate issue.
>

The point is that there should be no more than one menu item for that 
feature.  There are two, that's one too much.  There is no reason to turn 
that feature, which has no practical effect except that it uses a few more 
bytes in the heap, on and off from the menu.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  8:32                     ` Gregory Heytings
@ 2022-01-22  8:52                       ` Eli Zaretskii
  2022-01-22  9:08                         ` Gregory Heytings
  0 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-22  8:52 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, juri, emacs-devel

> Date: Sat, 22 Jan 2022 08:32:44 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, emacs-devel@gnu.org, juri@linkov.net
> 
> The point is that there should be no more than one menu item for that 
> feature.  There are two, that's one too much.  There is no reason to turn 
> that feature, which has no practical effect except that it uses a few more 
> bytes in the heap, on and off from the menu.

I already explained why it would be very unusual to have a feature
that can only be turned on from the menu, but cannot be turned off.  I
see no reason for such strange behavior for this single mode.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  8:52                       ` Eli Zaretskii
@ 2022-01-22  9:08                         ` Gregory Heytings
  2022-01-22  9:47                           ` Po Lu
                                             ` (2 more replies)
  0 siblings, 3 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-22  9:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, juri, emacs-devel

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


>> The point is that there should be no more than one menu item for that 
>> feature.  There are two, that's one too much.  There is no reason to 
>> turn that feature, which has no practical effect except that it uses a 
>> few more bytes in the heap, on and off from the menu.
>
> I already explained why it would be very unusual to have a feature that 
> can only be turned on from the menu, but cannot be turned off.  I see no 
> reason for such strange behavior for this single mode.
>

Which is why I suggested to remove the possibility of enabling it from the 
menu.  See the attached patch (again).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix-Undelete-Frame-menu-item.patch --]
[-- Type: text/x-diff; name=Fix-Undelete-Frame-menu-item.patch, Size: 1739 bytes --]

From af5dea30cc40ca7b8aabc58898b1e2bd755beae6 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sat, 22 Jan 2022 08:21:41 +0000
Subject: [PATCH] Fix Undelete Frame menu item

* lisp/menu-bar.el (menu-bar-file-menu): Replace the checkbox with a
a disabled item with a tooltip telling that the mode is disabled.
---
 lisp/menu-bar.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index 817c2d485e..8d4ac1692f 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -108,15 +108,15 @@ 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 [disabled-undelete-frame-mode]
+      '(menu-item "Undelete Frame" undelete-frame
+                  :visible (null undelete-frame-mode)
+                  :enable nil
+                  :help "Undelete-Frame mode is currently disabled"))
     (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 undelete-frame-mode
+                  :enable undelete-frame--deleted-frames
                   :help "Undelete the most recently deleted frame"))
 
     ;; Don't use delete-frame as event name because that is a special
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  9:08                         ` Gregory Heytings
@ 2022-01-22  9:47                           ` Po Lu
  2022-01-22  9:51                           ` Stefan Kangas
  2022-01-22 10:32                           ` Eli Zaretskii
  2 siblings, 0 replies; 62+ messages in thread
From: Po Lu @ 2022-01-22  9:47 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: Eli Zaretskii, juri, emacs-devel

Gregory Heytings <gregory@heytings.org> writes:

> Which is why I suggested to remove the possibility of enabling it from
> the menu.  See the attached patch (again).

Why are you opposed to there being two menu items for such a feature?

I haven't heard a single argument to that end.  All I hear is some
variant of "it defaces the files menu", which is just another way of
repeating that you don't want those items to be there.

I also count 3 items in the Files menu related to multi-tty, which is
objectively less likely to be used than frame undeletion, and probably
less important to tell new users about as well.

Thanks.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  9:08                         ` Gregory Heytings
  2022-01-22  9:47                           ` Po Lu
@ 2022-01-22  9:51                           ` Stefan Kangas
  2022-01-22  9:58                             ` Po Lu
  2022-01-22 10:32                           ` Eli Zaretskii
  2 siblings, 1 reply; 62+ messages in thread
From: Stefan Kangas @ 2022-01-22  9:51 UTC (permalink / raw)
  To: Gregory Heytings, Eli Zaretskii; +Cc: luangruo, emacs-devel, juri

Gregory Heytings <gregory@heytings.org> writes:

> Which is why I suggested to remove the possibility of enabling it from the
> menu.  See the attached patch (again).

If it is already relegated to second-class citizenship and not a default
feature, I don't understand why it belongs in the default menu at all.

The menu entry could be made visible only if the minor mode is enabled.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21 11:56     ` Eli Zaretskii
@ 2022-01-22  9:51       ` Stefan Kangas
  2022-01-22 10:36         ` Eli Zaretskii
  0 siblings, 1 reply; 62+ messages in thread
From: Stefan Kangas @ 2022-01-22  9:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, juri

Eli Zaretskii <eliz@gnu.org> writes:

>> I don't think I understand the benefit of hiding this behind a mode.
>
> That was the conclusion of our discussion of the original issue.
> Please re-read it if you want to understand the reasons.

I did of course, but I still fail to see it.  Is it about the memory
consumption?  If so, did anyone measure its impact?  I'd have thought
that the memory impact was insignificant.

Or is there some other important principle at stake here?  Feel free to
elaborate.

Re-reading that discussion (now twice) just takes me back to the
conclusion that this makes Emacs harder to use, with no clear benefit.

Perhaps we should have a `low-memory-mode' to turn down _several_
defaults and disable some "memory intensive" features.  That might have
a measurable impact (we would need to check that), and would make much
more sense to me than disabling this undo command for that reason.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  9:51                           ` Stefan Kangas
@ 2022-01-22  9:58                             ` Po Lu
  0 siblings, 0 replies; 62+ messages in thread
From: Po Lu @ 2022-01-22  9:58 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Gregory Heytings, Eli Zaretskii, emacs-devel, juri

Stefan Kangas <stefankangas@gmail.com> writes:

> If it is already relegated to second-class citizenship and not a default
> feature, I don't understand why it belongs in the default menu at all.

Not being enabled by default doesn't give a feature "second-class
citizenship", I should think.

Besides, we have lots of such features in the default menu bar: the
Options and Tools menus are full of them.

Thanks.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  9:08                         ` Gregory Heytings
  2022-01-22  9:47                           ` Po Lu
  2022-01-22  9:51                           ` Stefan Kangas
@ 2022-01-22 10:32                           ` Eli Zaretskii
  2022-01-22 11:15                             ` Gregory Heytings
  2 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-22 10:32 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, juri, emacs-devel

> Date: Sat, 22 Jan 2022 09:08:46 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, emacs-devel@gnu.org, juri@linkov.net
> 
> > I already explained why it would be very unusual to have a feature that 
> > can only be turned on from the menu, but cannot be turned off.  I see no 
> > reason for such strange behavior for this single mode.
> >
> 
> Which is why I suggested to remove the possibility of enabling it from the 
> menu.  See the attached patch (again).

Sorry, I don't agree with such a change, as it would be a step back.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22  9:51       ` Stefan Kangas
@ 2022-01-22 10:36         ` Eli Zaretskii
  2022-01-22 12:01           ` Stefan Kangas
  0 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-22 10:36 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel, juri

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 22 Jan 2022 09:51:39 +0000
> Cc: juri@linkov.net, emacs-devel@gnu.org
> 
> > That was the conclusion of our discussion of the original issue.
> > Please re-read it if you want to understand the reasons.
> 
> I did of course, but I still fail to see it.  Is it about the memory
> consumption?  If so, did anyone measure its impact?  I'd have thought
> that the memory impact was insignificant.

Memory consumption, and also turning on by default a new mode that
wasn't there before.

> Re-reading that discussion (now twice) just takes me back to the
> conclusion that this makes Emacs harder to use, with no clear benefit.

How can it make Emacs harder to use when by default absolutely nothing
has changed??  Please make your claims at least remotely aligned with
the facts.

> Perhaps we should have a `low-memory-mode' to turn down _several_
> defaults and disable some "memory intensive" features.  That might have
> a measurable impact (we would need to check that), and would make much
> more sense to me than disabling this undo command for that reason.

Bikeshedding go!



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22 10:32                           ` Eli Zaretskii
@ 2022-01-22 11:15                             ` Gregory Heytings
  2022-01-22 11:18                               ` Eli Zaretskii
  0 siblings, 1 reply; 62+ messages in thread
From: Gregory Heytings @ 2022-01-22 11:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, juri, emacs-devel


>>> I already explained why it would be very unusual to have a feature 
>>> that can only be turned on from the menu, but cannot be turned off. 
>>> I see no reason for such strange behavior for this single mode.
>>
>> Which is why I suggested to remove the possibility of enabling it from 
>> the menu.  See the attached patch (again).
>
> Sorry, I don't agree with such a change, as it would be a step back.
>

I give up.

I'm sorry to have contributed to that mess.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22 11:15                             ` Gregory Heytings
@ 2022-01-22 11:18                               ` Eli Zaretskii
  2022-01-22 13:37                                 ` Gregory Heytings
  0 siblings, 1 reply; 62+ messages in thread
From: Eli Zaretskii @ 2022-01-22 11:18 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: luangruo, juri, emacs-devel

> Date: Sat, 22 Jan 2022 11:15:36 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, emacs-devel@gnu.org, juri@linkov.net
> 
> > Sorry, I don't agree with such a change, as it would be a step back.
> >
> 
> I give up.
> 
> I'm sorry to have contributed to that mess.

You have absolutely nothing to be sorry about.  Your original code is
present with very minor changes, and the feature works as you designed
it.  Thanks for contributing it to Emacs.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22 10:36         ` Eli Zaretskii
@ 2022-01-22 12:01           ` Stefan Kangas
  0 siblings, 0 replies; 62+ messages in thread
From: Stefan Kangas @ 2022-01-22 12:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, juri

Eli Zaretskii <eliz@gnu.org> writes:

>> I did of course, but I still fail to see it.  Is it about the memory
>> consumption?  If so, did anyone measure its impact?  I'd have thought
>> that the memory impact was insignificant.
>
> Memory consumption,

Feel free to provide details.

>  and also turning on by default a new mode that
> wasn't there before.

We don't need any new mode, see Juri's patch.

>> Re-reading that discussion (now twice) just takes me back to the
>> conclusion that this makes Emacs harder to use, with no clear benefit.
>
> How can it make Emacs harder to use

It is harder to use this feature, because you first have to enable a new
mode, or the command will just error out.

There is also the mode itself, and the additional menu entry/entries.

The question is: what do we win from this additional complexity?
I didn't see any satisfactory answer to that question.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-22 11:18                               ` Eli Zaretskii
@ 2022-01-22 13:37                                 ` Gregory Heytings
  0 siblings, 0 replies; 62+ messages in thread
From: Gregory Heytings @ 2022-01-22 13:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: luangruo, emacs-devel, juri


>
> Your original code is present with very minor changes, and the feature 
> works as you designed it.
>

A feature with a poor user interface is a misfeature.  That double menu 
entry and that checkbox is like a wart on the nose; I cannot consider such 
a change to the user interface as a "very minor change".

You may remind that I turned that feature into a minor mode upon your 
request, and that I turned it into an opt-in minor mode upon your request. 
My only point is this thread was to at least keep the visible user 
interface clean.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: Undo mode
  2022-01-21  8:41 ` Gregory Heytings
                     ` (2 preceding siblings ...)
  2022-01-21 13:18   ` Yuri Khan
@ 2022-01-22 18:04   ` Juri Linkov
  3 siblings, 0 replies; 62+ messages in thread
From: Juri Linkov @ 2022-01-22 18:04 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: emacs-devel

> The hard-coded limit of 16 deleted frames was also a conscious design
> choice, to avoid adding yet another defcustom that nobody had asked
> (yet).

A customizable option will be needed anyway, and it has such nice side effect,
that its value 0 obviates the need for the special mode, because 0 effectively
disables this feature.  So if the memory consumption is a real problem, its
default value could be 0.



^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2022-01-22 18:04 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-20 20:45 Undo mode Juri Linkov
2022-01-21  0:42 ` Stefan Monnier
2022-01-21  8:16   ` Juri Linkov
2022-01-21 11:44     ` Eli Zaretskii
2022-01-21 13:50     ` Stefan Monnier
2022-01-21 15:07       ` Gregory Heytings
2022-01-21  1:57 ` Po Lu
2022-01-21  3:11   ` [External] : " Drew Adams
2022-01-21  3:40     ` Po Lu
2022-01-21  5:35       ` Drew Adams
2022-01-21  7:56       ` Tracking master (was: [External] : Re: Undo mode) Kévin Le Gouguec
2022-01-21 12:50     ` [External] : Re: Undo mode Stefan Monnier
2022-01-21 16:45       ` Drew Adams
2022-01-21  8:18   ` Juri Linkov
2022-01-21  8:56   ` Lars Ingebrigtsen
2022-01-21  9:15     ` Po Lu
2022-01-21  9:17       ` Lars Ingebrigtsen
2022-01-21 12:52         ` Stefan Monnier
2022-01-21 11:41     ` Eli Zaretskii
2022-01-21  7:17 ` Eli Zaretskii
2022-01-21 10:09   ` Stefan Kangas
2022-01-21 10:19     ` Gregory Heytings
2022-01-21 16:45       ` [External] : " Drew Adams
2022-01-21 11:56     ` Eli Zaretskii
2022-01-22  9:51       ` Stefan Kangas
2022-01-22 10:36         ` Eli Zaretskii
2022-01-22 12:01           ` Stefan Kangas
2022-01-21  8:41 ` Gregory Heytings
2022-01-21  9:40   ` Po Lu
2022-01-21  9:57     ` Gregory Heytings
2022-01-21 10:19       ` Po Lu
2022-01-21 10:26         ` Gregory Heytings
2022-01-21 10:34           ` Po Lu
2022-01-21 10:45             ` Gregory Heytings
2022-01-21 10:51               ` Po Lu
2022-01-21 12:10           ` Eli Zaretskii
2022-01-21 11:58       ` Eli Zaretskii
2022-01-21 13:50         ` Gregory Heytings
2022-01-21 14:32           ` Eli Zaretskii
2022-01-21 14:48             ` Gregory Heytings
2022-01-21 19:41               ` Eli Zaretskii
2022-01-21 20:58                 ` Gregory Heytings
2022-01-22  0:47                   ` Po Lu
2022-01-22  7:11                   ` Eli Zaretskii
2022-01-22  8:32                     ` Gregory Heytings
2022-01-22  8:52                       ` Eli Zaretskii
2022-01-22  9:08                         ` Gregory Heytings
2022-01-22  9:47                           ` Po Lu
2022-01-22  9:51                           ` Stefan Kangas
2022-01-22  9:58                             ` Po Lu
2022-01-22 10:32                           ` Eli Zaretskii
2022-01-22 11:15                             ` Gregory Heytings
2022-01-22 11:18                               ` Eli Zaretskii
2022-01-22 13:37                                 ` Gregory Heytings
2022-01-21 11:48   ` Eli Zaretskii
2022-01-21 13:18   ` Yuri Khan
2022-01-21 14:29     ` Eli Zaretskii
2022-01-21 15:07       ` Yuri Khan
2022-01-21 19:22         ` Eli Zaretskii
2022-01-21 16:46       ` [External] : " Drew Adams
2022-01-22 18:04   ` Juri Linkov
2022-01-21  8:55 ` Lars Ingebrigtsen

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.