Subject: Re: [PATCH 1/2] Do not output whitespace on blank lines

From: Junio C Hamano <[email-masked]>
Date: Sun, 29 May 2016 17:30:59 -0700

René Scharfe <[email-masked]> writes:

> Am 29.05.2016 um 17:36 schrieb Dave Nicolson:
>> ---
>
> git diff marks context lines (in unified diff format) with a preceding
> space character.  Your intent is to remove that marker for empty
> context lines, right?  Why?  How much smaller do diffs get by that
> (assuming output size reduction is one of the reasons)?
>
> How compatible is it with patch, git apply and other programs that
> handle diffs?  Interestingly in that context, POSIX [*] allows it,
> saying: "It is implementation-defined whether an empty unaffected line
> is written as an empty line or a line containing a single <space>
> character."

Indeed POSIX allows it, and I am aware of a push to GNU diff some
time ago that making this possible (I do not offhand know if it was
a move to make it default, though), which is why "git apply" does
tolerate an empty line in the common context that is not a single SP
on a line since b507b465 (git-apply: prepare for upcoming GNU diff
-u format change., 2006-10-19).

But that does not mean we should flip the default without any
justification like this patch does.

FWIW, GNU diffutils 3.3 seems _not_ to default to this behaviour,
and require you to say --suppress-blank-empty when asking for this
form of output.  The rationale for this option in their
documentation is:

    Some text editors and email agents routinely delete trailing
    blanks, so it can be a problem to deal with diff output files
    that contain them. You can avoid this problem with the
    --suppress-blank-empty option. It causes diff to omit trailing
    blanks at the end of output lines in normal, context, and
    unified format, unless the trailing blanks were already present
    in the input.

If you think about this, the above does not make _any_ sense as a
justification for that feature.  "Some editors tend to remove SPs
that you do want if they appear at the end of line, which is a
problem.  To _AVOID_ this problem, we give an option not to produce
the SP that may be eaten by such editors in the first place".

Seriously?  The reason why you do want them in the first place is
because eating them silently would cause trouble on the receiving
end, so it is very understandable to prepare the side that _uses_
diff output to accept such an output that is missing the SPs at the
end of the line.  That justification does not make sense for the
side that _generates_ output at all.

In any case, I am not strongly opposed to add --suppress-blank-empty
option to "git diff" family, even though I do not see much point in
it.  I'll not accept a change to make that the default, until "diff"
implementations everybody else uses uses such default and POSIX
starts saying "a single SP on a line to represent an empty shared
context is deprecated".