Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:14
"Jay Soffian" [off-list ref] writes:
On Feb 16, 2008 1:57 AM, Junio C Hamano [off-list ref] 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.
quoted
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.