unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCHv3] erc: fix erc-reuse-buffers behavior
@ 2020-07-31 13:58 Mingde (Matthew) Zeng
  2020-08-03 15:14 ` Amin Bandali
  0 siblings, 1 reply; 7+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-07-31 13:58 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      | 78 +++++++++++++++++++++++++-------------------
 2 files changed, 57 insertions(+), 43 deletions(-)

diff --git a/lisp/erc/erc-join.el b/lisp/erc/erc-join.el
index 280d6bfe0f..3fa6943eba 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 bfe8a2b42e..5675bb8708 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -1606,33 +1606,43 @@ 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))
-                   (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 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))
+                     (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 +3107,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] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-07-31 13:58 [PATCHv3] erc: fix erc-reuse-buffers behavior Mingde (Matthew) Zeng
@ 2020-08-03 15:14 ` Amin Bandali
  2020-08-04  3:00   ` Mingde (Matthew) Zeng
  0 siblings, 1 reply; 7+ messages in thread
From: Amin Bandali @ 2020-08-03 15:14 UTC (permalink / raw)
  To: Mingde (Matthew) Zeng; +Cc: Lars Ingebrigtsen, emacs-devel

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

Thanks for the patch, Matthew.

I just noticed Lars installed 6f94c2405f4c82b63da19de89549aff1fad7e594
on master, changing erc-generate-new-buffer-name.  Will your patch
require adjusting to accommodate Lars's changes?

Minor nits:

- Please omit the "Signed-off-by:" from your commit messages to Emacs.
- Consider dropping the second "* lisp/erc/erc.el" for (erc-cmd-JOIN),
  since the previous entry (erc-generate-new-buffer-name) was also a
  change to lisp/erc/erc.el.

Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-08-03 15:14 ` Amin Bandali
@ 2020-08-04  3:00   ` Mingde (Matthew) Zeng
  2020-08-04  8:30     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-08-04  3:00 UTC (permalink / raw)
  To: Amin Bandali; +Cc: Mingde (Matthew) Zeng, Lars Ingebrigtsen, emacs-devel


Amin Bandali <bandali@gnu.org> writes:

> Thanks for the patch, Matthew.
>
> I just noticed Lars installed 6f94c2405f4c82b63da19de89549aff1fad7e594
> on master, changing erc-generate-new-buffer-name.  Will your patch
> require adjusting to accommodate Lars's changes?

I had a look at Lars' change, that particular commit simply changes target to (not target), it is not enough to fix the problem entirely (as outlined in my commit message). Also, changing to (not target) technically does avoid this specific problem, but imo it logically doesn't make sense. When invoking `erc-generate-new-buffer-name`, target is not an optional parameter and should never be nil, that line is unnecessary amd therefore removed from my commit.

Yes, I could adjust to accommodate Lars' changes, but I would remove the (not target) line, following my original logic, which is the only line Lars changed. Unless I am convinced that (not target) line is necessary to exist.


I have another question, since I've never contributed to Emacs directly before, can you tell me is there any way to quickly find out whether a bug is tracked by GNU bug reports or not?

>
> Minor nits:
>
> - Please omit the "Signed-off-by:" from your commit messages to Emacs.
> - Consider dropping the second "* lisp/erc/erc.el" for (erc-cmd-JOIN),
>   since the previous entry (erc-generate-new-buffer-name) was also a
>   change to lisp/erc/erc.el.

Will fix them in a future patch, after the above is resolved.

>
> Thanks.


--
Mingde (Matthew) Zeng



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-08-04  3:00   ` Mingde (Matthew) Zeng
@ 2020-08-04  8:30     ` Lars Ingebrigtsen
  2020-08-04 17:08       ` Mingde (Matthew) Zeng
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-04  8:30 UTC (permalink / raw)
  To: Mingde (Matthew) Zeng; +Cc: Amin Bandali, emacs-devel

"Mingde (Matthew) Zeng" <matthewzmd@gmail.com> writes:

> I had a look at Lars' change, that particular commit simply changes
> target to (not target), it is not enough to fix the problem entirely
> (as outlined in my commit message). Also, changing to (not target)
> technically does avoid this specific problem, but imo it logically
> doesn't make sense. When invoking `erc-generate-new-buffer-name`,
> target is not an optional parameter and should never be nil, that line
> is unnecessary amd therefore removed from my commit.

The only usage of that function is (indirectly) from here:

(defun erc-open (&optional server port nick full-name
                           connect passwd tgt-list channel process)

[...]

        (buffer (erc-get-buffer-create server port channel))

So I assumed that target could indeed be nil -- especially since that
function has always checked whether target is nil or not, only that the
previous check was reverse of what it should have been.

> I have another question, since I've never contributed to Emacs
> directly before, can you tell me is there any way to quickly find out
> whether a bug is tracked by GNU bug reports or not?

The bug tracker has a search field, if that's what you're asking?

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-08-04  8:30     ` Lars Ingebrigtsen
@ 2020-08-04 17:08       ` Mingde (Matthew) Zeng
  2020-08-04 17:33         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Mingde (Matthew) Zeng @ 2020-08-04 17:08 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Mingde (Matthew) Zeng, Amin Bandali, emacs-devel


Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Mingde (Matthew) Zeng" <matthewzmd@gmail.com> writes:
>
>> I had a look at Lars' change, that particular commit simply changes
>> target to (not target), it is not enough to fix the problem entirely
>> (as outlined in my commit message). Also, changing to (not target)
>> technically does avoid this specific problem, but imo it logically
>> doesn't make sense. When invoking `erc-generate-new-buffer-name`,
>> target is not an optional parameter and should never be nil, that line
>> is unnecessary amd therefore removed from my commit.
>
> The only usage of that function is (indirectly) from here:
>
> (defun erc-open (&optional server port nick full-name
>                            connect passwd tgt-list channel process)
>
> [...]
>
>         (buffer (erc-get-buffer-create server port channel))
>
> So I assumed that target could indeed be nil -- especially since that
> function has always checked whether target is nil or not, only that the
> previous check was reverse of what it should have been.

Okay that sounds right, I'll rebase my patch.

>
>> I have another question, since I've never contributed to Emacs
>> directly before, can you tell me is there any way to quickly find out
>> whether a bug is tracked by GNU bug reports or not?
>
> The bug tracker has a search field, if that's what you're asking?

I see, found it.

Also, is there a way to respond to a bug (i.e  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40121) without subscribing to bug-gnu-emacs? I cannot find a "Comment" button similar to a bug-tracking site like bugzilla (which is what i'm more familiar with). Really like to avoid multiple people working on the same bug in the future.

--
Mingde (Matthew) Zeng



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-08-04 17:08       ` Mingde (Matthew) Zeng
@ 2020-08-04 17:33         ` Lars Ingebrigtsen
  2020-08-04 17:36           ` Noam Postavsky
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-04 17:33 UTC (permalink / raw)
  To: Mingde (Matthew) Zeng; +Cc: Amin Bandali, emacs-devel

"Mingde (Matthew) Zeng" <matthewzmd@gmail.com> writes:

> Also, is there a way to respond to a bug (i.e
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40121) without
> subscribing to bug-gnu-emacs? I cannot find a "Comment" button similar
> to a bug-tracking site like bugzilla (which is what i'm more familiar
> with). Really like to avoid multiple people working on the same bug in
> the future.

No, the only way to comment on a bug is to send an email to
<bug-number>@debbugs.gnu.org.

The Emacs interface to the bug database makes this a bit easier --
install the debbugs-gnu package and use the M-x debbugs-gnu command.

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv3] erc: fix erc-reuse-buffers behavior
  2020-08-04 17:33         ` Lars Ingebrigtsen
@ 2020-08-04 17:36           ` Noam Postavsky
  0 siblings, 0 replies; 7+ messages in thread
From: Noam Postavsky @ 2020-08-04 17:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Mingde (Matthew) Zeng, Amin Bandali, Emacs developers

On Tue, 4 Aug 2020 at 13:33, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
> "Mingde (Matthew) Zeng" <matthewzmd@gmail.com> writes:
>
> > Also, is there a way to respond to a bug (i.e
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=40121) without
> > subscribing to bug-gnu-emacs? I cannot find a "Comment" button similar
> > to a bug-tracking site like bugzilla (which is what i'm more familiar
> > with). Really like to avoid multiple people working on the same bug in
> > the future.
>
> No, the only way to comment on a bug is to send an email to
> <bug-number>@debbugs.gnu.org.

But you don't need to be subscribed to bug-gnu-emacs to do that.



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-08-04 17:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 13:58 [PATCHv3] erc: fix erc-reuse-buffers behavior Mingde (Matthew) Zeng
2020-08-03 15:14 ` Amin Bandali
2020-08-04  3:00   ` Mingde (Matthew) Zeng
2020-08-04  8:30     ` Lars Ingebrigtsen
2020-08-04 17:08       ` Mingde (Matthew) Zeng
2020-08-04 17:33         ` Lars Ingebrigtsen
2020-08-04 17:36           ` Noam Postavsky

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