From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Dave Nicolson <david.nicolson@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] Do not output whitespace on blank lines
Date: Sun, 29 May 2016 17:30:59 -0700 [thread overview]
Message-ID: <xmqqa8j8pet8.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <574B25F4.8090409@web.de> ("René Scharfe"'s message of "Sun, 29 May 2016 19:25:08 +0200")
René Scharfe <l.s.r@web.de> 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".
prev parent reply other threads:[~2016-05-30 0:31 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
2016-05-30 0:30 ` Junio C Hamano [this message]
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=xmqqa8j8pet8.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=david.nicolson@gmail.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.