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

From: René Scharfe <l.s.r@web.de>
Date: May 29, 2016, 17:25 UTC
Message-ID: <574B25F4.8090409@web.de>
In-Reply-To: <01020154fd28aa12-f536bf3e-58df-4d1f-b903-929b429954d3-000000@eu-west-1.amazonses.com>

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

Show 13 quoted lines

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

Show 6 quoted lines

>   	}
>
>   	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