[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCHv2] erc: fix erc-reuse-buffers behavior
From: |
Basil L. Contovounesios |
Subject: |
Re: [PATCHv2] erc: fix erc-reuse-buffers behavior |
Date: |
Mon, 27 Jul 2020 15:33:43 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
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