From: "René Scharfe" <l.s.r@web.de>
To: Dave Nicolson <david.nicolson@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] Do not output whitespace on blank lines
Date: Sun, 29 May 2016 19:25:08 +0200 [thread overview]
Message-ID: <574B25F4.8090409@web.de> (raw)
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).
> 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
next prev parent reply other threads:[~2016-05-29 17:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-29 15:36 [PATCH 1/2] Do not output whitespace on blank lines Dave Nicolson
2016-05-29 15:36 ` [PATCH 2/2] Fix tests Dave Nicolson
2016-05-29 17:06 ` [PATCH 1/2] Do not output whitespace on blank lines Thomas Gummerer
2016-05-29 17:25 ` René Scharfe [this message]
2016-05-30 0:30 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=574B25F4.8090409@web.de \
--to=l.s.r@web.de \
--cc=david.nicolson@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).