public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: "Björn Gustavsson" <bgustavsson@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Add a test for a problem in "rebase --whitespace=fix"
Date: Sun, 07 Feb 2010 10:38:10 -0800	[thread overview]
Message-ID: <7vtytsevsd.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 4B6E7564.7040109@gmail.com

Björn Gustavsson <bgustavsson@gmail•com> writes:

> The command "git rebase --whitespace=fix HEAD~<N>" is supposed to
> only clean up trailing whitespace, and the expectation is that it
> cannot fail.

I don't know if the expectation is sound to begin with, for exactly the
reason you mention below.

> Unfortunately, if one commit adds a blank line at the end of a file
> and a subsequent commit adds more non-blank lines after the blank
> line,...

First, is this a condition that we want to change the behaviour to
"succeed" later?  

Imagine that the gap between abc and def block in your example is much
larger to exceed the number of pre-context lines of your second patch
(usually 3), and imagine you are the "git apply --whitespace=fix" program
you have updated to "fix" the preceived problem.  You know you earlier
might have stripped some blank lines at the EOF, but there is nothing that
tells you if you had only 3 blank lines, or you had even more.  How many
blank lines will you be adding back?

I think one fundamental difference between stripping of blanks at EOL and
blanks at EOF is that the former, even after applying an earlier patch
with the whitespace fix, could be fudged to an unspecified number of
whitespaces while you are applying the second one, exactly because you
will strip them out from the output of the second one anyway.  But the
latter will have to _appear_ in the result, as you have demonstrated, as a
gap between abc and def blocks in your example.  Simply there is not
enough information to do so.

Around 1.6.6/1.6.5.3 timeframe, we have separated blank-at-{eol,eof} out
of trailing-space to allow users to keep the traling blank lines.  Perhaps
you could demonstrate what is expected to work (and not bothering with
what is not ever expected to work) by changing the test like this.

I added one "trailing whitespace at EOL" example to keep the "strip
trailing whitespace" part working, by the way.

 t/t3417-rebase-whitespace-fix.sh |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/t/t3417-rebase-whitespace-fix.sh b/t/t3417-rebase-whitespace-fix.sh
index 55cbce7..0e366f8 100755
--- a/t/t3417-rebase-whitespace-fix.sh
+++ b/t/t3417-rebase-whitespace-fix.sh
@@ -7,9 +7,9 @@ This test runs git rebase --whitespace=fix and make sure that it works.
 
 . ./test-lib.sh
 
-# prepare initial revision of "file" with a blank line at the end
-cat >file <<EOF
-a
+# prepare initial revision of "file" with a trailing blank and a blank line at the end
+sed -e 's/Z//' >file <<\EOF
+a    Z
 b
 c
 
@@ -20,6 +20,7 @@ cat >expect <<EOF
 a
 b
 c
+
 EOF
 
 # prepare second revision of "file"
@@ -33,7 +34,9 @@ e
 f
 EOF
 
-test_expect_failure 'blanks line at end of file; extend at end of file' '
+test_expect_success 'keep blanks line at end of file' '
+	git config core.whitespace -blank-at-eof &&
+
 	git commit --allow-empty -m "Initial empty commit" &&
 	git add file && git commit -m first &&
 	mv second file &&

next prev parent reply	other threads:[~2010-02-07 18:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-07  8:10 [PATCH] Add a test for a problem in "rebase --whitespace=fix" Björn Gustavsson
2010-02-07 18:38 ` Junio C Hamano [this message]
2010-02-07 22:44   ` Björn Gustavsson
2010-02-08  0:15     ` Junio C Hamano
2010-02-08  7:37       ` Björn Gustavsson
2010-02-09 21:58         ` Junio C Hamano
2010-02-10 20:20           ` Björn Gustavsson

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:55cbce7 dfblob:0e366f8 )
 OR (
bs:"Re: [PATCH] Add a test for a problem in "
bs:"rebase --whitespace=fix" )
	(help)

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=7vtytsevsd.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=bgustavsson@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox