bug#69738: [BUG] rmail-mail-new-frame doesn't delete the new frame after composing the message on Emacs 29.2

bug#69738: [BUG] rmail-mail-new-frame doesn't delete the new frame after composing the message on Emacs 29.2

[rameiko87]

For brevity I don't include the steps for C-c nor for C-s but only for 
C-d and C-k; but I can assure you that the case of C-c would yield the 
same result; I wonder what the desired result would be for C-s instead.

Steps to reproduce:

emacs -Q -nw
M-: (setq rmail-mail-new-frame t)
M-x rmail
m
C-c C-d

alternatively, use C-c C-k on the last line.

Expected result:
The frame is deleted (as the manual specifies at the end of 34.10).

Actual result:
The frame is not deleted.

bug#69738: [BUG] rmail-mail-new-frame doesn't delete the new frame after composing the message on Emacs 29.2

[Eli Zaretskii]

> Date: Mon, 11 Mar 2024 22:49:54 +0000
> From: rameiko87@posteo.net
> 
> For brevity I don't include the steps for C-c nor for C-s but only for 
> C-d and C-k; but I can assure you that the case of C-c would yield the 
> same result; I wonder what the desired result would be for C-s instead.
> 
> Steps to reproduce:
> 
> emacs -Q -nw
> M-: (setq rmail-mail-new-frame t)
> M-x rmail
> m
> C-c C-d
> 
> alternatively, use C-c C-k on the last line.
> 
> Expected result:
> The frame is deleted (as the manual specifies at the end of 34.10).
> 
> Actual result:
> The frame is not deleted.

It's a documentation bug: that frame is deleted only if certain
conditions are met.  One of those conditions is that there are
visible frames on display besides the frame where the message was
composed.  In the -nw session, that condition is not met.  See
rmail-mail-return for more details about what is actually being done
in each case.

IOW: this is a feature, and (IMO) a reasonable one at that, it just
needs a better documentation.

bug#69738: [BUG] rmail-mail-new-frame doesn't delete the new frame after composing the message on Emacs 29.2

[Eli Zaretskii]

> Cc: 69738@debbugs.gnu.org
> Date: Tue, 12 Mar 2024 15:54:47 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Mon, 11 Mar 2024 22:49:54 +0000
> > From: rameiko87@posteo.net
> > 
> > For brevity I don't include the steps for C-c nor for C-s but only for 
> > C-d and C-k; but I can assure you that the case of C-c would yield the 
> > same result; I wonder what the desired result would be for C-s instead.
> > 
> > Steps to reproduce:
> > 
> > emacs -Q -nw
> > M-: (setq rmail-mail-new-frame t)
> > M-x rmail
> > m
> > C-c C-d
> > 
> > alternatively, use C-c C-k on the last line.
> > 
> > Expected result:
> > The frame is deleted (as the manual specifies at the end of 34.10).
> > 
> > Actual result:
> > The frame is not deleted.
> 
> It's a documentation bug: that frame is deleted only if certain
> conditions are met.  One of those conditions is that there are
> visible frames on display besides the frame where the message was
> composed.  In the -nw session, that condition is not met.  See
> rmail-mail-return for more details about what is actually being done
> in each case.
> 
> IOW: this is a feature, and (IMO) a reasonable one at that, it just
> needs a better documentation.

I've now made the documentation more accurate (on the emacs-29
branch), and I'm therefore closing this bug.

bug#69738: Followup

[rameiko87]

Hello,

I use emacs -nw. I tried but just can't: the current way that 
rmail-mail-return is implemented makes no sense when 
rmail-mail-new-frame is true: every time I send an email I'm left with 
an extra frame displaying the duplicate of a buffer which either is 
already open on another frame, or which was buried and for some reason 
now resuscitates. Every time I have to manually delete such frame. It's 
very reasonable to expect that after creating a new frame just to send 
an email, then such frame is gotten rid of when the message is send (or 
aborted or whatever), and we're back to the original frame (as was 
originally implied by the manual).

The fact that it's such a reasonable expectation and that it takes so 
much inconvenience to delete the extra frame manually every time, makes 
me think that it should be this way by default, hence the manual was 
good and the code was to be changed... and I bet that every person which 
uses -nw with rmail-mail-new-frame will agree with me; is there any good 
reason to keep it this way, which escapes my analysis?

bug#69738: Followup

[Eli Zaretskii]

> Date: Sat, 13 Apr 2024 17:44:32 +0000
> From: rameiko87@posteo.net
> 
> Hello,
> 
> I use emacs -nw. I tried but just can't: the current way that 
> rmail-mail-return is implemented makes no sense when 
> rmail-mail-new-frame is true: every time I send an email I'm left with 
> an extra frame displaying the duplicate of a buffer which either is 
> already open on another frame, or which was buried and for some reason 
> now resuscitates. Every time I have to manually delete such frame. It's 
> very reasonable to expect that after creating a new frame just to send 
> an email, then such frame is gotten rid of when the message is send (or 
> aborted or whatever), and we're back to the original frame (as was 
> originally implied by the manual).
> 
> The fact that it's such a reasonable expectation and that it takes so 
> much inconvenience to delete the extra frame manually every time, makes 
> me think that it should be this way by default, hence the manual was 
> good and the code was to be changed... and I bet that every person which 
> uses -nw with rmail-mail-new-frame will agree with me; is there any good 
> reason to keep it this way, which escapes my analysis?

Does the patch below solve your use cases?

diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index d422383..5ab67b2 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -3755,9 +3755,12 @@ rmail-mail-return
    ;; probably wants to delete it now.
    ((display-multi-frame-p)
     (delete-frame))
-   ;; The previous frame is where normally they have the Rmail buffer
-   ;; displayed.
-   (t (other-frame -1))))
+   (t
+    ;; The previous frame is where normally they have the Rmail buffer
+    ;; displayed.
+    (let ((fr (selected-frame)))
+      (other-frame -1)
+      (delete-frame fr)))))
 
 (defun rmail-mail ()
   "Send mail in another window.

bug#69738: Followup

[rameiko87]

Why not just remove the condition of (display-multi-frame-p)? It's 
neater, and I can't see any drawbacks compared to your patch (but the 
fact that your code insists on switching to other before deleting the 
frame makes me think there must be some reason...?)

> Does the patch below solve your use cases?
> 
> diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
> index d422383..5ab67b2 100644
> --- a/lisp/mail/rmail.el
> +++ b/lisp/mail/rmail.el
> @@ -3755,9 +3755,12 @@ rmail-mail-return
>     ;; probably wants to delete it now.
>     ((display-multi-frame-p)
>      (delete-frame))
> -   ;; The previous frame is where normally they have the Rmail buffer
> -   ;; displayed.
> -   (t (other-frame -1))))
> +   (t
> +    ;; The previous frame is where normally they have the Rmail buffer
> +    ;; displayed.
> +    (let ((fr (selected-frame)))
> +      (other-frame -1)
> +      (delete-frame fr)))))
> 
>  (defun rmail-mail ()
>    "Send mail in another window.

bug#69738: Followup

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 16:14:20 +0000
> From: rameiko87@posteo.net
> Cc: 69738@debbugs.gnu.org
> 
> Why not just remove the condition of (display-multi-frame-p)? It's 
> neater, and I can't see any drawbacks compared to your patch (but the 
> fact that your code insists on switching to other before deleting the 
> frame makes me think there must be some reason...?)

Yes, I have my reasons: I'd like to make sure we switch to the exact
frame the user wants -- the one showing the Rmail buffer.  Unlike on
GUI displays, only a single frame is shown on a TTY, so if we
accidentally switch to the wrong frame, the user will not see the
frame they need, something that does happen on GUI terminals.

Does the patch as I sent it work for you?  If not, please tell what
doesn't work.

bug#69738: Followup

[rameiko87]

Dear Eli,

To answer your question: your patch doesn't work, and the reason is that 
I have Rmail on frame #6 and Elfeed on frame #3. Rmail-reply creates 
frame #42, from which both C-x 5 0 and C-x 5 o land on frame #27. C-u - 
C-x 5 o goes from #42 to #3, and also from #27 to #3 after #42 was 
deleted.

As for the current code for GUI, I can't understand why it _works_ since 
the same exact problem should arise. I never used the GUI but I would 
expect that it actually doesn't work for GUI either, for the same 
reasons above. I think the design of Emacs makes the order of frames 
rigid, so every new frame can be arbitrarily far from the original Rmail 
frame.

One way is to remember the frame where Rmail was and revert back to that 
one after deleting the reply frame.

On 14.04.2024 18:30, Eli Zaretskii wrote:
>> Why not just remove the condition of (display-multi-frame-p)? It's
>> neater, and I can't see any drawbacks compared to your patch (but the
>> fact that your code insists on switching to other before deleting the
>> frame makes me think there must be some reason...?)
> 
> Yes, I have my reasons: I'd like to make sure we switch to the exact
> frame the user wants -- the one showing the Rmail buffer.  Unlike on
> GUI displays, only a single frame is shown on a TTY, so if we
> accidentally switch to the wrong frame, the user will not see the
> frame they need, something that does happen on GUI terminals.
> 
> Does the patch as I sent it work for you?  If not, please tell what
> doesn't work.

bug#69738: Followup

[rameiko87]

Actually the neat solution is probably to treat frames as a ring, so 
that C-x 5 o would go back to the previous frame. Is there any reason 
why frames were not conceived as a ring, but ``rigidly''?


> Dear Eli,
> 
> To answer your question: your patch doesn't work, and the reason is
> that I have Rmail on frame #6 and Elfeed on frame #3. Rmail-reply
> creates frame #42, from which both C-x 5 0 and C-x 5 o land on frame
> #27. C-u - C-x 5 o goes from #42 to #3, and also from #27 to #3 after
> #42 was deleted.
> 
> As for the current code for GUI, I can't understand why it _works_
> since the same exact problem should arise. I never used the GUI but I
> would expect that it actually doesn't work for GUI either, for the
> same reasons above. I think the design of Emacs makes the order of
> frames rigid, so every new frame can be arbitrarily far from the
> original Rmail frame.
> 
> One way is to remember the frame where Rmail was and revert back to
> that one after deleting the reply frame.
> 
> On 14.04.2024 18:30, Eli Zaretskii wrote:
>>> Why not just remove the condition of (display-multi-frame-p)? It's
>>> neater, and I can't see any drawbacks compared to your patch (but the
>>> fact that your code insists on switching to other before deleting the
>>> frame makes me think there must be some reason...?)
>> 
>> Yes, I have my reasons: I'd like to make sure we switch to the exact
>> frame the user wants -- the one showing the Rmail buffer.  Unlike on
>> GUI displays, only a single frame is shown on a TTY, so if we
>> accidentally switch to the wrong frame, the user will not see the
>> frame they need, something that does happen on GUI terminals.
>> 
>> Does the patch as I sent it work for you?  If not, please tell what
>> doesn't work.

bug#69738: Followup

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 17:47:06 +0000
> From: rameiko87@posteo.net
> Cc: 69738@debbugs.gnu.org
> 
> To answer your question: your patch doesn't work, and the reason is that 
> I have Rmail on frame #6 and Elfeed on frame #3. Rmail-reply creates 
> frame #42, from which both C-x 5 0 and C-x 5 o land on frame #27. C-u - 
> C-x 5 o goes from #42 to #3, and also from #27 to #3 after #42 was 
> deleted.

Strange, it did work for me.  Moreover, the original code always
succeeds to switch back to the frame where I have the Rmail buffer, it
just doesn't delete the frame where the outgoing mail was composed, so
I don't think I understand why the same code doesn't work for you.
But I never got to 42 frames, of course.  So please show a minimal
recipe for reproducing this, after applying the patch I sent, and
starting from "emacs -Q -nw".  There's probably something I'm missing.

Thanks.

bug#69738: Followup

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 17:55:55 +0000
> From: rameiko87@posteo.net
> Cc: 69738@debbugs.gnu.org
> 
> Actually the neat solution is probably to treat frames as a ring, so 
> that C-x 5 o would go back to the previous frame. Is there any reason 
> why frames were not conceived as a ring, but ``rigidly''?

Thanks, but it's a non-starter to change how frames are managed, just
to solve this minor issue.  The solution cannot be too complicated,
certainly not with TTY frames, where even the window manager isn't
involved.

bug#69738: Followup

[rameiko87]

> So please show a minimal
> recipe for reproducing this, after applying the patch I sent, and
> starting from "emacs -Q -nw".  There's probably something I'm missing.

emacs -nw -Q (FRAME 1 IS SCRATCH)
C-x 5 2
M-x load-file PATCH
M-x rmail (FRAME 2 IS RMAIL)
M-: (setq rmail-mail-new-frame t)
C-x 5 2
C-x b *Messages* (FRAME 3 IS MESSAGES)
C-x 5 o (BACK TO FRAME 2)
m (NEW MAIL OPENS ON FRAME 4)
C-u - C-x 5 o (THIS IS WHAT THE PATCH DOES, REVERTING BACK TO FRAME 1 
WHICH IS SCRATCH AND NOT RMAIL)

Does it make sense?

Curiously, replacing the last line by C-c C-k kills the draft but 
doesn't change the frame (note that I applied the patch). I deduce that 
rmail-mail-return is not called by C-c C-k... Am I doing something 
wrong?

bug#69738: Followup

[Eli Zaretskii]

> Date: Mon, 15 Apr 2024 10:15:55 +0000
> From: rameiko87@posteo.net
> Cc: 69738@debbugs.gnu.org
> 
> > So please show a minimal
> > recipe for reproducing this, after applying the patch I sent, and
> > starting from "emacs -Q -nw".  There's probably something I'm missing.
> 
> emacs -nw -Q (FRAME 1 IS SCRATCH)
> C-x 5 2
> M-x load-file PATCH
> M-x rmail (FRAME 2 IS RMAIL)
> M-: (setq rmail-mail-new-frame t)
> C-x 5 2
> C-x b *Messages* (FRAME 3 IS MESSAGES)
> C-x 5 o (BACK TO FRAME 2)
> m (NEW MAIL OPENS ON FRAME 4)
> C-u - C-x 5 o (THIS IS WHAT THE PATCH DOES, REVERTING BACK TO FRAME 1 
> WHICH IS SCRATCH AND NOT RMAIL)
> 
> Does it make sense?

I guess it does, although it evidently breaks the expectations of
Rmail.  Please try a more thorough patch below.

> Curiously, replacing the last line by C-c C-k kills the draft but 
> doesn't change the frame (note that I applied the patch). I deduce that 
> rmail-mail-return is not called by C-c C-k... Am I doing something 
> wrong?

It looks like "C-c C-k" (implemented in message.el) was not intended
to return to the original buffer.

diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
index d422383..437f120 100644
--- a/lisp/mail/rmail.el
+++ b/lisp/mail/rmail.el
@@ -3684,7 +3684,12 @@ rmail-start-mail
 				   other-headers)
   (let ((switch-function
 	 (cond (same-window nil)
-	       (rmail-mail-new-frame 'switch-to-buffer-other-frame)
+	       (rmail-mail-new-frame
+                (progn
+                  ;; Record the frame from which we invoked this command.
+                  (modify-frame-parameters (selected-frame)
+				           '((rmail-orig-frame . t)))
+                  'switch-to-buffer-other-frame))
 	       (t 'switch-to-buffer-other-window)))
 	yank-action)
     (if replybuffer
@@ -3714,6 +3719,11 @@ rmail-start-mail
 	  (modify-frame-parameters (selected-frame)
 				   '((mail-dedicated-frame . t)))))))
 
+(defun rmail--find-orig-rmail-frame ()
+  (car (filtered-frame-list
+        (lambda (frame)
+          (eq (frame-parameter frame 'rmail-orig-frame) t)))))
+
 (defun rmail-mail-return (&optional newbuf)
   "Try to return to Rmail from the mail window.
 If optional argument NEWBUF is specified, it is the Rmail buffer
@@ -3755,9 +3765,19 @@ rmail-mail-return
    ;; probably wants to delete it now.
    ((display-multi-frame-p)
     (delete-frame))
-   ;; The previous frame is where normally they have the Rmail buffer
-   ;; displayed.
-   (t (other-frame -1))))
+   (t
+    ;; Try to find the original Rmail frame and make it the top frame.
+    (let ((fr (selected-frame))
+          (orig-fr (rmail--find-orig-rmail-frame)))
+      (if orig-fr
+          (progn
+            (modify-frame-parameters orig-fr '((rmail-orig-frame . nil)))
+            (select-frame-set-input-focus orig-fr))
+        ;; If we cannot find the frame from which we started, punt, and
+        ;; display the previous frame, which is where they normally have
+        ;; the Rmail buffer displayed.
+        (other-frame -1))
+      (delete-frame fr)))))
 
 (defun rmail-mail ()
   "Send mail in another window.

bug#69738: Followup

[Eli Zaretskii]

Ping!  Could you please try the patch I propose below?

> Cc: 69738@debbugs.gnu.org
> Date: Sat, 20 Apr 2024 14:11:51 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Mon, 15 Apr 2024 10:15:55 +0000
> > From: rameiko87@posteo.net
> > Cc: 69738@debbugs.gnu.org
> > 
> > > So please show a minimal
> > > recipe for reproducing this, after applying the patch I sent, and
> > > starting from "emacs -Q -nw".  There's probably something I'm missing.
> > 
> > emacs -nw -Q (FRAME 1 IS SCRATCH)
> > C-x 5 2
> > M-x load-file PATCH
> > M-x rmail (FRAME 2 IS RMAIL)
> > M-: (setq rmail-mail-new-frame t)
> > C-x 5 2
> > C-x b *Messages* (FRAME 3 IS MESSAGES)
> > C-x 5 o (BACK TO FRAME 2)
> > m (NEW MAIL OPENS ON FRAME 4)
> > C-u - C-x 5 o (THIS IS WHAT THE PATCH DOES, REVERTING BACK TO FRAME 1 
> > WHICH IS SCRATCH AND NOT RMAIL)
> > 
> > Does it make sense?
> 
> I guess it does, although it evidently breaks the expectations of
> Rmail.  Please try a more thorough patch below.
> 
> > Curiously, replacing the last line by C-c C-k kills the draft but 
> > doesn't change the frame (note that I applied the patch). I deduce that 
> > rmail-mail-return is not called by C-c C-k... Am I doing something 
> > wrong?
> 
> It looks like "C-c C-k" (implemented in message.el) was not intended
> to return to the original buffer.
> 
> diff --git a/lisp/mail/rmail.el b/lisp/mail/rmail.el
> index d422383..437f120 100644
> --- a/lisp/mail/rmail.el
> +++ b/lisp/mail/rmail.el
> @@ -3684,7 +3684,12 @@ rmail-start-mail
>  				   other-headers)
>    (let ((switch-function
>  	 (cond (same-window nil)
> -	       (rmail-mail-new-frame 'switch-to-buffer-other-frame)
> +	       (rmail-mail-new-frame
> +                (progn
> +                  ;; Record the frame from which we invoked this command.
> +                  (modify-frame-parameters (selected-frame)
> +				           '((rmail-orig-frame . t)))
> +                  'switch-to-buffer-other-frame))
>  	       (t 'switch-to-buffer-other-window)))
>  	yank-action)
>      (if replybuffer
> @@ -3714,6 +3719,11 @@ rmail-start-mail
>  	  (modify-frame-parameters (selected-frame)
>  				   '((mail-dedicated-frame . t)))))))
>  
> +(defun rmail--find-orig-rmail-frame ()
> +  (car (filtered-frame-list
> +        (lambda (frame)
> +          (eq (frame-parameter frame 'rmail-orig-frame) t)))))
> +
>  (defun rmail-mail-return (&optional newbuf)
>    "Try to return to Rmail from the mail window.
>  If optional argument NEWBUF is specified, it is the Rmail buffer
> @@ -3755,9 +3765,19 @@ rmail-mail-return
>     ;; probably wants to delete it now.
>     ((display-multi-frame-p)
>      (delete-frame))
> -   ;; The previous frame is where normally they have the Rmail buffer
> -   ;; displayed.
> -   (t (other-frame -1))))
> +   (t
> +    ;; Try to find the original Rmail frame and make it the top frame.
> +    (let ((fr (selected-frame))
> +          (orig-fr (rmail--find-orig-rmail-frame)))
> +      (if orig-fr
> +          (progn
> +            (modify-frame-parameters orig-fr '((rmail-orig-frame . nil)))
> +            (select-frame-set-input-focus orig-fr))
> +        ;; If we cannot find the frame from which we started, punt, and
> +        ;; display the previous frame, which is where they normally have
> +        ;; the Rmail buffer displayed.
> +        (other-frame -1))
> +      (delete-frame fr)))))
>  
>  (defun rmail-mail ()
>    "Send mail in another window.
> 
> 
> 
> 

bug#69738: Followup

[rameiko87]

On Sat, May 04 2024, Eli Zaretskii wrote:

> Ping!  Could you please try the patch I propose below?

Sure, will check this very soon.

bug#69738: Followup

[rameiko87]

> Ping!  Could you please try the patch I propose below?

I was attempting to try the patch, but I don't know how to incorporate
your changes unless by manually rewriting rmail.el: is there an
automatic way of doing it?

bug#69738: Followup

[Eli Zaretskii]

> From: rameiko87@posteo.net
> Cc: 69738@debbugs.gnu.org
> Date: Sun, 12 May 2024 23:42:17 +0100
> 
> > Ping!  Could you please try the patch I propose below?
> 
> I was attempting to try the patch, but I don't know how to incorporate
> your changes unless by manually rewriting rmail.el: is there an
> automatic way of doing it?

Semi-automatic.  First, patch rmail.el:

  $ patch -d /path/to/rmail.el -p3 < PATCH-FILE

(where PATCH-FILE is the file to which you copy the patch I sent, and
/path/to/rmail.el is the absolute file name of rmail.el on your
system.)

Then byte-compile and load the patched rmail.el:

  $ emacs -nw
  C-x C-f /path/to/rmail.el
  M-x emacs-lisp-byte-compile-and-load RET

Now you have the modified rmail.el byte-compiled and loaded into your
session, and can try the recipes that previously didn't work.

Thanks.