Re: Add support for TLS client certificates to 'erc-tls'

[J.P.]

Amin Bandali <bandali@gnu.org> writes:

>> It reconnected successfully with no hiccups, so I think that's one
>> for the win column.

This continues to be the case with the latest changes applied; ditto
when using the authinfo variant of the new :client-certificate param.

>> Beyond that, users may appreciate a mention of the new additions in the
>> info manual and maybe the wiki as well (instead of just NEWS).
>
> Certainly; done in v2.

Nice job. This sort of thing can be a real time sink (at least for me).

One question. And stop me if I've confused myself here, but the doc
string and the tex-info section for `erc-tls' both offer the same
example (borrowed from their plain `erc' counterparts) in which the
function `erc-compute-full-name' is said to be consulted despite the
:full-name "Harry S. Truman" keyword arg being present. While it's
(technically) true that `erc-compute-full-name' always runs, I suspect
its inclusion in the original example was inadvertent (or something
about it has changed since 2007). Would it perhaps make sense to omit
`erc-compute-full-name' from the Truman example?

>> As is, I think it basically enforces non-nil (unless that's the idea).
>
> Indeed :-).  My intention was to enforce non-nil there, to always make
> the connection in an asynchronous/non-blocking way, similar to its dual,
> `erc-open-network-stream'.  I think passing :nowait nil could be useful
> for debugging, but off top of my head I can't think of any other reason
> one would want to do it.  And for debugging, one could easily redefine
> the function completely.  On the other hand, I don't see any harm in
> allowing/respecting :nowait nil if it's explicitly set.  So I've changed
> the above plist-get to plist-member in v2.

Thanks, but I shouldn't have proposed that tweak without a concrete use
case in mind. In fact, I've only ever needed :nowait to be nil when
using a custom opener. If my suggestion made things more confusing or
distracting, my apologies. Perhaps it's best to just revert to what you
had originally or even just force it to t. (Not that you should waste
another second on this relative nothingburger.)

> Thanks for the suggestion.  After some thought, I decided to change the
> interface of erc-tls from the two separate :client-key and :client-crt
> arguments in v1 of the patch to just one :client-certificate in v2,
> matching that of `open-network-stream'.  I tested the change with
> `:client-certificate t' and confirm that it works fine for me.

The interface change is a welcome improvement, IMO. Although dealing
with a list may feel slightly more cumbersome to some, the multiple
helpful examples make that a nonissue. (And sparing contributors an
extra param to worry about is a nice bonus as well.) But I guess the
real win is in accommodating the authinfo facility, which seems a rather
obvious and essential inclusion in retrospect. Great work!