Re: [PATCH 2/3] send-email: only consider lines containing @ or <> for automatic Cc'ing

[Rasmus Villemoes <rv@rasmusvillemoes.dk>]

On 2018-10-11 08:06, Junio C Hamano wrote:
> Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:
> 
>> I considered that (and also had a version where I simply insisted on a @
>> being present), but that means the user no longer would get prompted
>> about the cases where the address was just slightly obfuscated, e.g. the
>>
>> Cc: John Doe <john at doe.com>
>>
>> cases, which would be a regression, I guess. So I do want to pass such
>> cases through, and have them be dealt with when process_address_list
>> gets called.
> 
> We are only tightening with this patch, and we were passing any
> random things through with the original code anyway, so without
> [PATCH 3/3], this step must be making it only better, but I have to
> wonder one thing.
> 
> You keep saying "get prompted" but are we sure we always stop and
> ask (and preferrably---fail and abort when the end user is not
> available at the terminal to interact) when we have such a
> questionable address?
> 

I dunno. I guess I've never considered non-interactive use of
send-email. But the ask() in validate_address does have default q[uit],
which I suppose gets used if stdin is /dev/null? I did do an experiment
adding a bunch of the random odd patterns found in kernel commit
messages to see how send-email reacted before/after this, and the only
things that got filtered away (i.e., no longer prompted about) were
things where the user probably couldn't easily fix it anyway. In the
cases where there was a "Cc: stable" that might be fixed to the proper
stable@vger.kernel.org, the logic in extract_valid_address simply saw
that as a local address, so we didn't use to be prompted, but simply
sent to stable@localhost. Now we simply don't pass that through. So, for
non-interactive use, I guess the effect of this patch is to allow more
cases to complete succesfully, since we filter away (some) cases where
extract_valid_address would cause us to prompt (and thus quit).

So, it seems you're ok with this tightening, but some comment on the
non-interactive use case should be made in the commit log? Or am I
misunderstanding?

Thanks,
Rasmus