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

From: René Scharfe <[email-masked]>
Date: Sun, 29 May 2016 19:25:08 +0200

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."

Also: missing sign-off (see Documentation/SubmittingPatches).

>   diff.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff.c b/diff.c
> index d3734d3..459b36a 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -471,7 +471,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
>   		has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
>   		if (has_trailing_carriage_return)
>   			len--;
> -		nofirst = 0;
> +		nofirst = len == 0 && (char)first == ' ' ? 1 : 0;

Why the cast from int to char?  And you also don't need the "? 1 : 0" part.

>   	}
>
>   	if (len || !nofirst) {
>
> --
> https://github.com/git/git/pull/245

Your second patch adjusts tests.  It would be better to combine the two 
in order to keep tests working with each commit.

René

[*] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html