* [PATCHv2] erc: fix erc-reuse-buffers behavior
@ 2020-08-11 18:53 Mingde (Matthew) Zeng
0 siblings, 0 replies; 4+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-08-11 18:53 UTC (permalink / raw)
To: 40121, emacs-devel; +Cc: larsi
By the definition of erc-reuse-buffers, if non-nil it should create a
new buffer when joining channels with same names on different
servers. The current behavior of erc-reuse-buffers is:
1. when non-nil, it will always reuse the same channel buffer,
resulting in server A's channel gets reconnected to the channel with
the same name of server B.
2. when nil, the buffer-name of the joined channel is
'#channel/server'. However if one tries to '/join #channel' from the
server buffer, it creates a new empty buffer with buffer-name
'#channel', instead of opening the already-joined channel buffer.
This bug is partly documented in bug#40121.
* lisp/erc/erc.el (erc-generate-new-buffer-name): fixes behavior 1,
also determines if the '#channel/server' buffer already exists
and will reuse that buffer when joining on the same
server. Additionally when creating a new buffer with
'#channel/serverB', the existing buffer '#channel' on 'severA' will be
renamed to '#channel/serverA' for the sake of consistency.
(erc-cmd-JOIN): with some refactoring, it ensures
the joined channel buffer is selected and thereby fixes the second
behavior.
* lisp/erc/erc-join.el (erc-autojoin-channels): the logic is
simplified ensuring that when autojoining channels specified in
erc-autojoin-channels-alist, if there exists an erc buffer with the
same channel name but a different server, it will create a new buffer
to join the channel. The current logic is very weak that will skip
joining same channel on different servers altogether.
---
lisp/erc/erc-join.el | 22 ++++++------
lisp/erc/erc.el | 85 +++++++++++++++++++++++++-------------------
2 files changed, 61 insertions(+), 46 deletions(-)
diff --git a/lisp/erc/erc-join.el b/lisp/erc/erc-join.el
index e4faf6bd79..79c111082f 100644
--- a/lisp/erc/erc-join.el
+++ b/lisp/erc/erc-join.el
@@ -153,18 +153,20 @@ This function is run from `erc-nickserv-identified-hook'."
'erc-autojoin-channels-delayed
server nick (current-buffer))))
;; `erc-autojoin-timing' is `connect':
- (dolist (l erc-autojoin-channels-alist)
- (when (string-match (car l) server)
- (let ((server (or erc-session-server erc-server-announced-name)))
+ (let ((server (or erc-session-server erc-server-announced-name)))
+ (dolist (l erc-autojoin-channels-alist)
+ (when (string-match-p (car l) server)
(dolist (chan (cdr l))
- (let ((buffer (erc-get-buffer chan)))
- ;; Only auto-join the channels that we aren't already in
- ;; using a different nick.
+ (let ((buffer
+ (car (erc-buffer-filter
+ (lambda ()
+ (let ((current (erc-default-target)))
+ (and (stringp current)
+ (string-match-p (car l)
+ (or erc-session-server erc-server-announced-name))
+ (string-equal (erc-downcase chan)
+ (erc-downcase current)))))))))
(when (or (not buffer)
- ;; If the same channel is joined on another
- ;; server the best-effort is to just join
- (not (string-match (car l)
- (process-name erc-server-process)))
(not (with-current-buffer buffer
(erc-server-process-alive))))
(erc-server-join-channel server chan))))))))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 404a4c0997..41d7516fbb 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1608,36 +1608,47 @@ symbol, it may have these values:
(defun erc-generate-new-buffer-name (server port target)
"Create a new buffer name based on the arguments."
(when (numberp port) (setq port (number-to-string port)))
- (let ((buf-name (or target
- (or (let ((name (concat server ":" port)))
- (when (> (length name) 1)
- name))
- ;; This fallback should in fact never happen
- "*erc-server-buffer*")))
- buffer-name)
+ (let* ((buf-name (or target
+ (let ((name (concat server ":" port)))
+ (when (> (length name) 1)
+ name))
+ ;; This fallback should in fact never happen.
+ "*erc-server-buffer*"))
+ (full-buf-name (concat buf-name "/" server))
+ (dup-buf-name (buffer-name (car (erc-channel-list nil))))
+ buffer-name)
;; Reuse existing buffers, but not if the buffer is a connected server
;; buffer and not if its associated with a different server than the
;; current ERC buffer.
- ;; if buf-name is taken by a different connection (or by something !erc)
- ;; then see if "buf-name/server" meets the same criteria
- (dolist (candidate (list buf-name (concat buf-name "/" server)))
- (if (and (not buffer-name)
- erc-reuse-buffers
- (or (not (get-buffer candidate))
- ;; Looking for a server buffer, so there's no target.
- (and (not target)
- (with-current-buffer (get-buffer candidate)
- (and (erc-server-buffer-p)
- (not (erc-server-process-alive)))))
- ;; Channel buffer; check that it's from the right server.
- (and target
- (with-current-buffer (get-buffer candidate)
- (and (string= erc-session-server server)
- (erc-port-equal erc-session-port port))))))
- (setq buffer-name candidate)))
- ;; if buffer-name is unset, neither candidate worked out for us,
+ ;; If buf-name is taken by a different connection (or by something !erc)
+ ;; then see if "buf-name/server" meets the same criteria.
+ (if (and dup-buf-name (string-match-p (concat buf-name "/") dup-buf-name))
+ (setq buffer-name full-buf-name) ; ERC buffer with full name already exists.
+ (dolist (candidate (list buf-name full-buf-name))
+ (if (and (not buffer-name)
+ erc-reuse-buffers
+ (or (not (get-buffer candidate))
+ ;; Looking for a server buffer, so there's no target.
+ (and (not target)
+ (with-current-buffer (get-buffer candidate)
+ (and (erc-server-buffer-p)
+ (not (erc-server-process-alive)))))
+ ;; Channel buffer; check that it's from the right server.
+ (and target
+ (with-current-buffer (get-buffer candidate)
+ (and (string= erc-session-server server)
+ (erc-port-equal erc-session-port port))))))
+ (setq buffer-name candidate)
+ (when (and (not buffer-name) (get-buffer buf-name) erc-reuse-buffers)
+ ;; A new buffer will be created with the name buf-name/server, rename
+ ;; the existing name-duplicated buffer with the same format as well.
+ (with-current-buffer (get-buffer buf-name)
+ (when (derived-mode-p 'erc-mode) ; ensure it's an erc buffer
+ (rename-buffer
+ (concat buf-name "/" (or erc-session-server erc-server-announced-name)))))))))
+ ;; If buffer-name is unset, neither candidate worked out for us,
;; fallback to the old <N> uniquification method:
- (or buffer-name (generate-new-buffer-name (concat buf-name "/" server)))))
+ (or buffer-name (generate-new-buffer-name full-buf-name))))
(defun erc-get-buffer-create (server port target)
"Create a new buffer based on the arguments."
@@ -3153,16 +3164,18 @@ were most recently invited. See also `invitation'."
(setq chnl (erc-ensure-channel-name channel)))
(when chnl
;; Prevent double joining of same channel on same server.
- (let ((joined-channels
- (mapcar #'(lambda (chanbuf)
- (with-current-buffer chanbuf (erc-default-target)))
- (erc-channel-list erc-server-process))))
- (if (erc-member-ignore-case chnl joined-channels)
- (switch-to-buffer (car (erc-member-ignore-case chnl
- joined-channels)))
- (let ((server (with-current-buffer (process-buffer erc-server-process)
- (or erc-session-server erc-server-announced-name))))
- (erc-server-join-channel server chnl key))))))
+ (let* ((joined-channels
+ (mapcar #'(lambda (chanbuf)
+ (with-current-buffer chanbuf (erc-default-target)))
+ (erc-channel-list erc-server-process)))
+ (server (with-current-buffer (process-buffer erc-server-process)
+ (or erc-session-server erc-server-announced-name)))
+ (chnl-name (car (erc-member-ignore-case chnl joined-channels))))
+ (if chnl-name
+ (switch-to-buffer (if (get-buffer chnl-name)
+ chnl-name
+ (concat chnl-name "/" server)))
+ (erc-server-join-channel server chnl key)))))
t)
(defalias 'erc-cmd-CHANNEL 'erc-cmd-JOIN)
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCHv2] erc: fix erc-reuse-buffers behavior
@ 2020-07-24 7:20 Mingde (Matthew) Zeng
2020-07-27 12:33 ` Basil L. Contovounesios
0 siblings, 1 reply; 4+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-07-24 7:20 UTC (permalink / raw)
To: emacs-devel; +Cc: bandali
By definition of erc-reuse-buffers, if non-nil it should create a
new buffer when joining channels with same names on different
servers. The current behavior of erc-reuse-buffers is:
1. when non-nil, it will always reuse the same channel buffer,
resulting in server A's channel gets reconnected to the channel with
the same name of server B.
2. when nil, the buffer-name of the joined channel is
'#channel/server'. However if one tries to '/join #channel' from the
server buffer, it creates a new empty buffer with buffer-name
'#channel', instead of opening the already-joined channel buffer.
* lisp/erc/erc.el (erc-generate-new-buffer-name): since `target' is
always non-nil, buffer-name is non-nil, erc-reuse-buffers is t, it
will always use the first candidate, resulting in behavior 1. This is
now fixed, and both channel buffers with the same name will be formated
'#channel/serverA' and '#channel/serverB' for the sake of consistency.
* lisp/erc/erc.el (erc-cmd-JOIN): with some refactoring, it ensures
the joined channel buffer is selected and thereby fixes behavior 2.
* lisp/erc/erc-join.el (erc-autojoin-channels): the logic is
simplified ensuring that when autojoining channels specified in
erc-autojoin-channels-alist, if there exists an erc buffer with the
same channel name but a different server, it will create a new buffer
to join the channel. The current logic is very weak that will skip
joining same channel on different servers altogether.
Signed-off-by: Mingde (Matthew) Zeng <matthewzmd@gmail.com>
---
lisp/erc/erc-join.el | 22 +++++++------
lisp/erc/erc.el | 74 ++++++++++++++++++++++++++------------------
2 files changed, 56 insertions(+), 40 deletions(-)
diff --git a/lisp/erc/erc-join.el b/lisp/erc/erc-join.el
index 280d6bfe0f..c59f7118d1 100644
--- a/lisp/erc/erc-join.el
+++ b/lisp/erc/erc-join.el
@@ -153,18 +153,20 @@ This function is run from `erc-nickserv-identified-hook'."
'erc-autojoin-channels-delayed
server nick (current-buffer))))
;; `erc-autojoin-timing' is `connect':
- (dolist (l erc-autojoin-channels-alist)
- (when (string-match (car l) server)
- (let ((server (or erc-session-server erc-server-announced-name)))
+ (let ((server (or erc-session-server erc-server-announced-name)))
+ (dolist (l erc-autojoin-channels-alist)
+ (when (string-match (car l) server)
(dolist (chan (cdr l))
- (let ((buffer (erc-get-buffer chan)))
- ;; Only auto-join the channels that we aren't already in
- ;; using a different nick.
+ (let ((buffer
+ (car (erc-buffer-filter
+ (lambda ()
+ (let ((current (erc-default-target)))
+ (and (stringp current)
+ (string-match (car l)
+ (or erc-session-server erc-server-announced-name))
+ (string-equal (erc-downcase chan)
+ (erc-downcase current)))))))))
(when (or (not buffer)
- ;; If the same channel is joined on another
- ;; server the best-effort is to just join
- (not (string-match (car l)
- (process-name erc-server-process)))
(not (with-current-buffer buffer
(erc-server-process-alive))))
(erc-server-join-channel server chan))))))))
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 3880778794..2b2cc578d9 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1606,33 +1606,45 @@ symbol, it may have these values:
(defun erc-generate-new-buffer-name (server port target)
"Create a new buffer name based on the arguments."
(when (numberp port) (setq port (number-to-string port)))
- (let ((buf-name (or target
- (or (let ((name (concat server ":" port)))
- (when (> (length name) 1)
- name))
- ;; This fallback should in fact never happen
- "*erc-server-buffer*")))
- buffer-name)
+ (let* ((buf-name (or target
+ (or (let ((name (concat server ":" port)))
+ (when (> (length name) 1)
+ name))
+ ;; This fallback should in fact never happen
+ "*erc-server-buffer*")))
+ (full-buf-name (concat buf-name "/" server))
+ (dup-buf-name (car (mapcar #'(lambda (chanbuf)
+ (buffer-name chanbuf))
+ (erc-channel-list nil))))
+ buffer-name)
;; Reuse existing buffers, but not if the buffer is a connected server
;; buffer and not if its associated with a different server than the
;; current ERC buffer.
;; if buf-name is taken by a different connection (or by something !erc)
;; then see if "buf-name/server" meets the same criteria
- (dolist (candidate (list buf-name (concat buf-name "/" server)))
- (if (and (not buffer-name)
- erc-reuse-buffers
- (or (not (get-buffer candidate))
- (or target
- (with-current-buffer (get-buffer candidate)
- (and (erc-server-buffer-p)
- (not (erc-server-process-alive)))))
- (with-current-buffer (get-buffer candidate)
- (and (string= erc-session-server server)
- (erc-port-equal erc-session-port port)))))
- (setq buffer-name candidate)))
+ (if (and dup-buf-name (string-match (concat buf-name "/") dup-buf-name))
+ (setq buffer-name full-buf-name) ; already exists erc buffer with the full name
+ (dolist (candidate (list buf-name full-buf-name))
+ (if (and (not buffer-name)
+ erc-reuse-buffers
+ (or (not (get-buffer candidate))
+ (with-current-buffer (get-buffer candidate)
+ (and (erc-server-buffer-p)
+ (not (erc-server-process-alive))))
+ (with-current-buffer (get-buffer candidate)
+ (and (string= erc-session-server server)
+ (erc-port-equal erc-session-port port)))))
+ (setq buffer-name candidate)
+ (when (and (not buffer-name) (get-buffer buf-name) erc-reuse-buffers)
+ ;; a new buffer will be created with the name buf-name/server, rename
+ ;; the existing name-duplicated buffer with the same format as well
+ (with-current-buffer (get-buffer buf-name)
+ (when (derived-mode-p 'erc-mode) ; ensure it's an erc buffer
+ (rename-buffer
+ (concat buf-name "/" (or erc-session-server erc-server-announced-name)))))))))
;; if buffer-name is unset, neither candidate worked out for us,
;; fallback to the old <N> uniquification method:
- (or buffer-name (generate-new-buffer-name (concat buf-name "/" server)))))
+ (or buffer-name (generate-new-buffer-name full-buf-name))))
(defun erc-get-buffer-create (server port target)
"Create a new buffer based on the arguments."
@@ -3097,16 +3109,18 @@ were most recently invited. See also `invitation'."
(setq chnl (erc-ensure-channel-name channel)))
(when chnl
;; Prevent double joining of same channel on same server.
- (let ((joined-channels
- (mapcar #'(lambda (chanbuf)
- (with-current-buffer chanbuf (erc-default-target)))
- (erc-channel-list erc-server-process))))
- (if (erc-member-ignore-case chnl joined-channels)
- (switch-to-buffer (car (erc-member-ignore-case chnl
- joined-channels)))
- (let ((server (with-current-buffer (process-buffer erc-server-process)
- (or erc-session-server erc-server-announced-name))))
- (erc-server-join-channel server chnl key))))))
+ (let* ((joined-channels
+ (mapcar #'(lambda (chanbuf)
+ (with-current-buffer chanbuf (erc-default-target)))
+ (erc-channel-list erc-server-process)))
+ (server (with-current-buffer (process-buffer erc-server-process)
+ (or erc-session-server erc-server-announced-name)))
+ (chnl-name (car (erc-member-ignore-case chnl joined-channels))))
+ (if chnl-name
+ (switch-to-buffer (if (get-buffer chnl-name)
+ chnl-name
+ (concat chnl-name "/" server)))
+ (erc-server-join-channel server chnl key)))))
t)
(defalias 'erc-cmd-CHANNEL 'erc-cmd-JOIN)
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2] erc: fix erc-reuse-buffers behavior
2020-07-24 7:20 Mingde (Matthew) Zeng
@ 2020-07-27 12:33 ` Basil L. Contovounesios
2020-07-31 14:03 ` Mingde (Matthew) Zeng
0 siblings, 1 reply; 4+ messages in thread
From: Basil L. Contovounesios @ 2020-07-27 12:33 UTC (permalink / raw)
To: Mingde (Matthew) Zeng; +Cc: bandali, emacs-devel
Just some minor nits from me.
> diff --git a/lisp/erc/erc-join.el b/lisp/erc/erc-join.el
> index 280d6bfe0f..c59f7118d1 100644
> --- a/lisp/erc/erc-join.el
> +++ b/lisp/erc/erc-join.el
> @@ -153,18 +153,20 @@ This function is run from `erc-nickserv-identified-hook'."
> 'erc-autojoin-channels-delayed
> server nick (current-buffer))))
> ;; `erc-autojoin-timing' is `connect':
> - (dolist (l erc-autojoin-channels-alist)
> - (when (string-match (car l) server)
> - (let ((server (or erc-session-server erc-server-announced-name)))
> + (let ((server (or erc-session-server erc-server-announced-name)))
> + (dolist (l erc-autojoin-channels-alist)
> + (when (string-match (car l) server)
Can this and other occurrences of string-match be replaced with string-match-p?
> diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
> index 3880778794..2b2cc578d9 100644
> --- a/lisp/erc/erc.el
> +++ b/lisp/erc/erc.el
> @@ -1606,33 +1606,45 @@ symbol, it may have these values:
> (defun erc-generate-new-buffer-name (server port target)
> "Create a new buffer name based on the arguments."
> (when (numberp port) (setq port (number-to-string port)))
> - (let ((buf-name (or target
> - (or (let ((name (concat server ":" port)))
> - (when (> (length name) 1)
> - name))
> - ;; This fallback should in fact never happen
> - "*erc-server-buffer*")))
> - buffer-name)
> + (let* ((buf-name (or target
> + (or (let ((name (concat server ":" port)))
> + (when (> (length name) 1)
> + name))
> + ;; This fallback should in fact never happen
> + "*erc-server-buffer*")))
Isn't (or A (or B C)) the same as (or A B C)?
Please also end sentences with a full stop.
> + (full-buf-name (concat buf-name "/" server))
> + (dup-buf-name (car (mapcar #'(lambda (chanbuf)
> + (buffer-name chanbuf))
> + (erc-channel-list nil))))
AKA (buffer-name (car (erc-channel-list nil)))?
FWIW, #' is redundant on (lambda ...) in Elisp.
> + buffer-name)
> ;; Reuse existing buffers, but not if the buffer is a connected server
> ;; buffer and not if its associated with a different server than the
> ;; current ERC buffer.
> ;; if buf-name is taken by a different connection (or by something !erc)
> ;; then see if "buf-name/server" meets the same criteria
> - (dolist (candidate (list buf-name (concat buf-name "/" server)))
> - (if (and (not buffer-name)
> - erc-reuse-buffers
> - (or (not (get-buffer candidate))
> - (or target
> - (with-current-buffer (get-buffer candidate)
> - (and (erc-server-buffer-p)
> - (not (erc-server-process-alive)))))
> - (with-current-buffer (get-buffer candidate)
> - (and (string= erc-session-server server)
> - (erc-port-equal erc-session-port port)))))
> - (setq buffer-name candidate)))
> + (if (and dup-buf-name (string-match (concat buf-name "/") dup-buf-name))
> + (setq buffer-name full-buf-name) ; already exists erc buffer with the full name
The comment is a bit unclear - should it instead say
"ERC buffer with the full name already exists."?
> + (dolist (candidate (list buf-name full-buf-name))
> + (if (and (not buffer-name)
> + erc-reuse-buffers
> + (or (not (get-buffer candidate))
> + (with-current-buffer (get-buffer candidate)
> + (and (erc-server-buffer-p)
> + (not (erc-server-process-alive))))
> + (with-current-buffer (get-buffer candidate)
> + (and (string= erc-session-server server)
> + (erc-port-equal erc-session-port port)))))
> + (setq buffer-name candidate)
> + (when (and (not buffer-name) (get-buffer buf-name) erc-reuse-buffers)
> + ;; a new buffer will be created with the name buf-name/server, rename
> + ;; the existing name-duplicated buffer with the same format as well
Ditto re: starting sentences with a capital letter and ending with a
full stop.
Thanks,
--
Basil
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv2] erc: fix erc-reuse-buffers behavior
2020-07-27 12:33 ` Basil L. Contovounesios
@ 2020-07-31 14:03 ` Mingde (Matthew) Zeng
0 siblings, 0 replies; 4+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-07-31 14:03 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Mingde (Matthew) Zeng, bandali, emacs-devel
Thanks for the review! Addressed in my new patch.
> Can this and other occurrences of string-match be replaced with string-match-p?
My new patch only fixed the string-match that I introduced.
I did a search and found *many* string-match in the current codebase. I suggest investigate them further to see if most of them can be replaced with string-match-p in a separate commit.
> FWIW, #' is redundant on (lambda ...) in Elisp.
A new commit could be used to change all #'(lambda ...) to (lambda ...), if it's worth the hassle.
Thanks,
Matthew
--
Mingde (Matthew) Zeng
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-11 18:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-11 18:53 [PATCHv2] erc: fix erc-reuse-buffers behavior Mingde (Matthew) Zeng
-- strict thread matches above, loose matches on Subject: below --
2020-07-24 7:20 Mingde (Matthew) Zeng
2020-07-27 12:33 ` Basil L. Contovounesios
2020-07-31 14:03 ` Mingde (Matthew) Zeng
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).