Re: [PATCH] Don't use $author_name undefined when $from contains no /\s</.

[Junio C Hamano]

Jim Meyering <address@hidden> writes:

> There were two differences, both involving removed trailing blanks.
> The first was a part of the diff: a line consisting of a single space
> denoting an empty line in the context.  I understood that those types
> of lines may safely be truncated (removing the trailing blank),
> and in fact, GNU diff -u (cvs) now does this by default:
>
> 2006-09-05  Paul Eggert  <address@hidden>
>
>         * NEWS: diff -u no longer outputs trailing white space unless the
>         input data has it.  Suggested by Jim Meyering.
>         * doc/diff.texi (Detailed Unified): Document this.
>         * src/context.c (pr_unidiff_hunk): Implement this.

Gaah.  Paul, why did you have to break this?  I see no good
reason, other than saving a single byte from the output stream
perhaps.

Leading ' ' at the context line is _not_ trailing white space;
it is a metadata just like a leading '+' or '-' is.

We could certainly update git-apply to understand it and we
probably would need to do so to cope with patch generated with
this *broken* GNU diff behaviour.

I see why some people consider why it _might_ be a good change.
A broken MUA tend to have trouble with lines that has only
whitespaces, so if a patch application program (patch or
git-apply) wants to deal with such a broken MUA, accepting a
totally empty line as if it is a line that has a single
whitespace at the beginning would save us from grief in some
cases.

However, I am not sure what "unless input data has it" means.
Does that mean if you have a line that has only one TAB (perhaps
caused by broken autoindent in the editor), that is "input data"
and is output as "SP TAB LF"?  If that is the case, then I do
not think dropping the leading SP only for an empty line makes
any sense.  A broken MUA would happily munge a line "SP TAB LF"
just as it would eat a line "SP LF".  Worse, such a MUA would
munge "+ TAB LF" into "+ LF", making the result of patch
application to be something the original patch author did not
intend to have.

If anything, this new behaviour makes the situation *actively*
worse.

By deciding to keep "SP TAB LF", you are saying that you _care_
about that trailing TAB in the patch and whitespace breakage
affects your payload in a bad way in your particular
application.  If that is the case, you would want to detect any
whitespace breakage a MUA might have caused before applying that
patch, and a broken context line that ought to be "SP LF" but
somehow comes out from MUA as "LF" would have served us as a
coalmine canary to help us detect the breakage.  Paul's change
to GNU diff is to kill that canary and I do not see any benefit
for doing so.

Why?

Please revert the patch, pretty please?