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

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#55540: 29.0.50; ERC launches autojoin-channels in current frame instead of original frame

[Lars Ingebrigtsen]

Pankaj Jangid <pankaj@codeisgreat.org> writes:

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

This is somewhat related to bug#51753 -- how erc handles these buffer
actions should be changed in general.

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

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

[Pankaj Jangid]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This is somewhat related to bug#51753 -- how erc handles these buffer
> actions should be changed in general.

Thanks. Setting the value of erc-join-buffer to ’bury is a good
workaround. Found in that bug discussion.

What is the control command for marking the bug as duplicate? I couldn’t
locate such a command in "debbugs" package.

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

[Lars Ingebrigtsen]

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> What is the control command for marking the bug as duplicate? I couldn’t
> locate such a command in "debbugs" package.

It's "forcemerge" -- it should be available in the debbugs-gnu package;
it's what I use for merging/closing/reopening bug reports.

-- 
(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: 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

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

[J.P.]

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

Hi Pankaj,

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

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

The changes I was referring to have been on HEAD for over a month now,
but I've been slow in getting back around to this bug (sorry). In case
you or anyone else is interested, I've reworked things a bit to leverage
the new buffer-association stuff, which should make finding a suitable
frame more reliable. You still have to set the options as initially
described up thread. Thanks.


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

From 5098c91eb6176e217f590bfa3da965cbe84653dc Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Wed, 10 Aug 2022 00:40:22 -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            |  61 +++++++-
 test/lisp/erc/erc-tests.el | 294 +++++++++++++++++++++++++++++++++++++
 2 files changed, 348 insertions(+), 7 deletions(-)

Interdiff:
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 79091e18d5..c3bb30368a 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1701,14 +1701,20 @@ erc-frame-dedicated-flag
   :type 'boolean)
 
 (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,
-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."
+  "Determines whether new frames are always created.
+
+A value of t means only create a frame for undisplayed buffers.
+`displayed' means use any existing, potentially hidden frame
+already displaying a buffer from the same network context or,
+failing that, a frame showing any ERC buffer.  As a last resort,
+`displayed' defaults to the selected frame, except for brand new
+connections, for which the invoking frame is always used.  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'.  And like most options in
+the `erc-buffer' customize group, this has no effect on server
+buffers while reconnecting because those are always buried."
   :package-version '(ERC . "5.4.1") ; FIXME update when publishing to ELPA
   :group 'erc-buffers
   :type '(choice boolean
@@ -2143,11 +2149,13 @@ erc-update-modules
             (funcall sym 1)
           (error "`%s' is not a known ERC module" mod))))))
 
-(defun erc--setup-buffer-first-win (frame a b)
+(defun erc--setup-buffer-first-window (frame a b)
   (catch 'found
     (walk-window-tree
      (lambda (w)
-       (when (eq (buffer-local-value a (window-buffer w)) b)
+       (when (cond ((functionp a) (with-current-buffer (window-buffer w)
+                                    (funcall a b)))
+                   (t (eq (buffer-local-value a (window-buffer w)) b)))
          (throw 'found t)))
      frame nil 0)))
 
@@ -2156,17 +2164,19 @@ erc--display-buffer-use-some-frame
 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)))
+      ((idp (lambda (value)
+              (and erc-networks--id
+                   (erc-networks--id-equal-p erc-networks--id value))))
+       (procp (lambda (frame)
+                (erc--setup-buffer-first-window frame idp erc-networks--id)))
+       (ercp (lambda (frame)
+               (erc--setup-buffer-first-window frame 'major-mode 'erc-mode)))
+       ((or (cdr (frame-list)) (funcall ercp (selected-frame)))))
+    ;; Workaround to avoid calling `window--display-buffer' directly
+    (or (display-buffer-use-some-frame buffer
+                                       `((frame-predicate . ,procp) ,@alist))
+        (display-buffer-use-some-frame buffer
+                                       `((frame-predicate . ,ercp) ,@alist)))))
 
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
@@ -2187,6 +2197,7 @@ erc-setup-buffer
       ((and (eq erc-reuse-frames 'displayed)
             (not (get-buffer-window buffer t)))
        (display-buffer buffer '((erc--display-buffer-use-some-frame)
+                                (inhibit-switch-frame . t)
                                 (inhibit-same-window . t))))
       ((or (not erc-reuse-frames)
            (not (get-buffer-window buffer t)))
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 11c501ea84..0986fa6b8f 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -336,124 +336,299 @@ 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)
+(defun erc-tests--run-in-term (&optional debug)
+  (let* ((default-directory (getenv "EMACS_TEST_DIRECTORY"))
+         (emacs (expand-file-name invocation-name invocation-directory))
+         (process-environment (cons "ERC_TESTS_SUBPROCESS=1"
+                                    process-environment))
+         (name (ert-test-name (ert-running-test)))
+         (temp-file (make-temp-file "erc-term-test-"))
+         (cmd `(let ((stats 1))
+                 (setq enable-dir-local-variables nil)
+                 (unwind-protect
+                     (setq stats (ert-run-tests-batch ',name))
+                   (unless ,debug
+                     (let ((buf (with-current-buffer (messages-buffer)
+                                  (buffer-string))))
+                       (with-temp-file ,temp-file
+                         (insert buf)))
+                     (kill-emacs (ert-stats-completed-unexpected stats))))))
+         ;; `ert-test' object in Emacs 29 has a `file-name' field
+         (file-name (symbol-file name 'ert--test))
+         (default-directory (expand-file-name (file-name-directory file-name)))
+         ;; Make subprocess terminal bigger than controlling.
+         (buf (cl-letf (((symbol-function 'window-screen-lines)
+                         (lambda () 20))
+                        ((symbol-function 'window-max-chars-per-line)
+                         (lambda () 40)))
+                (make-term (symbol-name name) emacs nil "-Q" "-nw"
+                           "-L" (file-name-directory (locate-library "erc"))
+                           "-l" file-name "-eval" (format "%S" cmd))))
+         (proc (get-buffer-process buf))
+         (err (lambda ()
+                (with-temp-buffer
+                  (insert-file-contents temp-file)
+                  (message "Subprocess: %s" (buffer-string))
+                  (delete-file temp-file)))))
+    (with-current-buffer buf
+      (set-process-query-on-exit-flag proc nil)
+      (with-timeout (10 (funcall err) (error "Timed out awaiting result"))
+        (while (process-live-p proc)
+          (accept-process-output proc 0.1)))
+      (while (accept-process-output proc))
+      (goto-char (point-min))
+      ;; Otherwise gives process exited abnormally with exit-code >0
+      (unless (search-forward (format "Process %s finished" name) nil t)
+        (funcall err)
+        (ert-fail (when (search-forward "exited" nil t)
+                    (buffer-substring-no-properties (point-at-bol)
+                                                    (point-at-eol)))))
+      (delete-file temp-file)
+      (when noninteractive
+        (kill-buffer)))))
+
+(defun erc-tests--servars (source &rest vars)
+  (unless (bufferp source)
+    (setq source (get-buffer source)))
+  (dolist (var vars)
+    (should (local-variable-if-set-p var))
+    (set var (buffer-local-value var source))))
+
+(defun erc-tests--erc-reuse-frames (test &optional debug)
+  (if (and (or debug noninteractive) (not (getenv "ERC_TESTS_SUBPROCESS")))
+      (progn
+        (when (memq system-type '(windows-nt ms-dos))
+          (ert-skip "System must be UNIX"))
+        (erc-tests--run-in-term debug))
+    (should-not erc-frame-dedicated-flag)
+    (should (eq erc-reuse-frames t))
+    (let ((erc-join-buffer 'frame)
+          (erc-reuse-frames t)
+          (erc-frame-alist nil)
+          (orig-frame (selected-frame))
+          erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+      (delete-other-frames)
+      (delete-other-windows)
+      (set-window-buffer (selected-window) "*scratch*")
+      (funcall test orig-frame)
+      (delete-other-frames orig-frame)
+      (delete-other-windows))))
+
+;; TODO add cases for frame-display behavior while reconnecting
+
+(defun erc-tests--erc-reuse-frames--t (_)
+  (ert-info ("New server buffer creates and raises second frame")
+    (with-current-buffer (generate-new-buffer "server")
+      (erc-mode)
+      (setq erc-server-process (start-process "server"
+                                              (current-buffer) "sleep" "10")
+            erc-frame-alist (cons '(name . "server") default-frame-alist)
+            erc-network 'foonet
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should (equal "server" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))))
 
-    (ert-info ("Value: t")
-      (with-current-buffer (generate-new-buffer "server")
+  (ert-info ("New channel creates and raises third frame")
+    (with-current-buffer (generate-new-buffer "#chan")
+      (erc-mode)
+      (erc-tests--servars "server" 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq 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))
+      (should (equal "#chan" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))
+      (should (cddr (frame-list))))))
+
+(ert-deftest erc-reuse-frames--t ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (erc-tests--erc-reuse-frames--t orig-frame)
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+(defun erc-tests--erc-reuse-frames--displayed-single (_ server-name chan-name)
+
+  (should (eq erc-reuse-frames 'displayed))
+
+  (ert-info ("New server buffer shown in existing frame")
+    (with-current-buffer (generate-new-buffer server-name)
+      (erc-mode)
+      (setq erc-server-process (start-process server-name (current-buffer)
+                                              "sleep" "10")
+            erc-frame-alist (cons `(name . ,server-name) default-frame-alist)
+            erc-network (make-symbol server-name)
+            erc-server-current-nick "tester"
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should-not (equal server-name (frame-parameter (window-frame) 'name)))
+      ;; New server buffer window appears in split below ERT/scratch
+      (should (get-buffer-window (current-buffer) t))))
+
+  (ert-info ("New channel shown in existing frame")
+    (with-current-buffer (generate-new-buffer chan-name)
+      (erc-mode)
+      (erc-tests--servars server-name 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq erc-frame-alist (cons `(name . ,chan-name) default-frame-alist)
+            erc-default-recipients (list chan-name))
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should-not (equal chan-name (frame-parameter (window-frame) 'name)))
+      ;; New channel buffer replaces server in lower window
+      (should (get-buffer-window (current-buffer) t))
+      (should-not (get-buffer-window server-name t)))))
+
+(ert-deftest erc-reuse-frames--displayed-single ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-single orig-frame
+                                                      "server" "#chan")
+       (should-not (cdr (frame-list))))
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+(defun erc-tests--assert-server-split (buffer-or-name frame-name)
+  ;; Assert current buffer resides on one side of a horizontal split
+  ;; in the "server" frame but is not selected.
+  (let* ((buffer-window (get-buffer-window buffer-or-name t))
+         (buffer-frame (window-frame buffer-window)))
+    (should (equal frame-name (frame-parameter buffer-frame 'name)))
+    (should (memq buffer-window (car-safe (window-tree buffer-frame))))
+    (should-not (eq buffer-window (frame-selected-window)))
+    buffer-frame))
+
+(defun erc-tests--erc-reuse-frames--displayed-double (_)
+  (should (eq erc-reuse-frames 'displayed))
+
+  (make-frame '((name . "other")))
+  (select-frame (make-frame '((name . "server"))) 'no-record)
+  (set-window-buffer (selected-window) "*scratch*") ; invokes `erc'
+
+  ;; A user invokes an entry point and switches immediately to a new
+  ;; frame before autojoin kicks in (bug#55540).
+
+  (ert-info ("New server buffer shown in selected frame")
+    (with-current-buffer (generate-new-buffer "server")
+      (erc-mode)
+      (setq erc-server-process (start-process "server" (current-buffer)
+                                              "sleep" "10")
+            erc-network 'foonet
+            erc-server-current-nick "tester"
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should (equal "server" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))))
+
+  (select-frame-by-name "other")
+
+  (ert-info ("New channel shown in dedicated frame")
+    (with-current-buffer (generate-new-buffer "#chan")
+      (erc-mode)
+      (erc-tests--servars "server" 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq 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))
+      (erc-tests--assert-server-split (current-buffer) "server")
+      ;; New channel buffer replaces server in lower window of other frame
+      (should-not (get-buffer-window "server" t)))))
+
+(ert-deftest erc-reuse-frames--displayed-double ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-double orig-frame))
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+;; If a frame showing ERC buffers exists among other frames, new,
+;; additional connections will use the existing IRC frame.  However,
+;; if two or more frames exist with ERC buffers unique to a particular
+;; connection, the correct frame will be found.
+
+(defun erc-tests--erc-reuse-frames--displayed-full (orig-frame)
+  (erc-tests--erc-reuse-frames--displayed-double orig-frame)
+  ;; Server buffer is not displayed because #chan has replaced it in
+  ;; the "server" frame, which is not selected.
+  (should (equal "other" (frame-parameter (window-frame) 'name)))
+  (erc-tests--erc-reuse-frames--displayed-single orig-frame "ircd" "#spam")
+  (should (equal "other" (frame-parameter (window-frame) 'name)))
+
+  ;; Buffer "#spam" has replaced "ircd", which earlier replaced
+  ;; "#chan" in frame "server".  But this is confusing, so...
+  (ert-info ("Arrange windows for second connection in other frame")
+    (set-window-buffer (selected-window) "ircd")
+    (split-window-below)
+    (set-window-buffer (next-window) "#spam")
+    (should (equal (cddar (window-tree))
+                   (list (get-buffer-window "ircd" t)
+                         (get-buffer-window "#spam" t)))))
+
+  (ert-info ("Arrange windows for first connection in server frame")
+    (select-frame-by-name "server")
+    (set-window-buffer (selected-window) "server")
+    (set-window-buffer (next-window) "#chan")
+    (should (equal (cddar (window-tree))
+                   (list (get-buffer-window "server" t)
+                         (get-buffer-window "#chan" t)))))
+
+  ;; Select original ERT frame
+  (ert-info ("New target for connection server finds appropriate frame")
+    (select-frame orig-frame 'no-record)
+    (with-current-buffer (window-buffer (selected-window))
+      (should (member (buffer-name) '("*ert*" "*scratch*")))
+      (with-current-buffer (generate-new-buffer "alice")
+        (erc-mode)
+        (erc-tests--servars "server" 'erc-server-process 'erc-networks--id)
+        (setq erc-default-recipients '("alice"))
+        (should-not (get-buffer-window (current-buffer) t))
+        (erc-setup-buffer (current-buffer))
+        ;; Window created in frame "server"
+        (should (eq (selected-frame) orig-frame))
+        (erc-tests--assert-server-split (current-buffer) "server"))))
+
+  (ert-info ("New target for connection ircd finds appropriate frame")
+    (select-frame orig-frame 'no-record)
+    (with-current-buffer (window-buffer (selected-window))
+      (should (member (buffer-name) '("*ert*" "*scratch*")))
+      (with-current-buffer (generate-new-buffer "bob")
         (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)
+        (erc-tests--servars "ircd" 'erc-server-process 'erc-networks--id)
+        (setq erc-default-recipients '("bob"))
         (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"))
+        ;; Window created in frame "other"
+        (should (eq (selected-frame) orig-frame))
+        (erc-tests--assert-server-split (current-buffer) "other")))))
+
+(ert-deftest erc-reuse-frames--displayed-full ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-full orig-frame))
+     (dolist (b '("server" "ircd" "bob" "alice" "#spam" "#chan"))
+       (kill-buffer b)))))
 
 (ert-deftest erc-lurker-maybe-trim ()
   (let (erc-lurker-trim-nicks
-- 
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: 19248 bytes --]

From 5098c91eb6176e217f590bfa3da965cbe84653dc Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Sat, 21 May 2022 03: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 buffers from the same connection.
(erc--setup-buffer-first-window, erc--display-buffer-use-some-frame):
Add helpers to support 'display' variant of `erc-resuse-frames'
(erc-reuse-frames, erc-tests--servars, erc-tests--assert-server-split,
erc-tests--erc-reuse-frames, erc-tests--run-in-term): Add test case
and supporting helpers. (Bug#55540)
---
 lisp/erc/erc.el            |  61 +++++++-
 test/lisp/erc/erc-tests.el | 294 +++++++++++++++++++++++++++++++++++++
 2 files changed, 348 insertions(+), 7 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 151d75e7ce..c3bb30368a 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1702,11 +1702,23 @@ erc-frame-dedicated-flag
 
 (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'."
+
+A value of t means only create a frame for undisplayed buffers.
+`displayed' means use any existing, potentially hidden frame
+already displaying a buffer from the same network context or,
+failing that, a frame showing any ERC buffer.  As a last resort,
+`displayed' defaults to the selected frame, except for brand new
+connections, for which the invoking frame is always used.  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'.  And like most options in
+the `erc-buffer' customize group, this has no effect on server
+buffers while reconnecting because those are always buried."
+  :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."
@@ -2137,6 +2149,35 @@ erc-update-modules
             (funcall sym 1)
           (error "`%s' is not a known ERC module" mod))))))
 
+(defun erc--setup-buffer-first-window (frame a b)
+  (catch 'found
+    (walk-window-tree
+     (lambda (w)
+       (when (cond ((functionp a) (with-current-buffer (window-buffer w)
+                                    (funcall a b)))
+                   (t (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*
+      ((idp (lambda (value)
+              (and erc-networks--id
+                   (erc-networks--id-equal-p erc-networks--id value))))
+       (procp (lambda (frame)
+                (erc--setup-buffer-first-window frame idp erc-networks--id)))
+       (ercp (lambda (frame)
+               (erc--setup-buffer-first-window frame 'major-mode 'erc-mode)))
+       ((or (cdr (frame-list)) (funcall ercp (selected-frame)))))
+    ;; Workaround to avoid calling `window--display-buffer' directly
+    (or (display-buffer-use-some-frame buffer
+                                       `((frame-predicate . ,procp) ,@alist))
+        (display-buffer-use-some-frame buffer
+                                       `((frame-predicate . ,ercp) ,@alist)))))
+
 (defun erc-setup-buffer (buffer)
   "Consults `erc-join-buffer' to find out how to display `BUFFER'."
   (pcase (if (zerop (erc-with-server-buffer
@@ -2152,15 +2193,21 @@ 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-switch-frame . t)
+                                (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 0f222edacf..0986fa6b8f 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -336,6 +336,300 @@ erc--switch-to-buffer
     (dolist (b '("server" "other" "#chan" "#foo" "#fake"))
       (kill-buffer b))))
 
+(defun erc-tests--run-in-term (&optional debug)
+  (let* ((default-directory (getenv "EMACS_TEST_DIRECTORY"))
+         (emacs (expand-file-name invocation-name invocation-directory))
+         (process-environment (cons "ERC_TESTS_SUBPROCESS=1"
+                                    process-environment))
+         (name (ert-test-name (ert-running-test)))
+         (temp-file (make-temp-file "erc-term-test-"))
+         (cmd `(let ((stats 1))
+                 (setq enable-dir-local-variables nil)
+                 (unwind-protect
+                     (setq stats (ert-run-tests-batch ',name))
+                   (unless ,debug
+                     (let ((buf (with-current-buffer (messages-buffer)
+                                  (buffer-string))))
+                       (with-temp-file ,temp-file
+                         (insert buf)))
+                     (kill-emacs (ert-stats-completed-unexpected stats))))))
+         ;; `ert-test' object in Emacs 29 has a `file-name' field
+         (file-name (symbol-file name 'ert--test))
+         (default-directory (expand-file-name (file-name-directory file-name)))
+         ;; Make subprocess terminal bigger than controlling.
+         (buf (cl-letf (((symbol-function 'window-screen-lines)
+                         (lambda () 20))
+                        ((symbol-function 'window-max-chars-per-line)
+                         (lambda () 40)))
+                (make-term (symbol-name name) emacs nil "-Q" "-nw"
+                           "-L" (file-name-directory (locate-library "erc"))
+                           "-l" file-name "-eval" (format "%S" cmd))))
+         (proc (get-buffer-process buf))
+         (err (lambda ()
+                (with-temp-buffer
+                  (insert-file-contents temp-file)
+                  (message "Subprocess: %s" (buffer-string))
+                  (delete-file temp-file)))))
+    (with-current-buffer buf
+      (set-process-query-on-exit-flag proc nil)
+      (with-timeout (10 (funcall err) (error "Timed out awaiting result"))
+        (while (process-live-p proc)
+          (accept-process-output proc 0.1)))
+      (while (accept-process-output proc))
+      (goto-char (point-min))
+      ;; Otherwise gives process exited abnormally with exit-code >0
+      (unless (search-forward (format "Process %s finished" name) nil t)
+        (funcall err)
+        (ert-fail (when (search-forward "exited" nil t)
+                    (buffer-substring-no-properties (point-at-bol)
+                                                    (point-at-eol)))))
+      (delete-file temp-file)
+      (when noninteractive
+        (kill-buffer)))))
+
+(defun erc-tests--servars (source &rest vars)
+  (unless (bufferp source)
+    (setq source (get-buffer source)))
+  (dolist (var vars)
+    (should (local-variable-if-set-p var))
+    (set var (buffer-local-value var source))))
+
+(defun erc-tests--erc-reuse-frames (test &optional debug)
+  (if (and (or debug noninteractive) (not (getenv "ERC_TESTS_SUBPROCESS")))
+      (progn
+        (when (memq system-type '(windows-nt ms-dos))
+          (ert-skip "System must be UNIX"))
+        (erc-tests--run-in-term debug))
+    (should-not erc-frame-dedicated-flag)
+    (should (eq erc-reuse-frames t))
+    (let ((erc-join-buffer 'frame)
+          (erc-reuse-frames t)
+          (erc-frame-alist nil)
+          (orig-frame (selected-frame))
+          erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+      (delete-other-frames)
+      (delete-other-windows)
+      (set-window-buffer (selected-window) "*scratch*")
+      (funcall test orig-frame)
+      (delete-other-frames orig-frame)
+      (delete-other-windows))))
+
+;; TODO add cases for frame-display behavior while reconnecting
+
+(defun erc-tests--erc-reuse-frames--t (_)
+  (ert-info ("New server buffer creates and raises second frame")
+    (with-current-buffer (generate-new-buffer "server")
+      (erc-mode)
+      (setq erc-server-process (start-process "server"
+                                              (current-buffer) "sleep" "10")
+            erc-frame-alist (cons '(name . "server") default-frame-alist)
+            erc-network 'foonet
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should (equal "server" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))))
+
+  (ert-info ("New channel creates and raises third frame")
+    (with-current-buffer (generate-new-buffer "#chan")
+      (erc-mode)
+      (erc-tests--servars "server" 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq 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))
+      (should (equal "#chan" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))
+      (should (cddr (frame-list))))))
+
+(ert-deftest erc-reuse-frames--t ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (erc-tests--erc-reuse-frames--t orig-frame)
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+(defun erc-tests--erc-reuse-frames--displayed-single (_ server-name chan-name)
+
+  (should (eq erc-reuse-frames 'displayed))
+
+  (ert-info ("New server buffer shown in existing frame")
+    (with-current-buffer (generate-new-buffer server-name)
+      (erc-mode)
+      (setq erc-server-process (start-process server-name (current-buffer)
+                                              "sleep" "10")
+            erc-frame-alist (cons `(name . ,server-name) default-frame-alist)
+            erc-network (make-symbol server-name)
+            erc-server-current-nick "tester"
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should-not (equal server-name (frame-parameter (window-frame) 'name)))
+      ;; New server buffer window appears in split below ERT/scratch
+      (should (get-buffer-window (current-buffer) t))))
+
+  (ert-info ("New channel shown in existing frame")
+    (with-current-buffer (generate-new-buffer chan-name)
+      (erc-mode)
+      (erc-tests--servars server-name 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq erc-frame-alist (cons `(name . ,chan-name) default-frame-alist)
+            erc-default-recipients (list chan-name))
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should-not (equal chan-name (frame-parameter (window-frame) 'name)))
+      ;; New channel buffer replaces server in lower window
+      (should (get-buffer-window (current-buffer) t))
+      (should-not (get-buffer-window server-name t)))))
+
+(ert-deftest erc-reuse-frames--displayed-single ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-single orig-frame
+                                                      "server" "#chan")
+       (should-not (cdr (frame-list))))
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+(defun erc-tests--assert-server-split (buffer-or-name frame-name)
+  ;; Assert current buffer resides on one side of a horizontal split
+  ;; in the "server" frame but is not selected.
+  (let* ((buffer-window (get-buffer-window buffer-or-name t))
+         (buffer-frame (window-frame buffer-window)))
+    (should (equal frame-name (frame-parameter buffer-frame 'name)))
+    (should (memq buffer-window (car-safe (window-tree buffer-frame))))
+    (should-not (eq buffer-window (frame-selected-window)))
+    buffer-frame))
+
+(defun erc-tests--erc-reuse-frames--displayed-double (_)
+  (should (eq erc-reuse-frames 'displayed))
+
+  (make-frame '((name . "other")))
+  (select-frame (make-frame '((name . "server"))) 'no-record)
+  (set-window-buffer (selected-window) "*scratch*") ; invokes `erc'
+
+  ;; A user invokes an entry point and switches immediately to a new
+  ;; frame before autojoin kicks in (bug#55540).
+
+  (ert-info ("New server buffer shown in selected frame")
+    (with-current-buffer (generate-new-buffer "server")
+      (erc-mode)
+      (setq erc-server-process (start-process "server" (current-buffer)
+                                              "sleep" "10")
+            erc-network 'foonet
+            erc-server-current-nick "tester"
+            erc-networks--id (erc-networks--id-create nil)
+            erc--server-last-reconnect-count 0)
+      (set-process-buffer erc-server-process (current-buffer))
+      (set-process-query-on-exit-flag erc-server-process nil)
+      (should-not (get-buffer-window (current-buffer) t))
+      (erc-setup-buffer (current-buffer))
+      (should (equal "server" (frame-parameter (window-frame) 'name)))
+      (should (get-buffer-window (current-buffer) t))))
+
+  (select-frame-by-name "other")
+
+  (ert-info ("New channel shown in dedicated frame")
+    (with-current-buffer (generate-new-buffer "#chan")
+      (erc-mode)
+      (erc-tests--servars "server" 'erc-server-process 'erc-networks--id
+                          'erc-network)
+      (setq 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))
+      (erc-tests--assert-server-split (current-buffer) "server")
+      ;; New channel buffer replaces server in lower window of other frame
+      (should-not (get-buffer-window "server" t)))))
+
+(ert-deftest erc-reuse-frames--displayed-double ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-double orig-frame))
+     (dolist (b '("server" "#chan"))
+       (kill-buffer b)))))
+
+;; If a frame showing ERC buffers exists among other frames, new,
+;; additional connections will use the existing IRC frame.  However,
+;; if two or more frames exist with ERC buffers unique to a particular
+;; connection, the correct frame will be found.
+
+(defun erc-tests--erc-reuse-frames--displayed-full (orig-frame)
+  (erc-tests--erc-reuse-frames--displayed-double orig-frame)
+  ;; Server buffer is not displayed because #chan has replaced it in
+  ;; the "server" frame, which is not selected.
+  (should (equal "other" (frame-parameter (window-frame) 'name)))
+  (erc-tests--erc-reuse-frames--displayed-single orig-frame "ircd" "#spam")
+  (should (equal "other" (frame-parameter (window-frame) 'name)))
+
+  ;; Buffer "#spam" has replaced "ircd", which earlier replaced
+  ;; "#chan" in frame "server".  But this is confusing, so...
+  (ert-info ("Arrange windows for second connection in other frame")
+    (set-window-buffer (selected-window) "ircd")
+    (split-window-below)
+    (set-window-buffer (next-window) "#spam")
+    (should (equal (cddar (window-tree))
+                   (list (get-buffer-window "ircd" t)
+                         (get-buffer-window "#spam" t)))))
+
+  (ert-info ("Arrange windows for first connection in server frame")
+    (select-frame-by-name "server")
+    (set-window-buffer (selected-window) "server")
+    (set-window-buffer (next-window) "#chan")
+    (should (equal (cddar (window-tree))
+                   (list (get-buffer-window "server" t)
+                         (get-buffer-window "#chan" t)))))
+
+  ;; Select original ERT frame
+  (ert-info ("New target for connection server finds appropriate frame")
+    (select-frame orig-frame 'no-record)
+    (with-current-buffer (window-buffer (selected-window))
+      (should (member (buffer-name) '("*ert*" "*scratch*")))
+      (with-current-buffer (generate-new-buffer "alice")
+        (erc-mode)
+        (erc-tests--servars "server" 'erc-server-process 'erc-networks--id)
+        (setq erc-default-recipients '("alice"))
+        (should-not (get-buffer-window (current-buffer) t))
+        (erc-setup-buffer (current-buffer))
+        ;; Window created in frame "server"
+        (should (eq (selected-frame) orig-frame))
+        (erc-tests--assert-server-split (current-buffer) "server"))))
+
+  (ert-info ("New target for connection ircd finds appropriate frame")
+    (select-frame orig-frame 'no-record)
+    (with-current-buffer (window-buffer (selected-window))
+      (should (member (buffer-name) '("*ert*" "*scratch*")))
+      (with-current-buffer (generate-new-buffer "bob")
+        (erc-mode)
+        (erc-tests--servars "ircd" 'erc-server-process 'erc-networks--id)
+        (setq erc-default-recipients '("bob"))
+        (should-not (get-buffer-window (current-buffer) t))
+        (erc-setup-buffer (current-buffer))
+        ;; Window created in frame "other"
+        (should (eq (selected-frame) orig-frame))
+        (erc-tests--assert-server-split (current-buffer) "other")))))
+
+(ert-deftest erc-reuse-frames--displayed-full ()
+  :tags '(:unstable)
+  (erc-tests--erc-reuse-frames
+   (lambda (orig-frame)
+     (let ((erc-reuse-frames 'displayed))
+       (erc-tests--erc-reuse-frames--displayed-full orig-frame))
+     (dolist (b '("server" "ircd" "bob" "alice" "#spam" "#chan"))
+       (kill-buffer b)))))
+
 (ert-deftest erc-lurker-maybe-trim ()
   (let (erc-lurker-trim-nicks
         (erc-lurker-ignore-chars "_`"))
-- 
2.36.1

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

[Pankaj Jangid]

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

> The changes I was referring to have been on HEAD for over a month now,
> but I've been slow in getting back around to this bug (sorry). In case
> you or anyone else is interested, I've reworked things a bit to leverage
> the new buffer-association stuff, which should make finding a suitable
> frame more reliable. You still have to set the options as initially
> described up thread. Thanks.

I will try this change tomorrow, and report. Thanks for the
updated. Thanks for your work.

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

[Lars Ingebrigtsen]

erc still pops up the buffer by default, I think?  I don't think it
should do that, because it's both pretty annoying and a security
issue -- you may suddenly be typing a password into an erc buffer.

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

[Lars Ingebrigtsen]

erc still pops up the buffer by default, I think?  I don't think it
should do that, because it's both pretty annoying and a security
issue -- you may suddenly be typing a password into an erc buffer.

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

[J.P.]

Hi Lars,

Lars Ingebrigtsen <larsi@gnus.org> writes:

> erc still pops up the buffer by default, I think?  I don't think it
> should do that, because it's both pretty annoying and a security
> issue -- you may suddenly be typing a password into an erc buffer.

Are you concerned about the behavior during initial connections as well
as automatic reconnections? Regardless, as you say, the default value of
`buffer' for `erc-join-buffer' (and `erc-reconnect-display') causes the
buffer in the selected window to be replaced with the
just-(re)initialized ERC buffer.

Also, depending on various factors, values other than `buffer' can
exhibit similar behavior. For example, in a dual-window split, a value
of `window-noselect' can result in a newly (re)joined channel replacing
the buffer in the selected window when the connection's server buffer is
showing in the other window. Plain `window' is much the same, but at
least there's somewhat of an expectation that the selected window's
buffer may change.

Really, the only safe option is `bury' (well, maybe also the proposed
frame stuff in Pankaj's patch, but only when an extra, ERC-specific
frame already exists, which we can't count on). That said, there's no
reason we'd have to stick with existing options/behavior when choosing a
new default. I guess it just comes down to what users want to happen.

If we're talking both `erc-join-buffer' and `erc-reconnect-display', one
idea would be to make `window-noselect' the default but change it to
mean the selected window is always left unmolested, no matter what. The
justification for the breaking change would be that the existing doc
string in

  (const :tag "Split window, don't select" window-noselect)

has always implied as much, namely that a window displaying the new
buffer will always be shown and never selected.

Although, if this only concerns `erc-reconnect-display', we could just
go with `bury' since (among the available options) that best minimizes
the disruption of a reestablished connection.

Hopefully folks have stronger opinions and/or better ideas. Thanks.

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

[J.P.]

Hi Lars,

Lars Ingebrigtsen <larsi@gnus.org> writes:

> erc still pops up the buffer by default, I think?  I don't think it
> should do that, because it's both pretty annoying and a security
> issue -- you may suddenly be typing a password into an erc buffer.

Are you concerned about the behavior during initial connections as well
as automatic reconnections? Regardless, as you say, the default value of
`buffer' for `erc-join-buffer' (and `erc-reconnect-display') causes the
buffer in the selected window to be replaced with the
just-(re)initialized ERC buffer.

Also, depending on various factors, values other than `buffer' can
exhibit similar behavior. For example, in a dual-window split, a value
of `window-noselect' can result in a newly (re)joined channel replacing
the buffer in the selected window when the connection's server buffer is
showing in the other window. Plain `window' is much the same, but at
least there's somewhat of an expectation that the selected window's
buffer may change.

Really, the only safe option is `bury' (well, maybe also the proposed
frame stuff in Pankaj's patch, but only when an extra, ERC-specific
frame already exists, which we can't count on). That said, there's no
reason we'd have to stick with existing options/behavior when choosing a
new default. I guess it just comes down to what users want to happen.

If we're talking both `erc-join-buffer' and `erc-reconnect-display', one
idea would be to make `window-noselect' the default but change it to
mean the selected window is always left unmolested, no matter what. The
justification for the breaking change would be that the existing doc
string in

  (const :tag "Split window, don't select" window-noselect)

has always implied as much, namely that a window displaying the new
buffer will always be shown and never selected.

Although, if this only concerns `erc-reconnect-display', we could just
go with `bury' since (among the available options) that best minimizes
the disruption of a reestablished connection.

Hopefully folks have stronger opinions and/or better ideas. Thanks.

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

[Lars Ingebrigtsen]

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

> Are you concerned about the behavior during initial connections as well
> as automatic reconnections?

Yes.

> Really, the only safe option is `bury' (well, maybe also the proposed
> frame stuff in Pankaj's patch, but only when an extra, ERC-specific
> frame already exists, which we can't count on). That said, there's no
> reason we'd have to stick with existing options/behavior when choosing a
> new default. I guess it just comes down to what users want to happen.

I think the default action should be to do nothing to the window setup
at all, and just add the notification to the mode line.

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

[J.P.]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> Really, the only safe option is `bury' (well, maybe also the proposed
>> frame stuff in Pankaj's patch, but only when an extra, ERC-specific
>> frame already exists, which we can't count on). That said, there's no
>> reason we'd have to stick with existing options/behavior when choosing a
>> new default. I guess it just comes down to what users want to happen.
>
> I think the default action should be to do nothing to the window setup
> at all, and just add the notification to the mode line.

I believe changing the default for `erc-join-buffer' (alone) to `bury'
is the easiest way to achieve that.

Thus, if no one objects within the next week or so, I will add the patch
below or similar to trunk (and what will become ERC 5.5), along with a
related NEWS entry. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Bury-new-ERC-buffers-by-default.patch --]
[-- Type: text/x-patch, Size: 5189 bytes --]

From 8b4edbca2771227d4e6ee464ddd210080339f63b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 6 Sep 2022 19:09:54 -0700
Subject: [PATCH] Bury new ERC buffers by default

* lisp/erc/erc.el (erc-join-buffer): Change default value to `bury'.
* test/lisp/erc/erc-scenarios-base-reconnect.el
(erc-scenarios-common-base-reconnect-options): Update helper to handle
new default value for option `erc-join-buffer'.
(erc-scenarios-base-reconnect-options--buffer): Update and rename
function `erc-scenarios-base-reconnect-options--default'.
(erc-scenarios-base-reconnect-options--default): Update and rename
function `erc-scenarios-base-reconnect-options--bury'.
---
 lisp/erc/erc.el                               |  3 +-
 test/lisp/erc/erc-scenarios-base-reconnect.el | 45 ++++++++++---------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 151d75e7ce..657ef8cdb3 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1651,7 +1651,7 @@ erc-default-port-tls
   "IRC port to use for encrypted connections if it cannot be \
 detected otherwise.")
 
-(defcustom erc-join-buffer 'buffer
+(defcustom erc-join-buffer 'bury
   "Determines how to display a newly created IRC buffer.
 
 The available choices are:
@@ -1662,6 +1662,7 @@ erc-join-buffer
   `bury'            - bury it in a new buffer,
   `buffer'          - in place of the current buffer,
   any other value  - in place of the current buffer."
+  :package-version '(ERC . "5.4.1") ; FIXME increment upon publishing to ELPA
   :group 'erc-buffers
   :type '(choice (const :tag "Split window and select" window)
                  (const :tag "Split window, don't select" window-noselect)
diff --git a/test/lisp/erc/erc-scenarios-base-reconnect.el b/test/lisp/erc/erc-scenarios-base-reconnect.el
index 30d692058d..49298dc594 100644
--- a/test/lisp/erc/erc-scenarios-base-reconnect.el
+++ b/test/lisp/erc/erc-scenarios-base-reconnect.el
@@ -99,10 +99,11 @@ erc-scenarios-common--base-reconnect-options
 
     (funcall test)
 
+    ;; A manual /JOIN command tells ERC we're done auto-reconnecting
     (with-current-buffer "FooNet" (erc-cmd-JOIN "#spam"))
 
-    (erc-d-t-wait-for 5 "Channel #spam shown when autojoined"
-      (eq (window-buffer) (get-buffer "#spam")))
+    (erc-d-t-ensure-for 1 "Newly joined chan ignores `erc-reconnect-display'"
+      (not (eq (window-buffer) (get-buffer "#spam"))))
 
     (ert-info ("Wait for auto reconnect")
       (with-current-buffer erc-server-buffer
@@ -114,43 +115,43 @@ erc-scenarios-common--base-reconnect-options
       (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))
         (funcall expect 10 "her elves come here anon")))))
 
-(ert-deftest erc-scenarios-base-reconnect-options--default ()
+(ert-deftest erc-scenarios-base-reconnect-options--buffer ()
   :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'buffer))
+  (should (eq erc-join-buffer 'bury))
   (should-not erc-reconnect-display)
 
   ;; FooNet (the server buffer) is not switched to because it's
   ;; already current (but not shown) when `erc-open' is called.  See
   ;; related conditional guard towards the end of that function.
 
-  (erc-scenarios-common--base-reconnect-options
-   (lambda ()
-     (pop-to-buffer-same-window "*Messages*")
+  (let ((erc-reconnect-display 'buffer))
+    (erc-scenarios-common--base-reconnect-options
+     (lambda ()
+       (pop-to-buffer-same-window "*Messages*")
 
-     (erc-d-t-ensure-for 1 "Server buffer not shown"
-       (not (eq (window-buffer) (get-buffer "FooNet"))))
+       (erc-d-t-ensure-for 1 "Server buffer not shown"
+         (not (eq (window-buffer) (get-buffer "FooNet"))))
 
-     (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
-       (eq (window-buffer) (get-buffer "#chan"))))))
+       (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
+         (eq (window-buffer) (get-buffer "#chan")))))))
 
-(ert-deftest erc-scenarios-base-reconnect-options--bury ()
+(ert-deftest erc-scenarios-base-reconnect-options--default ()
   :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'buffer))
+  (should (eq erc-join-buffer 'bury))
   (should-not erc-reconnect-display)
 
-  (let ((erc-reconnect-display 'bury))
-    (erc-scenarios-common--base-reconnect-options
+  (erc-scenarios-common--base-reconnect-options
 
-     (lambda ()
-       (pop-to-buffer-same-window "*Messages*")
+   (lambda ()
+     (pop-to-buffer-same-window "*Messages*")
 
-       (erc-d-t-ensure-for 1 "Server buffer not shown"
-         (not (eq (window-buffer) (get-buffer "FooNet"))))
+     (erc-d-t-ensure-for 1 "Server buffer not shown"
+       (not (eq (window-buffer) (get-buffer "FooNet"))))
 
-       (erc-d-t-ensure-for 3 "Channel #chan not shown"
-         (not (eq (window-buffer) (get-buffer "#chan"))))
+     (erc-d-t-ensure-for 3 "Channel #chan not shown"
+       (not (eq (window-buffer) (get-buffer "#chan"))))
 
-       (eq (window-buffer) (messages-buffer))))))
+     (eq (window-buffer) (messages-buffer)))))
 
 ;; Upon reconnecting, playback for channel and target buffers is
 ;; routed correctly.  Autojoin is irrelevant here, but for the
-- 
2.37.2

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

[J.P.]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> Really, the only safe option is `bury' (well, maybe also the proposed
>> frame stuff in Pankaj's patch, but only when an extra, ERC-specific
>> frame already exists, which we can't count on). That said, there's no
>> reason we'd have to stick with existing options/behavior when choosing a
>> new default. I guess it just comes down to what users want to happen.
>
> I think the default action should be to do nothing to the window setup
> at all, and just add the notification to the mode line.

I believe changing the default for `erc-join-buffer' (alone) to `bury'
is the easiest way to achieve that.

Thus, if no one objects within the next week or so, I will add the patch
below or similar to trunk (and what will become ERC 5.5), along with a
related NEWS entry. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Bury-new-ERC-buffers-by-default.patch --]
[-- Type: text/x-patch, Size: 5189 bytes --]

From 8b4edbca2771227d4e6ee464ddd210080339f63b Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 6 Sep 2022 19:09:54 -0700
Subject: [PATCH] Bury new ERC buffers by default

* lisp/erc/erc.el (erc-join-buffer): Change default value to `bury'.
* test/lisp/erc/erc-scenarios-base-reconnect.el
(erc-scenarios-common-base-reconnect-options): Update helper to handle
new default value for option `erc-join-buffer'.
(erc-scenarios-base-reconnect-options--buffer): Update and rename
function `erc-scenarios-base-reconnect-options--default'.
(erc-scenarios-base-reconnect-options--default): Update and rename
function `erc-scenarios-base-reconnect-options--bury'.
---
 lisp/erc/erc.el                               |  3 +-
 test/lisp/erc/erc-scenarios-base-reconnect.el | 45 ++++++++++---------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 151d75e7ce..657ef8cdb3 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1651,7 +1651,7 @@ erc-default-port-tls
   "IRC port to use for encrypted connections if it cannot be \
 detected otherwise.")
 
-(defcustom erc-join-buffer 'buffer
+(defcustom erc-join-buffer 'bury
   "Determines how to display a newly created IRC buffer.
 
 The available choices are:
@@ -1662,6 +1662,7 @@ erc-join-buffer
   `bury'            - bury it in a new buffer,
   `buffer'          - in place of the current buffer,
   any other value  - in place of the current buffer."
+  :package-version '(ERC . "5.4.1") ; FIXME increment upon publishing to ELPA
   :group 'erc-buffers
   :type '(choice (const :tag "Split window and select" window)
                  (const :tag "Split window, don't select" window-noselect)
diff --git a/test/lisp/erc/erc-scenarios-base-reconnect.el b/test/lisp/erc/erc-scenarios-base-reconnect.el
index 30d692058d..49298dc594 100644
--- a/test/lisp/erc/erc-scenarios-base-reconnect.el
+++ b/test/lisp/erc/erc-scenarios-base-reconnect.el
@@ -99,10 +99,11 @@ erc-scenarios-common--base-reconnect-options
 
     (funcall test)
 
+    ;; A manual /JOIN command tells ERC we're done auto-reconnecting
     (with-current-buffer "FooNet" (erc-cmd-JOIN "#spam"))
 
-    (erc-d-t-wait-for 5 "Channel #spam shown when autojoined"
-      (eq (window-buffer) (get-buffer "#spam")))
+    (erc-d-t-ensure-for 1 "Newly joined chan ignores `erc-reconnect-display'"
+      (not (eq (window-buffer) (get-buffer "#spam"))))
 
     (ert-info ("Wait for auto reconnect")
       (with-current-buffer erc-server-buffer
@@ -114,43 +115,43 @@ erc-scenarios-common--base-reconnect-options
       (with-current-buffer (erc-d-t-wait-for 10 (get-buffer "#spam"))
         (funcall expect 10 "her elves come here anon")))))
 
-(ert-deftest erc-scenarios-base-reconnect-options--default ()
+(ert-deftest erc-scenarios-base-reconnect-options--buffer ()
   :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'buffer))
+  (should (eq erc-join-buffer 'bury))
   (should-not erc-reconnect-display)
 
   ;; FooNet (the server buffer) is not switched to because it's
   ;; already current (but not shown) when `erc-open' is called.  See
   ;; related conditional guard towards the end of that function.
 
-  (erc-scenarios-common--base-reconnect-options
-   (lambda ()
-     (pop-to-buffer-same-window "*Messages*")
+  (let ((erc-reconnect-display 'buffer))
+    (erc-scenarios-common--base-reconnect-options
+     (lambda ()
+       (pop-to-buffer-same-window "*Messages*")
 
-     (erc-d-t-ensure-for 1 "Server buffer not shown"
-       (not (eq (window-buffer) (get-buffer "FooNet"))))
+       (erc-d-t-ensure-for 1 "Server buffer not shown"
+         (not (eq (window-buffer) (get-buffer "FooNet"))))
 
-     (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
-       (eq (window-buffer) (get-buffer "#chan"))))))
+       (erc-d-t-wait-for 5 "Channel #chan shown when autojoined"
+         (eq (window-buffer) (get-buffer "#chan")))))))
 
-(ert-deftest erc-scenarios-base-reconnect-options--bury ()
+(ert-deftest erc-scenarios-base-reconnect-options--default ()
   :tags '(:expensive-test)
-  (should (eq erc-join-buffer 'buffer))
+  (should (eq erc-join-buffer 'bury))
   (should-not erc-reconnect-display)
 
-  (let ((erc-reconnect-display 'bury))
-    (erc-scenarios-common--base-reconnect-options
+  (erc-scenarios-common--base-reconnect-options
 
-     (lambda ()
-       (pop-to-buffer-same-window "*Messages*")
+   (lambda ()
+     (pop-to-buffer-same-window "*Messages*")
 
-       (erc-d-t-ensure-for 1 "Server buffer not shown"
-         (not (eq (window-buffer) (get-buffer "FooNet"))))
+     (erc-d-t-ensure-for 1 "Server buffer not shown"
+       (not (eq (window-buffer) (get-buffer "FooNet"))))
 
-       (erc-d-t-ensure-for 3 "Channel #chan not shown"
-         (not (eq (window-buffer) (get-buffer "#chan"))))
+     (erc-d-t-ensure-for 3 "Channel #chan not shown"
+       (not (eq (window-buffer) (get-buffer "#chan"))))
 
-       (eq (window-buffer) (messages-buffer))))))
+     (eq (window-buffer) (messages-buffer)))))
 
 ;; Upon reconnecting, playback for channel and target buffers is
 ;; routed correctly.  Autojoin is irrelevant here, but for the
-- 
2.37.2

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

[Lars Ingebrigtsen]

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

> I believe changing the default for `erc-join-buffer' (alone) to `bury'
> is the easiest way to achieve that.
>
> Thus, if no one objects within the next week or so, I will add the patch
> below or similar to trunk (and what will become ERC 5.5), along with a
> related NEWS entry. Thanks.

Thanks; makes sense to me.

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

[Lars Ingebrigtsen]

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

> I believe changing the default for `erc-join-buffer' (alone) to `bury'
> is the easiest way to achieve that.
>
> Thus, if no one objects within the next week or so, I will add the patch
> below or similar to trunk (and what will become ERC 5.5), along with a
> related NEWS entry. Thanks.

Thanks; makes sense to me.

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

[J.P.]

>> Thus, if no one objects within the next week or so, I will add the patch
>> below or similar to trunk (and what will become ERC 5.5), along with a
>> related NEWS entry. Thanks.

This has been carried out.

Pankaj, all that remains is your frame stuff. If problems persist and/or
you've lost interest, please let us know, so I can act accordingly.
Thanks.

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

[J.P.]

>> Thus, if no one objects within the next week or so, I will add the patch
>> below or similar to trunk (and what will become ERC 5.5), along with a
>> related NEWS entry. Thanks.

This has been carried out.

Pankaj, all that remains is your frame stuff. If problems persist and/or
you've lost interest, please let us know, so I can act accordingly.
Thanks.

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

[Pankaj Jangid]

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

>>> Thus, if no one objects within the next week or so, I will add the patch
>>> below or similar to trunk (and what will become ERC 5.5), along with a
>>> related NEWS entry. Thanks.
>
> This has been carried out.
>
> Pankaj, all that remains is your frame stuff. If problems persist and/or
> you've lost interest, please let us know, so I can act accordingly.
> Thanks.

Now the behaviour is acceptable when "erc-autojoin-channels-alist" is
set to nil. i.e. the frame in which I launched "erc-tls" opens the erc
window instead of occupying the current frame.

But when I have few channels in "erc-autojoin-channels-alist" then the
channels are still openning in the current frame instead of the
dedicated frame that I started for "erc-tls". Is there some setting that
I missed in the conversation for this? My current "erc" related
settings are:

--8<---------------cut here---------------start------------->8---
(setq erc-prompt-for-password nil
      erc-prompt (lambda () (concat "[" (buffer-name) "]")))
(eval-when-compile (require 'erc-services))
(setq erc-prompt-for-nickserv-password nil
      erc-use-auth-source-for-nickserv-password t)
(setq erc-server "irc.libera.chat")
(setq erc-port 6697)
(setq erc-nick "my-nick")
(setq erc-user-full-name "My Full Name")
(eval-when-compile (require 'erc-join))
(setq erc-autojoin-channels-alist
      '(("libera.chat" "#emacs" "#erc" "#gnus")))
--8<---------------cut here---------------end--------------->8---

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

[Pankaj Jangid]

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

>>> Thus, if no one objects within the next week or so, I will add the patch
>>> below or similar to trunk (and what will become ERC 5.5), along with a
>>> related NEWS entry. Thanks.
>
> This has been carried out.
>
> Pankaj, all that remains is your frame stuff. If problems persist and/or
> you've lost interest, please let us know, so I can act accordingly.
> Thanks.

Now the behaviour is acceptable when "erc-autojoin-channels-alist" is
set to nil. i.e. the frame in which I launched "erc-tls" opens the erc
window instead of occupying the current frame.

But when I have few channels in "erc-autojoin-channels-alist" then the
channels are still openning in the current frame instead of the
dedicated frame that I started for "erc-tls". Is there some setting that
I missed in the conversation for this? My current "erc" related
settings are:

--8<---------------cut here---------------start------------->8---
(setq erc-prompt-for-password nil
      erc-prompt (lambda () (concat "[" (buffer-name) "]")))
(eval-when-compile (require 'erc-services))
(setq erc-prompt-for-nickserv-password nil
      erc-use-auth-source-for-nickserv-password t)
(setq erc-server "irc.libera.chat")
(setq erc-port 6697)
(setq erc-nick "my-nick")
(setq erc-user-full-name "My Full Name")
(eval-when-compile (require 'erc-join))
(setq erc-autojoin-channels-alist
      '(("libera.chat" "#emacs" "#erc" "#gnus")))
--8<---------------cut here---------------end--------------->8---

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

[J.P.]

Hi Pankaj,

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> Now the behaviour is acceptable when "erc-autojoin-channels-alist" is
> set to nil. i.e. the frame in which I launched "erc-tls" opens the erc
> window instead of occupying the current frame.

Hm, right, but I think that was always the baseline behavior, no? If
not, I wonder if the recent change to the default for `erc-join-buffer'
somehow affected this.

> But when I have few channels in "erc-autojoin-channels-alist" then the
> channels are still openning in the current frame instead of the
> dedicated frame that I started for "erc-tls". Is there some setting
> that I missed in the conversation for this?

Looks like it, but I should have re-summarized either way. So that's my
bad.

> (setq erc-prompt-for-nickserv-password nil
>       erc-use-auth-source-for-nickserv-password t)

This is unrelated (so feel free to skip ahead), but are you sure
auth-source is even being consulted here? I ask because `services' isn't
a default module, and your snippet doesn't modify `erc-modules', AFAICT.
Regardless, adding services to the repro mix at this point would only
complicate matters, so please forget I said anything. I mean, if really
necessary, you can just use Libera's server-password kludge while
testing (everything will still be encrypted):

  (erc-tls :password "$myaccount:$mypass" ...)

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

Ah, looks like we're missing

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

Also, you *did* apply the patch and rerun make (or at least delete the
.elc), right?:

  https://lists.gnu.org/archive/html/emacs-erc/2022-08/txtAqY2ukHPun.txt

(Yeah, those changes aren't on trunk.) Anyway, please let me know if
something doesn't add up.

Thanks,
J.P.

P.S. If you want, you can use the symbol `Libera.Chat' instead of the
string "libera.chat" in `erc-autojoin-channels-alist'.

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

[J.P.]

I've added a version of the frames patch as well as an option to control
buffer-display behavior for interactive entry-point invocations.

  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0e4c07dc
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3a012d1d

Thanks and closing.

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

[J.P.]

I've added a version of the frames patch as well as an option to control
buffer-display behavior for interactive entry-point invocations.

  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0e4c07dc
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3a012d1d

Thanks and closing.

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

[J.P.]

Emanuel Berg <incal@dataswamp.org> writes:

> J.P. wrote:
>
>> I believe changing the default for `erc-join-buffer' (alone)
>> to `bury' is the easiest way to achieve that.
>
> `erc-join-buffer' sounds like a command/function BTW.

Indeed. And the "join" part makes it sound like it's only supposed to
affect channels. Therefore, I think this should be aliased to something
like `erc-buffer-display' because it also determines how server buffers
are shown. Doing so would maybe also aid in discoverability by bringing
it more in line with `erc-query-display' and `erc-reconnect-display'.
The same goes for `erc-auto-query', which could likewise become
`erc-receive-query-display' or similar.

We should also probably mention somewhere that the new default value of
`bury' doesn't put existing buffers at the end of the buffer list.
Moreover, I'm thinking that having `bury' as the default for interactive
invocations (which is now the case) may be too stark a proposition since
the only visual feedback afforded new users is a blip in the mode line.
(FWIW, one user on Libera has recently complained about this, going so
far as to label it a bug.) Anyway, the "penultimate" patch in the POC
portion of the set proposed in bug#60428 [1] incorporates some of these
ideas. If anyone's interested, please check it out. Thanks.

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60428

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