bug#51753: ERC switches to channel buffer on reconnect

bug#51753: ERC switches to channel buffer on reconnect

[Stefan Kangas]

Severity: important

When ERC reconnects, it switches buffer to the channel buffers.  This is
*very* dangerous.

Consider the situation when the user is about to paste a password and
hit enter with a quick "C-y RET", when all of a sudden the ERC buffer
pops up a fraction of a second before you hit those keys.  Now your
password is on IRC.

At the very least, this behavior should be disabled by default, and
preferably also come with a big warning sign for anyone that intends to
enable it.

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

Stefan Kangas <stefan@marxist.se> writes:

> Severity: important
>
> When ERC reconnects, it switches buffer to the channel buffers.  This is
> *very* dangerous.

Hi Stefan. Any idea if this is a recent phenomenon or more along the
lines of traditional behavior, perhaps involving `erc-join-buffer', for
example? I ask this mostly out of self interest re

  commit 17e7cab1507a185d2c493548494abcad6a55d159
  Normalize usage of variable erc-server-reconnecting

and its followup

  commit ec9ddd6eaf3f1b118d3ce95a69e1d046166362d3
  Deprecate instead of redefine erc-server-reconnecting

(both recent and both mine).

Regardless, there's a solid chance what you're experiencing is
`erc-setup-buffer' related, which comes into play whenever `erc-open'
runs. For reconnects, this happens both during `erc-server-reconnect'
and again whenever `erc-server-JOIN' runs (for channels you join).

It'd also be nice to know how these JOIN replies are being triggered
(server-initiated, erc-join.el, manual /join, etc.). With the join
module, they'd likely be the result of a JOIN command (request) sent by
`erc-autojoin-channels', which runs on 376/422. This matters when trying
to pinpoint problematic interplay with the reconnect logic, if such a
thing exists.

> Consider the situation when the user is about to paste a password and
> hit enter with a quick "C-y RET", when all of a sudden the ERC buffer
> pops up a fraction of a second before you hit those keys.  Now your
> password is on IRC.
>
> At the very least, this behavior should be disabled by default, and
> preferably also come with a big warning sign for anyone that intends to
> enable it.

I don't think many would argue that this behavior isn't at least
annoying. While specific scenarios (like accidentally broadcasting creds
to a channel) are important to confront, we should also remember to take
in the long view whenever possible (IMO). And in doing so, I think we'll
find that the many downsides of interfacing with a services bot have
over time compelled the IRC world to embrace SASL (part of the IRCv3
initiative) as the preferred means of authentication. Thanks.

bug#51753: ERC switches to channel buffer on reconnect

[Lars Ingebrigtsen]

"J.P." <jp@neverwas.me> writes:

> Hi Stefan. Any idea if this is a recent phenomenon or more along the
> lines of traditional behavior, perhaps involving `erc-join-buffer', for
> example?

erc has behaved this way as long as I can remember, and it's really
annoying (especially when a server bounces and erc reconnects, popping
up windows everywhere while I'm trying to get some work done).

So I'm all for making erc stopping doing this.

erc does not pop up windows when somebody messages you (but just
displays something in the mode line), and I think that's what erc should
do for all buffers (by default; there could be a user option to allow it
to pop up windows, of course).

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

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> erc does not pop up windows when somebody messages you (but just
> displays something in the mode line), and I think that's what erc should
> do for all buffers (by default; there could be a user option to allow it
> to pop up windows, of course).

The simplest approach might be to just change the default values of
`erc-join-buffer' and `erc-auto-query' to 'bury. However, if people want
something different to happen when automatically reconnecting, we'd
probably have to remember whether `erc-server-reconnect-count' was ever
positive before crossing the logical connection threshold for the
current session.

This may come down to having `erc-connection-established' record the
count prior to resetting it (perhaps in a new, internal variable). And
then, during re-JOINs, `erc-setup-buffer' could weigh that recorded
value against some new option, like an `erc-display-reconnect' (or
whatever), and proceed accordingly.

bug#51753: ERC switches to channel buffer on reconnect

[Lars Ingebrigtsen]

"J.P." <jp@neverwas.me> writes:

> The simplest approach might be to just change the default values of
> `erc-join-buffer' and `erc-auto-query' to 'bury.

Yup.

> However, if people want something different to happen when
> automatically reconnecting, we'd probably have to remember whether
> `erc-server-reconnect-count' was ever positive before crossing the
> logical connection threshold for the current session.
>
> This may come down to having `erc-connection-established' record the
> count prior to resetting it (perhaps in a new, internal variable). And
> then, during re-JOINs, `erc-setup-buffer' could weigh that recorded
> value against some new option, like an `erc-display-reconnect' (or
> whatever), and proceed accordingly.

Sounds good to me.

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

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> However, if people want something different to happen when
>> automatically reconnecting, we'd probably have to remember whether
>> `erc-server-reconnect-count' was ever positive before crossing the
>> logical connection threshold for the current session.
>>
>> This may come down to having `erc-connection-established' record the
>> count prior to resetting it (perhaps in a new, internal variable). And
>> then, during re-JOINs, `erc-setup-buffer' could weigh that recorded
>> value against some new option, like an `erc-display-reconnect' (or
>> whatever), and proceed accordingly.
>
> Sounds good to me.

I wasn't sure if that meant I was supposed to work on this. If not,
please disregard.

Otherwise, the tests are in #48598 [1]. As for the name of the option
itself, I basically punted by going with `erc-reconnect-buffer' to try
and stay close to `erc-join-buffer'. If that doesn't matter, perhaps
`erc-reconnect-display' would be a better fit since we already have an
`erc-query-display' (even though that one's not as closely related).
Anyone with an opinion, please advise. Thanks.


[1] Around line 4736:

    https://gitlab.com/jpneverwas/erc-tools/-/raw/master/bugs/48598/patches/wip/0013-Update-ERC-scenarios-with-session-centric-naming.patch

    Or browsable (JS):

    https://gitlab.com/jpneverwas/erc-v3/-/blob/1333bda3c0d11ff06b1b2acbb27864c90d5ba303/test/erc-scenarios.el#L1668


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Customize-displaying-of-ERC-buffers-on-reconnect.patch --]
[-- Type: text/x-patch, Size: 3415 bytes --]

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 18 Nov 2021 23:39:54 -0800
Subject: [PATCH 01/29] Customize displaying of ERC buffers on reconnect

* lisp/erc/erc-backend.el (erc--server-last-reconnect-count):
Add variable to record last reconnect tally.

* lisp/erc/erc.el (erc-reconnect-buffer): Add option to specify
channel-buffer display behavior on reconnect.
(erc-setup-buffer): Use option `erc-reconnect-buffer' if warranted.
(erc-connection-established): Record reconnect count in internal var
before resetting.
---
 lisp/erc/erc-backend.el |  3 +++
 lisp/erc/erc.el         | 24 ++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 69f63dfbc4..4b39bd8a30 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -193,6 +193,9 @@ erc-server-connected
 (defvar-local erc-server-reconnect-count 0
   "Number of times we have failed to reconnect to the current server.")
 
+(defvar-local erc--server-last-reconnect-count 0
+  "Snapshot of reconnect count when connection established.")
+
 (defvar-local erc-server-quitting nil
   "Non-nil if the user requests a quit.")
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index c5a4fbe5a0..21ae25d920 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1521,6 +1521,22 @@ erc-join-buffer
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
+(defcustom erc-reconnect-buffer nil
+  "How (and whether) to display a channel buffer upon reconnecting.
+
+This only affects automatic reconnections and is ignored when issuing a
+/reconnect command or reinvoking `erc-tls' with the same args (assuming
+success, of course).  See `erc-join-buffer' for a description of
+possible values."
+  :group 'erc-buffers
+  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
+                 (const :tag "Split window and select" window)
+                 (const :tag "Split window, don't select" window-noselect)
+                 (const :tag "New frame" frame)
+                 (const :tag "Bury in new buffer" bury)
+                 (const :tag "Use current buffer" buffer)
+                 (const :tag "Use current buffer" t)))
+
 (defcustom erc-frame-alist nil
   "Alist of frame parameters for creating erc frames.
 A value of nil means to use `default-frame-alist'."
@@ -1950,7 +1966,10 @@ erc-update-modules
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
-  (pcase erc-join-buffer
+  (pcase (if (zerop (erc-with-server-buffer
+                      erc--server-last-reconnect-count))
+             erc-join-buffer
+           (or erc-reconnect-buffer erc-join-buffer))
     ('window
      (if (active-minibuffer-window)
          (display-buffer buffer)
@@ -4722,7 +4741,8 @@ erc-connection-established
             (nick (car (erc-response.command-args parsed)))
             (buffer (process-buffer proc)))
         (setq erc-server-connected t)
-	(setq erc-server-reconnect-count 0)
+        (setq erc--server-last-reconnect-count erc-server-reconnect-count
+              erc-server-reconnect-count 0)
         (erc-update-mode-line)
         (erc-set-initial-user-mode nick buffer)
         (erc-server-setup-periodical-ping buffer)
-- 
2.31.1

bug#51753: ERC switches to channel buffer on reconnect

[Stefan Kangas]

tags 51753 + patch
thanks

"J.P." <jp@neverwas.me> writes:

> I wasn't sure if that meant I was supposed to work on this. If not,
> please disregard.

You did exactly the right thing here.  Thank you for working on this!

> Otherwise, the tests are in #48598 [1]. As for the name of the option
> itself, I basically punted by going with `erc-reconnect-buffer' to try
> and stay close to `erc-join-buffer'. If that doesn't matter, perhaps
> `erc-reconnect-display' would be a better fit since we already have an
> `erc-query-display' (even though that one's not as closely related).
> Anyone with an opinion, please advise. Thanks.

On balance, I don't have a strong opinion either way.  (I do like
`erc-reconnect-display' slightly better, because many variables that end
with `-buffer' have to do with the _name_ of some buffer.  But it is
true that it is also nice to have a name similar to `erc-join-buffer'.)

I just have some minor nits below:

> +(defvar-local erc--server-last-reconnect-count 0
> +  "Snapshot of reconnect count when connection established.")

"when the connection was established", perhaps?

> +(defcustom erc-reconnect-buffer nil
> +  "How (and whether) to display a channel buffer upon reconnecting.
> +
> +This only affects automatic reconnections and is ignored when issuing a
> +/reconnect command or reinvoking `erc-tls' with the same args (assuming
> +success, of course).  See `erc-join-buffer' for a description of
> +possible values."
> +  :group 'erc-buffers
> +  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
> +                 (const :tag "Split window and select" window)
> +                 (const :tag "Split window, don't select" window-noselect)
> +                 (const :tag "New frame" frame)
> +                 (const :tag "Bury in new buffer" bury)
> +                 (const :tag "Use current buffer" buffer)
> +                 (const :tag "Use current buffer" t)))

What is the difference between `buffer' and t?  Should they really have
the same :tag?

If they are just two names for the same thing, I suggest we drop one of
them.

Other than that, LGTM (but I didn't test it).

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

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

Stefan Kangas <stefan@marxist.se> writes:

> On balance, I don't have a strong opinion either way.  (I do like
> `erc-reconnect-display' slightly better, because many variables that end
> with `-buffer' have to do with the _name_ of some buffer.  But it is
> true that it is also nice to have a name similar to `erc-join-buffer'.)

Screw it. Let's go with `erc-reconnect-display'.

> I just have some minor nits below:
>
>> +(defvar-local erc--server-last-reconnect-count 0
>> +  "Snapshot of reconnect count when connection established.")
>
> "when the connection was established", perhaps?

Now reads as ^.

> What is the difference between `buffer' and t?  Should they really have
> the same :tag?

No idea to both, unfortunately. But since the option is only used in
that one function (`erc-setup-buffer'), and since we don't claim
like-for-like compatibility with `erc-join-buffer' (which we've already
deviated from anyway by offering a choice of nil) ...

> I suggest we drop one of them.

Agreed. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v1-v2.diff --]
[-- Type: text/x-patch, Size: 2313 bytes --]

From d7e9871283cfc6f0e841adf99251ae7057d39d7b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 19 Nov 2021 04:41:50 -0800
Subject: NOT A PATCH


F. Jason Park (1):
  Customize displaying of ERC buffers on reconnect

 lisp/erc/erc-backend.el |  3 +++
 lisp/erc/erc.el         | 23 +++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

Interdiff:
diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 4b39bd8a30..0d586268e9 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -194,7 +194,7 @@ erc-server-reconnect-count
   "Number of times we have failed to reconnect to the current server.")
 
 (defvar-local erc--server-last-reconnect-count 0
-  "Snapshot of reconnect count when connection established.")
+  "Snapshot of reconnect count when the connection was established.")
 
 (defvar-local erc-server-quitting nil
   "Non-nil if the user requests a quit.")
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 21ae25d920..01be8ed397 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1521,7 +1521,7 @@ erc-join-buffer
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
-(defcustom erc-reconnect-buffer nil
+(defcustom erc-reconnect-display nil
   "How (and whether) to display a channel buffer upon reconnecting.
 
 This only affects automatic reconnections and is ignored when issuing a
@@ -1534,8 +1534,7 @@ erc-reconnect-buffer
                  (const :tag "Split window, don't select" window-noselect)
                  (const :tag "New frame" frame)
                  (const :tag "Bury in new buffer" bury)
-                 (const :tag "Use current buffer" buffer)
-                 (const :tag "Use current buffer" t)))
+                 (const :tag "Use current buffer" buffer)))
 
 (defcustom erc-frame-alist nil
   "Alist of frame parameters for creating erc frames.
@@ -1969,7 +1968,7 @@ erc-setup-buffer
   (pcase (if (zerop (erc-with-server-buffer
                       erc--server-last-reconnect-count))
              erc-join-buffer
-           (or erc-reconnect-buffer erc-join-buffer))
+           (or erc-reconnect-display erc-join-buffer))
     ('window
      (if (active-minibuffer-window)
          (display-buffer buffer)
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Customize-displaying-of-ERC-buffers-on-reconnect.patch --]
[-- Type: text/x-patch, Size: 3368 bytes --]

From d7e9871283cfc6f0e841adf99251ae7057d39d7b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 18 Nov 2021 23:39:54 -0800
Subject: [PATCH 1/1] Customize displaying of ERC buffers on reconnect

* lisp/erc/erc-backend.el (erc--server-last-reconnect-count):
Add variable to record last reconnect tally.

* lisp/erc/erc.el (erc-reconnect-buffer): Add option to specify
channel-buffer display behavior on reconnect.
(erc-setup-buffer): Use option `erc-reconnect-buffer' if warranted.
(erc-connection-established): Record reconnect count in internal var
before resetting.
---
 lisp/erc/erc-backend.el |  3 +++
 lisp/erc/erc.el         | 23 +++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 69f63dfbc4..0d586268e9 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -193,6 +193,9 @@ erc-server-connected
 (defvar-local erc-server-reconnect-count 0
   "Number of times we have failed to reconnect to the current server.")
 
+(defvar-local erc--server-last-reconnect-count 0
+  "Snapshot of reconnect count when the connection was established.")
+
 (defvar-local erc-server-quitting nil
   "Non-nil if the user requests a quit.")
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index c5a4fbe5a0..01be8ed397 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1521,6 +1521,21 @@ erc-join-buffer
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
+(defcustom erc-reconnect-display nil
+  "How (and whether) to display a channel buffer upon reconnecting.
+
+This only affects automatic reconnections and is ignored when issuing a
+/reconnect command or reinvoking `erc-tls' with the same args (assuming
+success, of course).  See `erc-join-buffer' for a description of
+possible values."
+  :group 'erc-buffers
+  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
+                 (const :tag "Split window and select" window)
+                 (const :tag "Split window, don't select" window-noselect)
+                 (const :tag "New frame" frame)
+                 (const :tag "Bury in new buffer" bury)
+                 (const :tag "Use current buffer" buffer)))
+
 (defcustom erc-frame-alist nil
   "Alist of frame parameters for creating erc frames.
 A value of nil means to use `default-frame-alist'."
@@ -1950,7 +1965,10 @@ erc-update-modules
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
-  (pcase erc-join-buffer
+  (pcase (if (zerop (erc-with-server-buffer
+                      erc--server-last-reconnect-count))
+             erc-join-buffer
+           (or erc-reconnect-display erc-join-buffer))
     ('window
      (if (active-minibuffer-window)
          (display-buffer buffer)
@@ -4722,7 +4740,8 @@ erc-connection-established
             (nick (car (erc-response.command-args parsed)))
             (buffer (process-buffer proc)))
         (setq erc-server-connected t)
-	(setq erc-server-reconnect-count 0)
+        (setq erc--server-last-reconnect-count erc-server-reconnect-count
+              erc-server-reconnect-count 0)
         (erc-update-mode-line)
         (erc-set-initial-user-mode nick buffer)
         (erc-server-setup-periodical-ping buffer)
-- 
2.31.1

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

It just occurred to me that we may also want to reset
`erc--server-last-reconnect-count' at some point after all (re)JOINing
is done with (both client- and server-initiated) so that the option
`erc-join-buffer' regains precedence over `erc-reconnect-display' for
the remainder of the connection. Doing so in `erc-cmd-JOIN' would cover
manually issued /JOINs as well as invocations of `erc-join-channel'.

bug#51753: ERC switches to channel buffer on reconnect

[Stefan Kangas]

"J.P." <jp@neverwas.me> writes:

> It just occurred to me that we may also want to reset
> `erc--server-last-reconnect-count' at some point after all (re)JOINing
> is done with (both client- and server-initiated) so that the option
> `erc-join-buffer' regains precedence over `erc-reconnect-display' for
> the remainder of the connection. Doing so in `erc-cmd-JOIN' would cover
> manually issued /JOINs as well as invocations of `erc-join-channel'.

Sounds good to me.

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

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

Stefan Kangas <stefan@marxist.se> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> It just occurred to me that we may also want to reset
>> `erc--server-last-reconnect-count' at some point after all (re)JOINing
>> is done with (both client- and server-initiated) so that the option
>> `erc-join-buffer' regains precedence over `erc-reconnect-display' for
>> the remainder of the connection. Doing so in `erc-cmd-JOIN' would cover
>> manually issued /JOINs as well as invocations of `erc-join-channel'.
>
> Sounds good to me.

Great. Added. Thanks.


Updated tests: https://gitlab.com/jpneverwas/erc-v3/-/blob/454ef4674af3d784bb47a81db5e7d526fb30a705/test/erc-scenarios.el#L1642



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v2-v3.diff --]
[-- Type: text/x-patch, Size: 846 bytes --]

From a9c084d4dbfad1e34e23f1d2451a34beadfdc258 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sun, 21 Nov 2021 00:40:31 -0800
Subject: NOT A PATCH

F. Jason Park (1):
  Customize displaying of ERC buffers on reconnect

 lisp/erc/erc-backend.el |  3 +++
 lisp/erc/erc.el         | 24 ++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

Interdiff:
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 01be8ed397..181da76f8e 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -3243,6 +3243,7 @@ erc-cmd-JOIN
             (switch-to-buffer (if (get-buffer chnl-name)
                                   chnl-name
                                 (concat chnl-name "/" server)))
+          (setq erc--server-last-reconnect-count 0)
 	  (erc-server-join-channel server chnl key)))))
   t)
 
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Customize-displaying-of-ERC-buffers-on-reconnect.patch --]
[-- Type: text/x-patch, Size: 3764 bytes --]

From a9c084d4dbfad1e34e23f1d2451a34beadfdc258 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Thu, 18 Nov 2021 23:39:54 -0800
Subject: [PATCH 1/1] Customize displaying of ERC buffers on reconnect

* lisp/erc/erc-backend.el (erc--server-last-reconnect-count):
Add variable to record last reconnect tally.

* lisp/erc/erc.el (erc-reconnect-buffer): Add option to specify
channel-buffer display behavior on reconnect.
(erc-setup-buffer): Use option `erc-reconnect-buffer' if warranted.
(erc-connection-established): Record reconnect count in internal var
before resetting.
(erc-cmd-JOIN): Forget last reconnect count when issuing a manual
/JOIN command.
---
 lisp/erc/erc-backend.el |  3 +++
 lisp/erc/erc.el         | 24 ++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 69f63dfbc4..0d586268e9 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -193,6 +193,9 @@ erc-server-connected
 (defvar-local erc-server-reconnect-count 0
   "Number of times we have failed to reconnect to the current server.")
 
+(defvar-local erc--server-last-reconnect-count 0
+  "Snapshot of reconnect count when the connection was established.")
+
 (defvar-local erc-server-quitting nil
   "Non-nil if the user requests a quit.")
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index c5a4fbe5a0..181da76f8e 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1521,6 +1521,21 @@ erc-join-buffer
                  (const :tag "Use current buffer" buffer)
                  (const :tag "Use current buffer" t)))
 
+(defcustom erc-reconnect-display nil
+  "How (and whether) to display a channel buffer upon reconnecting.
+
+This only affects automatic reconnections and is ignored when issuing a
+/reconnect command or reinvoking `erc-tls' with the same args (assuming
+success, of course).  See `erc-join-buffer' for a description of
+possible values."
+  :group 'erc-buffers
+  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
+                 (const :tag "Split window and select" window)
+                 (const :tag "Split window, don't select" window-noselect)
+                 (const :tag "New frame" frame)
+                 (const :tag "Bury in new buffer" bury)
+                 (const :tag "Use current buffer" buffer)))
+
 (defcustom erc-frame-alist nil
   "Alist of frame parameters for creating erc frames.
 A value of nil means to use `default-frame-alist'."
@@ -1950,7 +1965,10 @@ erc-update-modules
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
-  (pcase erc-join-buffer
+  (pcase (if (zerop (erc-with-server-buffer
+                      erc--server-last-reconnect-count))
+             erc-join-buffer
+           (or erc-reconnect-display erc-join-buffer))
     ('window
      (if (active-minibuffer-window)
          (display-buffer buffer)
@@ -3225,6 +3243,7 @@ erc-cmd-JOIN
             (switch-to-buffer (if (get-buffer chnl-name)
                                   chnl-name
                                 (concat chnl-name "/" server)))
+          (setq erc--server-last-reconnect-count 0)
 	  (erc-server-join-channel server chnl key)))))
   t)
 
@@ -4722,7 +4741,8 @@ erc-connection-established
             (nick (car (erc-response.command-args parsed)))
             (buffer (process-buffer proc)))
         (setq erc-server-connected t)
-	(setq erc-server-reconnect-count 0)
+        (setq erc--server-last-reconnect-count erc-server-reconnect-count
+              erc-server-reconnect-count 0)
         (erc-update-mode-line)
         (erc-set-initial-user-mode nick buffer)
         (erc-server-setup-periodical-ping buffer)
-- 
2.31.1

bug#51753: ERC switches to channel buffer on reconnect

[Stefan Kangas]

"J.P." <jp@neverwas.me> writes:

> Great. Added. Thanks.

Amin, what do you think of this patch?

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

These are just some related notes, mostly intended for posterity. Feel
free to ignore. Thanks.


I think at some point it'll be worth exploring the best time and place
for forgetting the current connection's origins with respect to whether
it sprang from an automated reconnect. The latest iteration of this
patch does this via `erc-cmd-JOIN'. But that's a bit of a compromise and
one that will hopefully be replaced with something smarter down the
road.

Ideally, resetting would occur immediately after all reconnect-related
JOINs have happened. As might be obvious, doing this ultimately involves
tracking more state, including but not limited to "JOINedness". In the
case of server-initiated joins, that implies doing so across sessions
[1]. For client-initiated ones, it means tracking request context. While
IRCv3 features like "label responses" were designed specifically to
assist with the latter [2], that doesn't mean we can't do so manually,
given sufficient motivation.

Indeed, when armed with a reliable way to uniquely identify a "logical"
channel subscription across connections [3], it becomes possible to do
this manually by stashing callbacks or continuation instructions. (Some
people may know this as the "command" pattern.) The real question then
becomes whether it's worth the added complexity.

BTW, if anyone has alternative ideas for a temporary solution (other
than `erc-cmd-JOIN'), please speak up. I originally thought about doing
the resetting in a subset of /CMDs originating from typed user input,
but that seemed more prone to inconveniencing users who may have long
incorporated the underlying functions into custom code (firing on
connection) [4]. One way around this would be to only do the resetting
when a user actually enters something at the prompt, but unless you want
that to happen on *any* /CMD, we'd need special handling [5] for the
handful desired (possibly chosen via configurable option).


~~~

[1] A related issue is the means of detecting whether we're joined to a
    channel. With traditional pub/sub services (IRC channels are really
    just "topics"), it's in the client's interest to stay abreast of its
    subscription status. And the service API also typically exposes a
    way for a client to query for this out of band. ERC *does* track a
    channel's JOIN'd state, but it's a bit hampered by legacy features
    related to how targets are currently handled (via
    `erc-default-recipients' and functions that access/mutate it).

[2] https://ircv3.net/irc/#labeled-responses

[3] "Logical" meaning including serially, i.e., spanning disconnects.

[4] In ERC's case, these commands aren't designated as being primarily
    intended for /interactive use (`erc-cmd-JOIN`, being one such
    function, naturally suffers from the same flaw).

[5] Hopefully something more flexible than adding/removing properties,
    like `process-not-needed', which creates problems when trying to
    override /CMDs.

bug#51753: bug#55540: 29.0.50; ERC launches autojoin-channels in current frame instead of original frame

[J.P.]

Just a breadcrumb for posterity: I've opened a related bug that picks up
where this one left off:

  bug#62833: 30.0.50; ERC 5.6: Rethink buffer-display options and behavior

bug#55540: 29.0.50; ERC launches autojoin-channels in current frame instead of original frame

[Pankaj Jangid]

This could be considered a feature request. Because I have not used
Emacs in this way before, so I don’t know what was the behaviour
earlier. Here is what happens,

1. emacs -Q

2. C-x 5 2

3. (setq erc-autojoin-channels-alist '(("libera.chat" "#emacs" "#erc" "#gnus")))

4. M-x erc-tls ; with following parameters - (irc.libera.chat, 6697,
   nickname, password)

5. While it is connecting to the server, switch to the other frame to
   work on other stuff

Result: After the ERC connects to the server, it opens the autojoin
channels in the current frame. Ideally, it should open the channels in
the (dedicated) original frame where ERC was launched.


In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2022-05-20 built on anant
Repository revision: 8a7d8bb04b38ba9849b60d63c9fcfd36856eb424
Repository branch: master
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --prefix=/home/pankaj/.local --with-pgtk'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY
PDUMPER PGTK PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS XIM
GTK3 ZLIB

Important settings:
  value of $LANG: en_IN.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  gnus-undo-mode: t
  editorconfig-mode: t
  which-key-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  override-global-mode: t
  hl-line-mode: t
  savehist-mode: t
  desktop-save-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/pankaj/.emacs.d/elpa/transient-20220514.945/transient hides /home/pankaj/.local/share/emacs/29.0.50/lisp/transient

Features:
(shadow misearch multi-isearch cl-print debug backtrace emacsbug tabify
org-capture org-refile gnus-html help-fns url-cache mailalias smtpmail
flyspell ispell sort smiley gnus-cite mm-archive mail-extr textsec
uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check
gnus-bcklg qp gnus-async gnus-ml nndraft nnmh utf-7 nnml nnfolder
bbdb-gnus bbdb-mua bbdb-com gnus-agent gnus-srvr gnus-score score-mode
nnvirtual gnus-msg nntp gnus-cache .gnus network-stream nsm epa-file
erc-list erc-menu erc-join erc-ring erc-networks erc-pcomplete erc-track
erc-match erc-button erc-fill erc-stamp erc-netsplit erc-goodies erc
erc-backend erc-loaddefs autoload radix-tree gnus-dired diary-lib
diary-loaddefs rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc
rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap sgml-mode facemenu nxml-util nxml-enc
xmltok sql view yaml-mode markdown-mode bug-reference conf-mode ol-eww
eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr
pixel-fill kinsoku url-file url-dired svg dom gnus-group gnus-undo
gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7
netrc nnoo gnus-spec gnus-int gnus-range gnus-win gnus nnheader range
ol-docview doc-view image-mode exif ol-bibtex ol-bbdb ol-w3m ol-doi
org-link-doi sh-script smie executable linum prettier-js js checkdoc
mule-util jka-compr vc-git company-oddmuse company-keywords
company-etags company-gtags company-dabbrev-code company-dabbrev
company-files company-clang company-capf company-cmake company-semantic
company-template company-bbdb company display-line-numbers elec-pair
editorconfig-core editorconfig-core-handle editorconfig-fnmatch init
my-init org-element avl-tree org org-macro org-footnote org-pcomplete
org-list org-faces org-entities org-version ob-plantuml ob-sql ob-css
ob-js ob-java ob-C cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs ob-python python ob-R ob ob-tangle
org-src ob-ref ob-lob ob-table ob-exp ob-comint ob-emacs-lisp ob-core
ob-eval org-table oc-basic bibtex ol org-keys oc org-compat advice
org-macs org-loaddefs cal-menu calendar cal-loaddefs sesman vc
vc-dispatcher clojure-mode lisp-mnt align editorconfig tree-sitter-langs
tree-sitter-langs-build tar-mode arc-mode archive-mode tree-sitter-hl
tree-sitter tree-sitter-load tree-sitter-cli tsc tsc-dyn tsc-dyn-get
dired-aux tsc-obsolete haskell-mode haskell-cabal haskell-utils
haskell-font-lock haskell-indentation haskell-string
haskell-sort-imports haskell-lexeme haskell-align-imports
haskell-complete-module haskell-ghc-support noutline outline
flymake-proc flymake warnings dabbrev haskell-customize go-mode
find-file ffap thingatpt etags fileloop generator gtags-mode xref
project which-key vterm magit-bookmark bookmark tramp tramp-loaddefs
trampver tramp-integration cus-edit pp wid-edit files-x tramp-compat
parse-time iso8601 ls-lisp face-remap compile color term disp-table
ehelp find-func vterm-module term/xterm xterm magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff
smerge-mode diff diff-mode git-commit rx log-edit pcvs-util add-log
magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor shell pcomplete comint ring
server ansi-color magit-mode transient magit-git magit-base
magit-section format-spec crm dash compat-27 compat-26 compat
exec-path-from-shell bbdb-message message sendmail yank-media rmc puny
dired dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068
epg-config gnus-util text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader bbdb bbdb-site
timezone edmacro kmacro modus-vivendi-theme modus-themes pcase delight
cl-extra help-mode use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core hl-line savehist desktop frameset avoid cus-load
finder-inf tex-site info package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core
cl-macs eieio-loaddefs password-cache json map url-vars seq gv subr-x
byte-opt bytecomp byte-compile cconv cl-loaddefs cl-lib early-init
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice
simple cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan
thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help
abbrev obarray oclosure cl-preloaded button loaddefs faces cus-face
macroexp files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo gtk pgtk lcms2 multi-tty make-network-process
emacs)

Memory information:
((conses 16 1240114 103092)
 (symbols 48 56131 26)
 (strings 32 244386 16200)
 (string-bytes 1 7269795)
 (vectors 16 129263)
 (vector-slots 8 2491781 350503)
 (floats 8 663 283)
 (intervals 56 6777 125)
 (buffers 992 118))

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

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

Hi Pankaj,

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> 5. While it is connecting to the server, switch to the other frame to
>    work on other stuff
>
> Result: After the ERC connects to the server, it opens the autojoin
> channels in the current frame. Ideally, it should open the channels in
> the (dedicated) original frame where ERC was launched.

Are you saying ERC ought to remember the frame in which an entry-point
command, like `erc-tls', was originally invoked? Or might it be enough
to make a best effort attempt at finding such a "dedicated" frame?

If it's door #2, what if we used the first frame found containing a
buffer belonging to the same connection (and if no such frame exists,
just leave it up to the display-buffer gods)? This way, we could rely
more fully on window.el facilities and avoid having to wrangle yet more
state. Or, if really necessary, we could maybe fall back on a strategy
that uses `window-prev-buffers' (or whatever) to find the most recently
used frame for that connection (overkill, IMO). Either way, any solution
would need to address not just autojoined channels but also
server-initiated JOINs, PMs, etc.

The attached patch claims to do something like I've described. If you're
able to try it, please set these options beforehand:

  (setq erc-join-buffer 'frame
        erc-auto-query 'frame
        erc-query-display 'frame
        erc-reuse-frames 'displayed)

If you're unable to try it, please let me know, and I can maybe add it
temporarily to one of ERC's installable bug packages.

Thanks,
J.P.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-erc-reuse-frames-to-favor-connections.patch --]
[-- Type: text/x-patch, Size: 9892 bytes --]

From be5ec788ea9fa4375b7d0c96b7d646796daf56d0 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 21 May 2022 00:04:04 -0700
Subject: [PATCH] Allow erc-reuse-frames to favor connections

* lisp/erc/erc.el (erc-reuse-frames): Add alternate value to favor
existing frames already displaying other buffers from the same
connection.
(erc-setup-buffer): Add case for "displayed" variant of
`erc-reuse-frames'.
---
 lisp/erc/erc.el            |  39 ++++++++++---
 test/lisp/erc/erc-tests.el | 115 +++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+), 8 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index ff482d4933..f82fb07a66 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1536,12 +1536,17 @@ erc-frame-dedicated-flag
   :type 'boolean)
 
 (defcustom erc-reuse-frames t
-  "Determines whether new frames are always created.
-Non-nil means that a new frame is not created to display an ERC
-buffer if there is already a window displaying it.  This only has
-effect when `erc-join-buffer' is set to `frame'."
+  "Non-nil means only create a frame for undisplayed buffers.
+For new target buffers, a value of 'displayed' extends this to mean use
+the frame of any buffer from the same server connection, visible or not.
+When this option is nil, a new frame is always created.  Regardless of
+its value, this option is ignored unless `erc-join-buffer' is set to
+`frame'.  Note that like most options in the `erc-buffer' customize
+group, this has no effect on server buffers while reconnecting."
+  :package-version '(ERC . "5.4.1") ; FIXME update when publishing to ELPA
   :group 'erc-buffers
-  :type 'boolean)
+  :type '(choice boolean
+                 (const displayed)))
 
 (defun erc-channel-p (channel)
   "Return non-nil if CHANNEL seems to be an IRC channel name."
@@ -1943,6 +1948,19 @@ erc-update-modules
             (funcall sym 1)
           (error "`%s' is not a known ERC module" mod))))))
 
+(defun erc--setup-buffer-frame-displayed (buffer)
+  "Maybe display BUFFER in an existing frame for the same connection.
+If performed, return window used; otherwise, return nil."
+  (when-let*
+      ((bs (erc-buffer-list nil erc-server-process))
+       (visit (lambda (w) (when (memq (window-buffer w) bs) (throw 'found t))))
+       (test (lambda (fr) (catch 'found (walk-windows visit 0 fr))))
+       ((or (cdr (frame-list)) (funcall test (selected-frame)))))
+    (display-buffer buffer `((display-buffer-use-some-frame)
+                             (inhibit-switch-frame . t)
+                             (inhibit-same-window . t)
+                             (frame-predicate . ,test)))))
+
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
   (pcase erc-join-buffer
@@ -1955,15 +1973,20 @@ erc-setup-buffer
     ('bury
      nil)
     ('frame
-     (when (or (not erc-reuse-frames)
-               (not (get-buffer-window buffer t)))
+     (cond
+      ((and (eq erc-reuse-frames 'displayed)
+            erc-default-recipients ; change this to `erc--target' after bug#48598
+            (not (get-buffer-window buffer t))
+            (erc--setup-buffer-frame-displayed buffer)))
+      ((or (not erc-reuse-frames)
+           (not (get-buffer-window buffer t)))
        (let ((frame (make-frame (or erc-frame-alist
                                     default-frame-alist))))
          (raise-frame frame)
          (select-frame frame))
        (switch-to-buffer buffer)
        (when erc-frame-dedicated-flag
-         (set-window-dedicated-p (selected-window) t))))
+         (set-window-dedicated-p (selected-window) t)))))
     (_
      (if (active-minibuffer-window)
          (display-buffer buffer)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 520f10dd4e..1eac6b22c9 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -171,6 +171,121 @@ erc--switch-to-buffer
     (dolist (b '("server" "other" "#chan" "#foo" "#fake"))
       (kill-buffer b))))
 
+(ert-deftest erc-reuse-frames ()
+  ;; TODO run this in a pseudo terminal subprocess for EMBA
+  (skip-unless (not noninteractive))
+  (should-not erc-frame-dedicated-flag)
+  (let ((erc-join-buffer 'frame)
+        (erc-reuse-frames t)
+        (orig-frame (selected-frame))
+        erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+
+    (ert-info ("Value: t")
+      (with-current-buffer (generate-new-buffer "server")
+        (erc-mode)
+        (setq erc-server-process (start-process "server" (current-buffer)
+                                                "sleep" "1")
+              erc-frame-alist (cons '(name . "server") default-frame-alist))
+        (set-process-query-on-exit-flag erc-server-process nil)
+        (should-not (get-buffer-window (current-buffer) t))
+        (erc-setup-buffer (current-buffer))
+        ;; New frame created and raised
+        (should (equal "server" (frame-parameter (window-frame) 'name)))
+        (should (get-buffer-window (current-buffer) t))
+
+        (with-current-buffer (generate-new-buffer "#chan")
+          (erc-mode)
+          (setq erc-server-process erc-server-process
+                erc-frame-alist (cons '(name . "#chan") default-frame-alist)
+                erc-default-recipients '("#chan"))
+          (should-not (get-buffer-window (current-buffer) t))
+          (erc-setup-buffer (current-buffer))
+          ;; Another frame was created just for #chan
+          (should (equal "#chan" (frame-parameter (window-frame) 'name)))
+          (should (get-buffer-window (current-buffer) t))
+          (delete-frame))
+
+        (select-frame-by-name "server")
+        (pop-to-buffer "#chan")
+        ;; The server frame contains two vertical windows
+        (let ((tree (window-tree)))
+          (should (memq (get-buffer-window "server" t) (car tree)))
+          (should (memq (get-buffer-window "#chan" t) (car tree))))
+        (should (eq (get-buffer "#chan") (window-buffer (selected-window))))
+        (should (eq (get-buffer "server") (window-buffer (next-window))))))
+
+    (ert-info ("Value: displayed, scratch frame selected")
+      (select-frame orig-frame)
+      (with-current-buffer "*scratch*"
+        (with-current-buffer (generate-new-buffer "#spam")
+          (erc-mode)
+          (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                       (get-buffer "server"))
+                erc-reuse-frames 'displayed
+                erc-frame-alist (cons '(name . "#spam") default-frame-alist)
+                erc-default-recipients '("#spam"))
+          (should-not (get-buffer-window (current-buffer) t))
+          (erc-setup-buffer (current-buffer))
+          ;; Window shows up in other frame
+          (should (eq (selected-frame) orig-frame))
+          (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+            (should (equal "server" (frame-parameter fr 'name)))
+            (with-selected-frame fr
+              (should (memq (get-buffer-window "#spam" t)
+                            (car (window-tree))))))))
+
+      (with-current-buffer "server"
+        (ert-info ("Value: displayed, server frame selected")
+          (select-frame-by-name "server")
+          (select-window (get-buffer-window "#spam"))
+          (with-current-buffer (generate-new-buffer "bob")
+            (erc-mode)
+            (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                         (get-buffer "server"))
+                  erc-frame-alist (cons '(name . "bob") default-frame-alist)
+                  erc-default-recipients '("bob"))
+            (should-not (get-buffer-window (current-buffer) t))
+            (erc-setup-buffer (current-buffer))
+            ;; Window shows up in this frame
+            (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+              (should (eq fr (selected-frame)))
+              (should (equal "server" (frame-parameter fr 'name)))
+              (with-selected-frame fr
+                (should (memq (get-buffer-window "bob" t)
+                              (car (window-tree)))))
+              ;; `inhibit-same-window' respected
+              (should-not (eq (get-buffer-window "bob") (selected-window))))))
+
+        (ert-info ("Value: displayed, other frames deleted")
+          (with-selected-frame orig-frame
+            (delete-frame))
+          (should-not (cdr (frame-list)))
+          (select-window (get-buffer-window "bob"))
+          (with-current-buffer (generate-new-buffer "alice")
+            (erc-mode)
+            (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                         (get-buffer "server"))
+                  erc-frame-alist (cons '(name . "alice") default-frame-alist)
+                  erc-default-recipients '("alice"))
+            (should-not (get-buffer-window (current-buffer) t))
+            (erc-setup-buffer (current-buffer))
+            (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+              (should (eq fr (selected-frame)))
+              (should (equal "server" (frame-parameter fr 'name)))
+              (with-selected-frame fr
+                (should (memq (get-buffer-window "alice" t)
+                              (car (window-tree)))))
+              (should-not (eq (get-buffer-window "alice")
+                              (selected-window)))))))))
+
+  (should-not (cdr (frame-list)))
+  (delete-other-windows)
+  (kill-buffer "server")
+  (kill-buffer "bob")
+  (kill-buffer "alice")
+  (kill-buffer "#spam")
+  (kill-buffer "#chan"))
+
 (ert-deftest erc-lurker-maybe-trim ()
   (let (erc-lurker-trim-nicks
         (erc-lurker-ignore-chars "_`"))
-- 
2.36.1

bug#51753: ERC switches to channel buffer on reconnect

[Pankaj Jangid]

"J.P." <jp@neverwas.me> writes:

> Are you saying ERC ought to remember the frame in which an entry-point
> command, like `erc-tls', was originally invoked? Or might it be enough
> to make a best effort attempt at finding such a "dedicated" frame?

I have tried the #2 as specified below. I kind of agree that #1 will
result in the desired behaviour.

> If it's door #2, what if we used the first frame found containing a
> buffer belonging to the same connection (and if no such frame exists,
> just leave it up to the display-buffer gods)? This way, we could rely
> more fully on window.el facilities and avoid having to wrangle yet more
> state. Or, if really necessary, we could maybe fall back on a strategy
> that uses `window-prev-buffers' (or whatever) to find the most recently
> used frame for that connection (overkill, IMO). Either way, any solution
> would need to address not just autojoined channels but also
> server-initiated JOINs, PMs, etc.
>
> The attached patch claims to do something like I've described. If you're
> able to try it, please set these options beforehand:
>
>   (setq erc-join-buffer 'frame
>         erc-auto-query 'frame
>         erc-query-display 'frame
>         erc-reuse-frames 'displayed)
>
> If you're unable to try it, please let me know, and I can maybe add it
> temporarily to one of ERC's installable bug packages.

I tried this and here is what is happening.

I launched emacs then I launched another frame and M-x erc-tls and come
back to the 1st frame for other work. "erc-tls" running fine in the
other frame but when it starts to join autojoin channels after
connecting, it creates one more frame for each channel instead of
reusing the 2nd frame.

bug#51753: ERC switches to channel buffer on reconnect

[J.P.]

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

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> Are you saying ERC ought to remember the frame in which an entry-point
>> command, like `erc-tls', was originally invoked? Or might it be enough
>> to make a best effort attempt at finding such a "dedicated" frame?
>
> I have tried the #2 as specified below. I kind of agree that #1 will
> result in the desired behaviour.

Thanks for trying out that patch. Basically, I bungled a few things that
prevented it from faithfully representing #2, even though I claimed
otherwise (sorry). IOW, I'm saying the approach may be salvageable and
only the implementation flawed.

> I tried this and here is what is happening.
>
> I launched emacs then I launched another frame and M-x erc-tls and come
> back to the 1st frame for other work. "erc-tls" running fine in the
> other frame but when it starts to join autojoin channels after
> connecting, it creates one more frame for each channel instead of
> reusing the 2nd frame.

Right, it seems I blindly fixated on the segregation aspect of your
description (again, apologies) and glossed over the obvious fact that
you don't want ERC creating frames, period. I've adapted the existing
patch with this in mind in hopes you're willing to try again.

One thing to be aware of is that even if we pivot to #1, the
connection-detection and buffer-association stuff will still be
unreliable in many cases. However, that should change after bug#48598
lands. That said, if you still feel strongly about #1, then please say
so (others following along as well). And likewise if you have
suggestions on the implementation or would like to try taking the wheel.
Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0000-v1-v2.diff --]
[-- Type: text/x-patch, Size: 4929 bytes --]

From 7155fbd9f4afe9c9fd9dd2c89e28830a9d6612d7 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 23 May 2022 00:15:18 -0700
Subject: [PATCH 0/1] *** NOT A PATCH ***

*** BLURB HERE ***

F. Jason Park (1):
  Allow erc-reuse-frames to favor connections

 lisp/erc/erc.el            |  52 +++++++++++++---
 test/lisp/erc/erc-tests.el | 119 +++++++++++++++++++++++++++++++++++++
 2 files changed, 163 insertions(+), 8 deletions(-)

Interdiff:
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index f82fb07a66..13ac0ec979 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1538,11 +1538,12 @@ erc-frame-dedicated-flag
 (defcustom erc-reuse-frames t
   "Non-nil means only create a frame for undisplayed buffers.
 For new target buffers, a value of 'displayed' extends this to mean use
-the frame of any buffer from the same server connection, visible or not.
-When this option is nil, a new frame is always created.  Regardless of
-its value, this option is ignored unless `erc-join-buffer' is set to
-`frame'.  Note that like most options in the `erc-buffer' customize
-group, this has no effect on server buffers while reconnecting."
+the frame of any buffer from the same server connection, visible or not,
+or, as a last resort, a frame showing any ERC buffer.  When this option
+is nil, a new frame is always created.  Regardless of its value, this
+option is ignored unless `erc-join-buffer' is set to `frame'.  Note that
+like most options in the `erc-buffer' customize group, this has no
+effect on server buffers while reconnecting."
   :package-version '(ERC . "5.4.1") ; FIXME update when publishing to ELPA
   :group 'erc-buffers
   :type '(choice boolean
@@ -1948,18 +1949,30 @@ erc-update-modules
             (funcall sym 1)
           (error "`%s' is not a known ERC module" mod))))))
 
-(defun erc--setup-buffer-frame-displayed (buffer)
+(defun erc--setup-buffer-first-win (frame a b)
+  (catch 'found
+    (walk-window-tree
+     (lambda (w)
+       (when (eq (buffer-local-value a (window-buffer w)) b)
+         (throw 'found t)))
+     frame nil 0)))
+
+(defun erc--display-buffer-use-some-frame (buffer alist)
   "Maybe display BUFFER in an existing frame for the same connection.
-If performed, return window used; otherwise, return nil."
+If performed, return window used; otherwise, return nil.  Forward ALIST
+to display-buffer machinery."
   (when-let*
-      ((bs (erc-buffer-list nil erc-server-process))
-       (visit (lambda (w) (when (memq (window-buffer w) bs) (throw 'found t))))
-       (test (lambda (fr) (catch 'found (walk-windows visit 0 fr))))
-       ((or (cdr (frame-list)) (funcall test (selected-frame)))))
-    (display-buffer buffer `((display-buffer-use-some-frame)
-                             (inhibit-switch-frame . t)
-                             (inhibit-same-window . t)
-                             (frame-predicate . ,test)))))
+      ((same-proc-p (lambda (fr)
+                      (erc--setup-buffer-first-win fr 'erc-server-process
+                                                   erc-server-process)))
+       (same-mode-p (lambda (fr)
+                      (erc--setup-buffer-first-win fr 'major-mode 'erc-mode)))
+       ((or (cdr (frame-list)) (funcall same-mode-p (selected-frame))))
+       (frame (car (or (filtered-frame-list same-proc-p)
+                       (filtered-frame-list same-mode-p))))
+       (window (get-lru-window frame nil t)))
+    ;; FIXME don't rely on internal window.el function (tab-bar also does it)
+    (window--display-buffer buffer window 'reuse alist)))
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
@@ -1975,9 +1988,9 @@ erc-setup-buffer
     ('frame
      (cond
       ((and (eq erc-reuse-frames 'displayed)
-            erc-default-recipients ; change this to `erc--target' after bug#48598
-            (not (get-buffer-window buffer t))
-            (erc--setup-buffer-frame-displayed buffer)))
+            (not (get-buffer-window buffer t)))
+       (display-buffer buffer `((erc--display-buffer-use-some-frame)
+                                (inhibit-same-window . t))))
       ((or (not erc-reuse-frames)
            (not (get-buffer-window buffer t)))
        (let ((frame (make-frame (or erc-frame-alist
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 1eac6b22c9..686db3bb2e 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -173,6 +173,10 @@ erc--switch-to-buffer
 
 (ert-deftest erc-reuse-frames ()
   ;; TODO run this in a pseudo terminal subprocess for EMBA
+  ;;
+  ;; TODO case that simulates automatic reconnecting, with an
+  ;; existing, unselected frame containing two windows, one with a
+  ;; dead ERC buffer and the other a non-ERC buffer
   (skip-unless (not noninteractive))
   (should-not erc-frame-dedicated-flag)
   (let ((erc-join-buffer 'frame)
-- 
2.36.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Allow-erc-reuse-frames-to-favor-connections.patch --]
[-- Type: text/x-patch, Size: 10576 bytes --]

From 7155fbd9f4afe9c9fd9dd2c89e28830a9d6612d7 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 21 May 2022 00:04:04 -0700
Subject: [PATCH 1/1] Allow erc-reuse-frames to favor connections

* lisp/erc/erc.el (erc-reuse-frames): Add alternate value to favor
existing frames already displaying other buffers from the same
connection.
(erc-setup-buffer): Add case for "displayed" variant of
`erc-reuse-frames'.
---
 lisp/erc/erc.el            |  52 +++++++++++++---
 test/lisp/erc/erc-tests.el | 119 +++++++++++++++++++++++++++++++++++++
 2 files changed, 163 insertions(+), 8 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index ff482d4933..13ac0ec979 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1536,12 +1536,18 @@ erc-frame-dedicated-flag
   :type 'boolean)
 
 (defcustom erc-reuse-frames t
-  "Determines whether new frames are always created.
-Non-nil means that a new frame is not created to display an ERC
-buffer if there is already a window displaying it.  This only has
-effect when `erc-join-buffer' is set to `frame'."
+  "Non-nil means only create a frame for undisplayed buffers.
+For new target buffers, a value of 'displayed' extends this to mean use
+the frame of any buffer from the same server connection, visible or not,
+or, as a last resort, a frame showing any ERC buffer.  When this option
+is nil, a new frame is always created.  Regardless of its value, this
+option is ignored unless `erc-join-buffer' is set to `frame'.  Note that
+like most options in the `erc-buffer' customize group, this has no
+effect on server buffers while reconnecting."
+  :package-version '(ERC . "5.4.1") ; FIXME update when publishing to ELPA
   :group 'erc-buffers
-  :type 'boolean)
+  :type '(choice boolean
+                 (const displayed)))
 
 (defun erc-channel-p (channel)
   "Return non-nil if CHANNEL seems to be an IRC channel name."
@@ -1943,6 +1949,31 @@ erc-update-modules
             (funcall sym 1)
           (error "`%s' is not a known ERC module" mod))))))
 
+(defun erc--setup-buffer-first-win (frame a b)
+  (catch 'found
+    (walk-window-tree
+     (lambda (w)
+       (when (eq (buffer-local-value a (window-buffer w)) b)
+         (throw 'found t)))
+     frame nil 0)))
+
+(defun erc--display-buffer-use-some-frame (buffer alist)
+  "Maybe display BUFFER in an existing frame for the same connection.
+If performed, return window used; otherwise, return nil.  Forward ALIST
+to display-buffer machinery."
+  (when-let*
+      ((same-proc-p (lambda (fr)
+                      (erc--setup-buffer-first-win fr 'erc-server-process
+                                                   erc-server-process)))
+       (same-mode-p (lambda (fr)
+                      (erc--setup-buffer-first-win fr 'major-mode 'erc-mode)))
+       ((or (cdr (frame-list)) (funcall same-mode-p (selected-frame))))
+       (frame (car (or (filtered-frame-list same-proc-p)
+                       (filtered-frame-list same-mode-p))))
+       (window (get-lru-window frame nil t)))
+    ;; FIXME don't rely on internal window.el function (tab-bar also does it)
+    (window--display-buffer buffer window 'reuse alist)))
+
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
   (pcase erc-join-buffer
@@ -1955,15 +1986,20 @@ erc-setup-buffer
     ('bury
      nil)
     ('frame
-     (when (or (not erc-reuse-frames)
-               (not (get-buffer-window buffer t)))
+     (cond
+      ((and (eq erc-reuse-frames 'displayed)
+            (not (get-buffer-window buffer t)))
+       (display-buffer buffer `((erc--display-buffer-use-some-frame)
+                                (inhibit-same-window . t))))
+      ((or (not erc-reuse-frames)
+           (not (get-buffer-window buffer t)))
        (let ((frame (make-frame (or erc-frame-alist
                                     default-frame-alist))))
          (raise-frame frame)
          (select-frame frame))
        (switch-to-buffer buffer)
        (when erc-frame-dedicated-flag
-         (set-window-dedicated-p (selected-window) t))))
+         (set-window-dedicated-p (selected-window) t)))))
     (_
      (if (active-minibuffer-window)
          (display-buffer buffer)
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 520f10dd4e..686db3bb2e 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -171,6 +171,125 @@ erc--switch-to-buffer
     (dolist (b '("server" "other" "#chan" "#foo" "#fake"))
       (kill-buffer b))))
 
+(ert-deftest erc-reuse-frames ()
+  ;; TODO run this in a pseudo terminal subprocess for EMBA
+  ;;
+  ;; TODO case that simulates automatic reconnecting, with an
+  ;; existing, unselected frame containing two windows, one with a
+  ;; dead ERC buffer and the other a non-ERC buffer
+  (skip-unless (not noninteractive))
+  (should-not erc-frame-dedicated-flag)
+  (let ((erc-join-buffer 'frame)
+        (erc-reuse-frames t)
+        (orig-frame (selected-frame))
+        erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+
+    (ert-info ("Value: t")
+      (with-current-buffer (generate-new-buffer "server")
+        (erc-mode)
+        (setq erc-server-process (start-process "server" (current-buffer)
+                                                "sleep" "1")
+              erc-frame-alist (cons '(name . "server") default-frame-alist))
+        (set-process-query-on-exit-flag erc-server-process nil)
+        (should-not (get-buffer-window (current-buffer) t))
+        (erc-setup-buffer (current-buffer))
+        ;; New frame created and raised
+        (should (equal "server" (frame-parameter (window-frame) 'name)))
+        (should (get-buffer-window (current-buffer) t))
+
+        (with-current-buffer (generate-new-buffer "#chan")
+          (erc-mode)
+          (setq erc-server-process erc-server-process
+                erc-frame-alist (cons '(name . "#chan") default-frame-alist)
+                erc-default-recipients '("#chan"))
+          (should-not (get-buffer-window (current-buffer) t))
+          (erc-setup-buffer (current-buffer))
+          ;; Another frame was created just for #chan
+          (should (equal "#chan" (frame-parameter (window-frame) 'name)))
+          (should (get-buffer-window (current-buffer) t))
+          (delete-frame))
+
+        (select-frame-by-name "server")
+        (pop-to-buffer "#chan")
+        ;; The server frame contains two vertical windows
+        (let ((tree (window-tree)))
+          (should (memq (get-buffer-window "server" t) (car tree)))
+          (should (memq (get-buffer-window "#chan" t) (car tree))))
+        (should (eq (get-buffer "#chan") (window-buffer (selected-window))))
+        (should (eq (get-buffer "server") (window-buffer (next-window))))))
+
+    (ert-info ("Value: displayed, scratch frame selected")
+      (select-frame orig-frame)
+      (with-current-buffer "*scratch*"
+        (with-current-buffer (generate-new-buffer "#spam")
+          (erc-mode)
+          (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                       (get-buffer "server"))
+                erc-reuse-frames 'displayed
+                erc-frame-alist (cons '(name . "#spam") default-frame-alist)
+                erc-default-recipients '("#spam"))
+          (should-not (get-buffer-window (current-buffer) t))
+          (erc-setup-buffer (current-buffer))
+          ;; Window shows up in other frame
+          (should (eq (selected-frame) orig-frame))
+          (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+            (should (equal "server" (frame-parameter fr 'name)))
+            (with-selected-frame fr
+              (should (memq (get-buffer-window "#spam" t)
+                            (car (window-tree))))))))
+
+      (with-current-buffer "server"
+        (ert-info ("Value: displayed, server frame selected")
+          (select-frame-by-name "server")
+          (select-window (get-buffer-window "#spam"))
+          (with-current-buffer (generate-new-buffer "bob")
+            (erc-mode)
+            (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                         (get-buffer "server"))
+                  erc-frame-alist (cons '(name . "bob") default-frame-alist)
+                  erc-default-recipients '("bob"))
+            (should-not (get-buffer-window (current-buffer) t))
+            (erc-setup-buffer (current-buffer))
+            ;; Window shows up in this frame
+            (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+              (should (eq fr (selected-frame)))
+              (should (equal "server" (frame-parameter fr 'name)))
+              (with-selected-frame fr
+                (should (memq (get-buffer-window "bob" t)
+                              (car (window-tree)))))
+              ;; `inhibit-same-window' respected
+              (should-not (eq (get-buffer-window "bob") (selected-window))))))
+
+        (ert-info ("Value: displayed, other frames deleted")
+          (with-selected-frame orig-frame
+            (delete-frame))
+          (should-not (cdr (frame-list)))
+          (select-window (get-buffer-window "bob"))
+          (with-current-buffer (generate-new-buffer "alice")
+            (erc-mode)
+            (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                         (get-buffer "server"))
+                  erc-frame-alist (cons '(name . "alice") default-frame-alist)
+                  erc-default-recipients '("alice"))
+            (should-not (get-buffer-window (current-buffer) t))
+            (erc-setup-buffer (current-buffer))
+            (let ((fr (window-frame (get-buffer-window (current-buffer) t))))
+              (should (eq fr (selected-frame)))
+              (should (equal "server" (frame-parameter fr 'name)))
+              (with-selected-frame fr
+                (should (memq (get-buffer-window "alice" t)
+                              (car (window-tree)))))
+              (should-not (eq (get-buffer-window "alice")
+                              (selected-window)))))))))
+
+  (should-not (cdr (frame-list)))
+  (delete-other-windows)
+  (kill-buffer "server")
+  (kill-buffer "bob")
+  (kill-buffer "alice")
+  (kill-buffer "#spam")
+  (kill-buffer "#chan"))
+
 (ert-deftest erc-lurker-maybe-trim ()
   (let (erc-lurker-trim-nicks
         (erc-lurker-ignore-chars "_`"))
-- 
2.36.1