From: Samuel GROOT <samuel.groot@grenoble-inp.org>
Date: Mon, 30 May 2016 14:01:29 GMT
Subject: Re: [WIP-PATCH 0/2] send-email: refactor the email parser loop
Message-ID: <0abbc11b-5741-ee2e-292c-245c2b974f8e@grenoble-inp.org>
In-Reply-To: <vpq8tyshh8f.fsf@anie.imag.fr>

On 05/29/2016 08:05 PM, Matthieu Moy wrote:
> Samuel GROOT <samuel.groot@grenoble-inp.org> writes:
>
>> Should we take what Eric suggested (see below) as standard output?
>>
>>> Since the headers are already shown after those lines, it's
>>> redundant to have the entire line.  And we could add
>>> trailers after the headers (with a blank line to delimit):
>>>
>>>     # existing header output:
>>>     From: F <F@example.com>
>>>     Cc: A <author@example.com>, One <one@example.com>
>>>     Subject: foo
>>>
>>>     # new trailer output
>>>     Signed-off-by: SoB <SoB@example.com>
>>>     Acked-by: ack <ack@example.com>
>
> I don't think adding the trailers in the output is needed. If the
> message says
>
>   Adding foo@example from Signed-off-by trailer
>
> I can guess that it's from "Signed-off-by: foo@example" without having
> it explicitly.
>
> Perhaps others think differently, but for me, the shortest output would
> be the better (if only to solve the "I never see these lines, they
> scrolled out of my terminal" issue).

I agree, the shorter the better.

> Ideally, I'd even like to shorten the current output a bit more (the
> X-Mailer: header is useless IMHO, and the Date: and Message-id: headers
> are probably not useful enough to be shown by default).

Agreed.

> (Just thinking aloud, obviously none of this should be a prerequisite to
> accept your refactoring patch)
>
>> And keep "(mbox) Adding ..." lines as error output, or maybe
>> displayable by a new option `--verbose`?
>
> I think the "Adding ..." lines make sense by default at least for
> beginners (just a few days ago, I received a bunch of test emails by
> your team follow by a "Oops, I just noticed you got Cc-ed in my tests"
> message ;-), that would probably have been worse without the message).
> There could be an advice.* option to deactivate them, though.

An advice.* option seems a good solution to me.

> The (mbox) prefix doesn't seem useful to me OTOH, I think it's a
> leftover debugging message.

(mbox) prefix was introduced by Ryan Anderson in 2005 (can't find the 
exact commit though), in opposition with the (non-mbox) format ("lots of 
email") that was used before.

Is the "lots of email" format still used?

When adding Cc: from a Signed-off-by: field, (body) prefix is used.