Re: [PATCH] Add a test for a problem in "rebase --whitespace=fix"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Björn Gustavsson <bgustavsson@xxxxxxxxx> 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 &&

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]