bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict

[Mike Kazantsev <mk.fraggod@gmail.com>]

On Thu, 15 Dec 2022 06:16:08 -0800
"J.P." <jp@neverwas.me> wrote:

> "J.P." <jp@neverwas.me> writes:
> 
> >>> Unfortunately, the best we can do for ERC 5.5 (Emacs 29) is to mention
> >>> somewhere, like in (info "(erc) Network Identifier"), that users really
> >>> worried about this issue should choose an `:id' containing characters
> >>> disallowed in nicks by their network (or just something improbable and
> >>> unlikely to be guessed). But, hopefully, we can address this in a more
> >>> DWIM-like fashion in an upcoming ELPA release, such as ERC 5.6.  
> >>
> >> Hopefully won't be an issue for most people, although it might be not
> >> that uncommon for this kind of service/bot/proxy ircd's to also use
> >> their name for messaging user with any kind of service-related info/issues.  
> >
> > Hm, right. That's good to know. I think the key for now is to make
> > people aware that they should assign an ID or modify
> > `erc-networks-alist' if they find themselves in a similar boat. Most
> > folks just connecting to a public network should be safe because those
> > NETWORKs are mostly titlecase and/or contain a dot. But, in the long
> > run, I'd definitely like the default behavior to account for this
> > possibility.  
> 
> Actually, an egregious oversight has come to light that makes this a
> more general (and more pressing) matter relevant to Emacs 29. Currently,
> renaming queries fails if *any* buffer, even a non-ERC buffer, exists
> whose name matches that of the target in question. And, AFAICT, no
> amount of :id or options twiddling can serve as a workaround. (If this
> is what you've been getting at this whole time, then apologies: I'm
> rather thick headed, if you haven't noticed.)
> 
> Anyhow, I have attempted to address this in the attached patch. If you
> or anyone out there is willing, please install it locally atop the
> current lisp/erc subtree on the emacs-29 branch and see if you can break
> it. Thanks in advance!

I think you indeed found the same thing I was thinking of and fixed it, thanks.

You mentioned connection process and how ID is being set in the previous
message, which indeed didn't seem to be same issues as I've tried to describe
(not clearly enough).

So thought to make a quick mockup of an IRC server and then send it,
with precise commands to run from ERC to reproduce exact problem,
as well actual output from that variable-change-tracking (which shows
how/when ID gets erased after successful connection, and only after
triggering conflicting query-buffer creation).

Haven't gotten around to do it yet though, but pretty sure you found
and fixed same exact issue in 0001-Fix-some-naming-issues-*.patch here,
so there should no longer be any need to do it.

I've just tested removing my custom ID-setting advices/code, using :id
instead, as well as reproducing that exact query-buffer-name conflict,
and it all works without any issues that I can see.
So will use that instead of local code hacks and won't need to worry
about making server-buffer names unique/distinct.


One small thing I've noticed about :id is that it has separate "Network
Identifier" section in "Connecting" info page, which doesn't seem to be linked
directly in its description - not sure if intentional, or maybe an oversight:

  When present, ID should be an opaque object for identifying the
  connection unequivocally.  (In most cases, this would be a string or a
  symbol composed of letters from the Latin alphabet.)  This option is
  generally unneeded, however.  See info node ‘(erc) Connecting’ for use
  cases.  Not available interactively.

Come to think of it, I'd probably also add the most obvious effect of :id
for naming the server buffer, i.e. something like this:

  When present, ID should be an opaque object for identifying the connection
  unequivocally, and will correspond to name of the created erc server buffer
  after connection. ...

Can probably be phrased better, but since main effect of "(erc ...)" command is
creating that server buffer, what it will be named seem to be an important detail
(if only for finding it afterwards).


Pretty sure patches you've attached address main issue I wanted to raise
(as well as couple other issues).

Thanks again for doing all the work here.

And apologies for not being able to articulate main problem more clearly.
Given how simple IRC is, should've definitely attached some easy-to-run
protocol example to reproduce it in the first place (maybe as .eld test-case),
instead of meandering description.


-- 
Mike Kazantsev // fraggod.net