Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages

[Junio C Hamano <gitster@pobox.com>]

"Jay Soffian" <jaysoffian@gmail.com> writes:

> On Feb 16, 2008 1:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Well, according to the RFC, that shouldn't be the case. The only
> lost information should be trailing whitespace. Embedded
> whitespace should not be altered.

Unfortunately, actual implementations and people's use matter
more.  I usually do not use graphical MUAs but when I tried to
paste long lines to Thunderbird (or was it Evolution, I do not
recall), it behaved as if I was typing the words, folded them at
convenient inter word spaces.  I would not blindly trust what
RFC says.

There also seem to be a strong correlation between people who
allow their MUA to send format=flowed when sending patches and
people who cut and paste their patches to their MUA, corrupting
whitespaces.

>> RFC3676 may be a good text communication medium.  It is just not
>> suitable for patch transfer.  Just don't use it.
>
> Would you consider making this configurable. Something like:
>
> applymail.allowed_flowed = true/false/warn
>
> If you're completely opposed though, we should modify git-am
> (and/or mailinfo) to reject format=flowed messages entirely, no?

I would not even consider applying flowed message these days (my
workflow is to review in MUA, save perhaps worthy ones to a
separate mailbox and after re-reviewing apply), so they will not
hit "git am" and it would not personally affect me.  Honestly, I
do not care very much either way.

But for some projects (perhaps the ones that do not value their
source as much as we do ;-)) accepting a subset of flowed
contents might be reasonable and even useful.  Maybe something
like this would be reasonably safe and still useful?

 - A format=flowed message that actually has a flowed line is
   always rejected;

 - If there is no flowed line (i.e. lines that end with SP) in
   the message, we unstuff the initial space to produce the
   result (there won't be anything else that is funny, will
   there?  In a patch we do not care much about the quotation of
   discussions):
 
   - If apply.whitespace is set to nowarn, we do not warn even
     though we might have lost the trailing whitespaces.

   - If apply.whitespace is set to warn, we warn about the
     flowed message.

   - If apply.whitespace is set to error or fix, we error out,
     but still leaving the result for manual inspection.

I dunno.

By the way, I do not think a solution that only uses
configuration is usable.

When you have to apply many patches, you set your configuration
to the most strict (i.e. apply.whitespace=error), and when the
processing errors out, you then inspect the situation and
manually override with the command line --whitespace=fix (or
"warn") to process that one message.  You need both.

Since mailinfo now has only a very few users (quiltimport and
"am"), we probably could add --whitespace option to mailinfo,
and teach "am" to pass --whitespace command line option it was
given (otherwise the value from apply.whitespace configuration)
when running mailinfo, if we were to do a "safer subset" as
outlined above.