From: "Basil L. Contovounesios" <contovob@tcd.ie>
To: "Mingde (Matthew) Zeng" <matthewzmd@gmail.com>
Cc: bandali@gnu.org, emacs-devel@gnu.org
Subject: Re: [PATCHv2] erc: fix erc-reuse-buffers behavior
Date: Mon, 27 Jul 2020 15:33:43 +0300 [thread overview]
Message-ID: <87o8o1t9w8.fsf@tcd.ie> (raw)
In-Reply-To: <87mu3p4bwm.fsf@gmail.com> (Mingde (Matthew) Zeng's message of "Fri, 24 Jul 2020 03:20:25 -0400")
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
next prev parent reply other threads:[~2020-07-27 12:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-24 7:20 [PATCHv2] erc: fix erc-reuse-buffers behavior Mingde (Matthew) Zeng
2020-07-27 12:33 ` Basil L. Contovounesios [this message]
2020-07-31 14:03 ` Mingde (Matthew) Zeng
-- strict thread matches above, loose matches on Subject: below --
2020-08-11 18:53 Mingde (Matthew) Zeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87o8o1t9w8.fsf@tcd.ie \
--to=contovob@tcd.ie \
--cc=bandali@gnu.org \
--cc=emacs-devel@gnu.org \
--cc=matthewzmd@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).