Re: bug#51753: ERC switches to channel buffer on reconnect

[Stefan Kangas]

tags 51753 + patch
thanks

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

> I wasn't sure if that meant I was supposed to work on this. If not,
> please disregard.

You did exactly the right thing here.  Thank you for working on this!

> Otherwise, the tests are in #48598 [1]. As for the name of the option
> itself, I basically punted by going with `erc-reconnect-buffer' to try
> and stay close to `erc-join-buffer'. If that doesn't matter, perhaps
> `erc-reconnect-display' would be a better fit since we already have an
> `erc-query-display' (even though that one's not as closely related).
> Anyone with an opinion, please advise. Thanks.

On balance, I don't have a strong opinion either way.  (I do like
`erc-reconnect-display' slightly better, because many variables that end
with `-buffer' have to do with the _name_ of some buffer.  But it is
true that it is also nice to have a name similar to `erc-join-buffer'.)

I just have some minor nits below:

> +(defvar-local erc--server-last-reconnect-count 0
> +  "Snapshot of reconnect count when connection established.")

"when the connection was established", perhaps?

> +(defcustom erc-reconnect-buffer nil
> +  "How (and whether) to display a channel buffer upon reconnecting.
> +
> +This only affects automatic reconnections and is ignored when issuing a
> +/reconnect command or reinvoking `erc-tls' with the same args (assuming
> +success, of course).  See `erc-join-buffer' for a description of
> +possible values."
> +  :group 'erc-buffers
> +  :type '(choice (const :tag "Use value of `erc-join-buffer'" nil)
> +                 (const :tag "Split window and select" window)
> +                 (const :tag "Split window, don't select" window-noselect)
> +                 (const :tag "New frame" frame)
> +                 (const :tag "Bury in new buffer" bury)
> +                 (const :tag "Use current buffer" buffer)
> +                 (const :tag "Use current buffer" t)))

What is the difference between `buffer' and t?  Should they really have
the same :tag?

If they are just two names for the same thing, I suggest we drop one of
them.

Other than that, LGTM (but I didn't test it).