bug#51753: ERC switches to channel buffer on reconnect

["J.P." <jp@neverwas.me>]

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