Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for password after other sources, overall simplifications

From: "J.P." <[email-masked]>
Date: Mon, 07 Jun 2021 19:23:39 -0700

Hi Olivier,

I'm sure you know this, but for others, both files have changed since
this patch was created. It applies cleanly for me atop this commit:

  297c0e0306f111c1e7564b2bb49a7e1a925a55bb

Okay, the layout of this module is a bit confusing to me. Perhaps that's
because it's regarded as an entry point of some kind and therefore
special? In the commentary, it says to enable it explicitly using the
minor mode interface (rather than add it to erc-modules). However, I
notice that the :set function for the option erc-nickserv-identify-mode
goes ahead and activates it in all the ways that matter (but the minor-
mode variable).

So no matter what, the function of the same name is called, which then
sets the same custom option (possibly again). I guess that's what that
comment about avoiding "recursive load at startup" is all about? For
now, I'll just assume something fancy's going on I don't yet comprehend.
(Maybe I'll check the old mailing list for clues.)

In general, I'm somewhat inclined to regard this module as nonessential
and legacy focused because it's not loaded by default and because these
days, things seem to be trending toward fewer interactions with nick
services beyond initial setup (where manual piloting is required
anyway). However, I think this module receives a fair amount of
attention on #emacs and elsewhere, so we might as well abide. Because I
don't use it myself, I'll spare you any dubious hands on feedback and
stick to the self-interested stuff affecting those improvements I'd like
to see in ERC for this coming release.

So, despite its specialness, I'm rather confident this module and your
changes to it will be spared the brunt of the library-wide modifications
I have in store. Basically, this would be a reorienting of ERC's notion
of connection identities toward a more network-centric view.

This module already depends on erc-networks, which is good. This means
most of what I'll be tweaking will be auth-source related. But I won't
touch any options concerning the when and the why of it all, which is
what you and Leon have addressed. I'll instead likely only be messing
with the arguments to the one auth-source-search invocation. If you're
interested in details, please follow bug #48598.

A couple specifics. In erc-nickserv-get-password,

    (erc-with-server-buffer
     (setq network erc-network
                   ^~~~~~~~~~~~
           server erc-session-server
           port erc-session-port))

would you mind using the function form of erc-network instead? I'm
focusing a lot on that one symbol in particular, and it'd be nice to
keep things consistent for now, if it's all the same to you.

My other note concerns erc-nickserv-identify. Assuming debug-on-error is
nil, it looks like this dings whenever erc-nickserv-get-password comes
up empty, which I guess can only happen when the three main
password-related user options are all nil (or the prompt gets
dismissed).

So, worst case scenario, people get dinged a few times straight away:
maybe once just after MOTD and once just before, in the case of an
initial re-NICK, and maybe again from a "please identify"/"nick taken"
NOTICE. But being Emacs users, they'd know to check *Messages* for
details (is that the idea?). If there's a realistic chance of a more
intense onslaught, I suppose one alternative might be to print something
to the active buffer using erc-display-error-notice instead. But you
know better than I, having actually used this.

Not sure if you're aware, but there's a bit of an integration going on
between erc-join.el and this module via erc-nickserv-identified-hook.
The autojoin module is pretty confusing, and my current bug addresses
some of that. My question for you is: do networks punish folks for
repeated failed JOIN attempts while unauthed? IOW, any clue whether
major IRCds or service daemons auto-TKLINE (or similar) for such
behavior? If there *is* a risk, I'd rather fix things on the autojoin
side because inhibiting timers during read-passwd would affect PONGs and
the outgoing flood queue, etc.

BTW, have you considered maybe generalizing this entire module (while
preserving the interface) to make it work with *any* services bot, e.g.,
FooServ, and not just nick-related stuff?

So, yeah, now comes the part where I admit to not having actually fired
this up and put it through its paces. But I'm certainly willing to if
you'd like the extra peace of mind. Shouldn't take more than a couple
days (i.e., nanoseconds to the yogi you've surely become by now). Let me
know, and thanks.

J.P.