[PATCH] t/.gitattributes: only ignore whitespace errors in test files

[PATCH] t/.gitattributes: only ignore whitespace errors in test files

[Lea Wiemann]

Only ignore whitespace errors in t/tNNNN-*.sh and the t/tNNNN
subdirectories.  Other files (like test libraries) should still be
checked.

Also fix a whitespace error in t/test-lib.sh.

Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
 t/.gitattributes |    3 ++-
 t/test-lib.sh    |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index 562b12e..ab6edbf 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1 +1,2 @@
-* -whitespace
+t[0-9][0-9][0-9][0-9]-*.sh -whitespace
+t[0-9][0-9][0-9][0-9]/* -whitespace
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2a08cdc..73079d8 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -168,7 +168,7 @@ trap 'die' exit
 # environment variables to work around this.
 #
 # In particular, quoting isn't enough, as the path may contain the same quote
-# that we're using. 
+# that we're using.
 test_set_editor () {
 	FAKE_EDITOR="$1"
 	export FAKE_EDITOR
-- 
1.5.6.rc2.23.gfef6b.dirty

Re: [PATCH] t/.gitattributes: only ignore whitespace errors in test files

[Jeff King]

On Fri, Jun 13, 2008 at 12:35:59AM +0200, Lea Wiemann wrote:

> Only ignore whitespace errors in t/tNNNN-*.sh and the t/tNNNN
> subdirectories.  Other files (like test libraries) should still be
> checked.

Why?

What is the difference between test-lib.sh and tNNNN-*.sh that makes one
subject to whitespace checking and the other not?

(I suspect the answer is "shoving all the code in tNNNN-*.sh into eval'd
strings screws up the whitespace checking", but my point is that I
shouldn't have to guess; the justification should go in the commit
message).

-Peff

[PATCH v2] t/.gitattributes: only ignore whitespace errors in test files

[Lea Wiemann]

Only ignore whitespace errors in t/tNNNN-*.sh and the t/tNNNN
subdirectories (since they can contain test-relevant trailing
whitespace).  Other files (like test libraries) should still be
checked.

Also fix a whitespace error in t/test-lib.sh.

Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Jeff King wrote:
> What is the difference between test-lib.sh and tNNNN-*.sh that makes one
> subject to whitespace checking and the other not?

I thought that was obvious since they had been not been checked before
either (see the diff). :) Anyways, added explanation in parens in the
commit message; nothing else has changed since v1.

 t/.gitattributes |    3 ++-
 t/test-lib.sh    |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index 562b12e..ab6edbf 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1 +1,2 @@
-* -whitespace
+t[0-9][0-9][0-9][0-9]-*.sh -whitespace
+t[0-9][0-9][0-9][0-9]/* -whitespace
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7a8bd27..e9c9081 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -168,7 +168,7 @@ trap 'die' exit
 # environment variables to work around this.
 #
 # In particular, quoting isn't enough, as the path may contain the same quote
-# that we're using. 
+# that we're using.
 test_set_editor () {
 	FAKE_EDITOR="$1"
 	export FAKE_EDITOR
-- 
1.5.6.rc2.33.g0b5e3.dirty

Re: [PATCH] t/.gitattributes: only ignore whitespace errors in test files

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Jun 13, 2008 at 12:35:59AM +0200, Lea Wiemann wrote:
>
>> Only ignore whitespace errors in t/tNNNN-*.sh and the t/tNNNN
>> subdirectories.  Other files (like test libraries) should still be
>> checked.
>
> Why?
>
> What is the difference between test-lib.sh and tNNNN-*.sh that makes one
> subject to whitespace checking and the other not?

Eventually we would want to make all of the t/*.sh not exempt from the
whitespace rules.  Some currently do have trailing whitespaces as part of
their embedded test vectors, but there are many that are more carefully
written to avoid trailing whitespaces, by marking the EOL explicitly with
a non whitespace characters in the source, and running sed to produce the
actual vector that is used in the test.  That style is vastly preferrable
than having actual lines that end with trailing whitespaces, because it
makes it much clearer what is being fed to the scripts and what are
expected output when reading the source.  You do not have to "cat -e" to
see what they exactly do.

So I think this is one step in the right direction.  I do not want to keep
tNNNN-*.sh exemption forever.

Re: [PATCH] t/.gitattributes: only ignore whitespace errors in test files

[Jeff King]

On Fri, Jun 13, 2008 at 03:00:35AM -0700, Junio C Hamano wrote:

> Eventually we would want to make all of the t/*.sh not exempt from the
> whitespace rules.  Some currently do have trailing whitespaces as part of

This turned out to be a fairly easy change, as most of the places had
been caught already.

Four part patch series follows. The only one potentially not
maint-worthy is 3/4, because it actually changes git's output slightly.

  1/4: fix whitespace violations in test scripts
  2/4: mask necessary whitespace policy violations in test scripts
  3/4: avoid trailing whitespace in zero-change diffstat lines
  4/4: enable whitespace checking of test scripts

-Peff

[PATCH 1/4] fix whitespace violations in test scripts

[Jeff King]

These violations are simply wrong, but were never caught
because whitespace policy checking is turned off in the test
scripts.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1502-rev-parse-parseopt.sh |    2 +-
 t/t3800-mktag.sh              |    2 +-
 t/t3903-stash.sh              |    2 +-
 t/t4014-format-patch.sh       |    6 +++---
 t/t4150-am.sh                 |    2 +-
 t/t5540-http-push.sh          |    2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index d24a47d..7cdd70a 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -5,7 +5,7 @@ test_description='test git rev-parse --parseopt'
 
 cat > expect.err <<EOF
 usage: some-command [options] <args>...
-    
+
     some-command does foo and bar!
 
     -h, --help            show the help
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index df1fd6f..3907e67 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -245,7 +245,7 @@ cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
-tagger T A Gger <tagger@example.com>  
+tagger T A Gger <tagger@example.com>
 
 EOF
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2d3ee3b..54d99ed 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -41,7 +41,7 @@ test_expect_success 'apply needs clean working directory' '
 	echo 4 > other-file &&
 	git add other-file &&
 	echo 5 > other-file &&
- 	test_must_fail git stash apply
+	test_must_fail git stash apply
 '
 
 test_expect_success 'apply stashed changes' '
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 3583e68..7fe853c 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -98,7 +98,7 @@ test_expect_success 'extra headers' '
 	sed -e "/^$/q" patch2 > hdrs2 &&
 	grep "^To: R. E. Cipient <rcipient@example.com>$" hdrs2 &&
 	grep "^Cc: S. E. Cipient <scipient@example.com>$" hdrs2
-	
+
 '
 
 test_expect_success 'extra headers without newlines' '
@@ -109,7 +109,7 @@ test_expect_success 'extra headers without newlines' '
 	sed -e "/^$/q" patch3 > hdrs3 &&
 	grep "^To: R. E. Cipient <rcipient@example.com>$" hdrs3 &&
 	grep "^Cc: S. E. Cipient <scipient@example.com>$" hdrs3
-	
+
 '
 
 test_expect_success 'extra headers with multiple To:s' '
@@ -170,7 +170,7 @@ test_expect_success 'thread cover-letter' '
 	git checkout side &&
 	git format-patch --cover-letter --thread -o patches/ master &&
 	FIRST_MID=$(grep "Message-Id:" patches/0000-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
-	for i in patches/0001-* patches/0002-* patches/0003-* 
+	for i in patches/0001-* patches/0002-* patches/0003-*
 	do
 	  grep "References: $FIRST_MID" $i &&
 	  grep "In-Reply-To: $FIRST_MID" $i || break
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 722ae96..bc98260 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -110,7 +110,7 @@ test_expect_success 'am applies patch correctly' '
 
 GIT_AUTHOR_NAME="Another Thor"
 GIT_AUTHOR_EMAIL="a.thor@example.com"
-GIT_COMMITTER_NAME="Co M Miter" 
+GIT_COMMITTER_NAME="Co M Miter"
 GIT_COMMITTER_EMAIL="c.miter@example.com"
 export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL
 
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 7372439..f15dd03 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -38,7 +38,7 @@ test_expect_success 'setup remote repository' '
 	cd - &&
 	mv test_repo.git $HTTPD_DOCUMENT_ROOT_PATH
 '
-	
+
 test_expect_success 'clone remote repository' '
 	cd "$ROOT_PATH" &&
 	git clone $HTTPD_URL/test_repo.git test_repo_clone
-- 
1.5.6.rc2.183.g04614

[PATCH 2/4] mask necessary whitespace policy violations in test scripts

[Jeff King]

All of these violations are necessary parts of the tests
(which are generally checking the behavior of trailing
whitespace, or contain diff fragments with empty lines).

Our solution is two-fold:

  1. Process input with whitespace problems using tr. This
     has the added bonus that it becomes very obvious where
     the bogus whitespace is intended to go.

  2. Move large diff fragments into their own supplemental
     files. This gets rid of the whitespace problem, since
     supplemental files are not checked, and it also makes
     the test script a bit easier to read.

Signed-off-by: Jeff King <peff@peff.net>
---
Actually, the diff fragments could simply use empty lines to indicate an
empty context line, instead of a single space. This matches the GNU diff
format which Linus added support for a while back. However, I think
this change actually makes the test script more readable, and there is
something wrong to me about taking advantage of a diff convention that
we complained so much about at the time. ;)

 t/t4015-diff-whitespace.sh |    8 +-
 t/t4109-apply-multifrag.sh |  132 +------------------------------------------
 t/t4109/patch1.patch       |   28 +++++++++
 t/t4109/patch2.patch       |   30 ++++++++++
 t/t4109/patch3.patch       |   31 ++++++++++
 t/t4109/patch4.patch       |   30 ++++++++++
 t/t4119-apply-config.sh    |    4 +-
 7 files changed, 129 insertions(+), 134 deletions(-)
 create mode 100644 t/t4109/patch1.patch
 create mode 100644 t/t4109/patch2.patch
 create mode 100644 t/t4109/patch3.patch
 create mode 100644 t/t4109/patch4.patch

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index ca0302f..b7cc6b2 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -62,16 +62,16 @@ EOF
 
 git update-index x
 
-cat << EOF > x
+tr '_' ' ' << EOF > x
 	whitespace at beginning
 whitespace 	 change
 white space in the middle
-whitespace at end  
+whitespace at end__
 unchanged line
 CR at end
 EOF
 
-tr 'Q' '\015' << EOF > expect
+tr 'Q_' '\015 ' << EOF > expect
 diff --git a/x b/x
 index d99af23..8b32fb5 100644
 --- a/x
@@ -84,7 +84,7 @@ index d99af23..8b32fb5 100644
 +	whitespace at beginning
 +whitespace 	 change
 +white space in the middle
-+whitespace at end  
++whitespace at end__
  unchanged line
 -CR at endQ
 +CR at end
diff --git a/t/t4109-apply-multifrag.sh b/t/t4109-apply-multifrag.sh
index bd40a21..ff5fdf3 100755
--- a/t/t4109-apply-multifrag.sh
+++ b/t/t4109-apply-multifrag.sh
@@ -9,134 +9,10 @@ test_description='git apply test patches with multiple fragments.
 '
 . ./test-lib.sh
 
-# setup
-
-cat > patch1.patch <<\EOF
-diff --git a/main.c b/main.c
-new file mode 100644
---- /dev/null
-+++ b/main.c
-@@ -0,0 +1,23 @@
-+#include <stdio.h>
-+
-+int func(int num);
-+void print_int(int num);
-+
-+int main() {
-+	int i;
-+
-+	for (i = 0; i < 10; i++) {
-+		print_int(func(i));
-+	}
-+
-+	return 0;
-+}
-+
-+int func(int num) {
-+	return num * num;
-+}
-+
-+void print_int(int num) {
-+	printf("%d", num);
-+}
-+
-EOF
-cat > patch2.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,7 +1,9 @@
-+#include <stdlib.h>
- #include <stdio.h>
- 
- int func(int num);
- void print_int(int num);
-+void print_ln();
- 
- int main() {
- 	int i;
-@@ -10,6 +12,8 @@
- 		print_int(func(i));
- 	}
- 
-+	print_ln();
-+
- 	return 0;
- }
- 
-@@ -21,3 +25,7 @@
- 	printf("%d", num);
- }
- 
-+void print_ln() {
-+	printf("\n");
-+}
-+
-EOF
-cat > patch3.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,9 +1,7 @@
--#include <stdlib.h>
- #include <stdio.h>
- 
- int func(int num);
- void print_int(int num);
--void print_ln();
- 
- int main() {
- 	int i;
-@@ -12,8 +10,6 @@
- 		print_int(func(i));
- 	}
- 
--	print_ln();
--
- 	return 0;
- }
- 
-@@ -25,7 +21,3 @@
- 	printf("%d", num);
- }
- 
--void print_ln() {
--	printf("\n");
--}
--
-EOF
-cat > patch4.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,13 +1,14 @@
- #include <stdio.h>
- 
- int func(int num);
--void print_int(int num);
-+int func2(int num);
- 
- int main() {
- 	int i;
- 
- 	for (i = 0; i < 10; i++) {
--		print_int(func(i));
-+		printf("%d", func(i));
-+		printf("%d", func3(i));
- 	}
- 
- 	return 0;
-@@ -17,7 +18,7 @@
- 	return num * num;
- }
- 
--void print_int(int num) {
--	printf("%d", num);
-+int func2(int num) {
-+	return num * num * num;
- }
- 
-EOF
+cp ../t4109/patch1.patch .
+cp ../t4109/patch2.patch .
+cp ../t4109/patch3.patch .
+cp ../t4109/patch4.patch .
 
 test_expect_success "S = git apply (1)" \
     'git apply patch1.patch patch2.patch'
diff --git a/t/t4109/patch1.patch b/t/t4109/patch1.patch
new file mode 100644
index 0000000..1d411fc
--- /dev/null
+++ b/t/t4109/patch1.patch
@@ -0,0 +1,28 @@
+diff --git a/main.c b/main.c
+new file mode 100644
+--- /dev/null
++++ b/main.c
+@@ -0,0 +1,23 @@
++#include <stdio.h>
++
++int func(int num);
++void print_int(int num);
++
++int main() {
++	int i;
++
++	for (i = 0; i < 10; i++) {
++		print_int(func(i));
++	}
++
++	return 0;
++}
++
++int func(int num) {
++	return num * num;
++}
++
++void print_int(int num) {
++	printf("%d", num);
++}
++
diff --git a/t/t4109/patch2.patch b/t/t4109/patch2.patch
new file mode 100644
index 0000000..8c6b06d
--- /dev/null
+++ b/t/t4109/patch2.patch
@@ -0,0 +1,30 @@
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,7 +1,9 @@
++#include <stdlib.h>
+ #include <stdio.h>
+ 
+ int func(int num);
+ void print_int(int num);
++void print_ln();
+ 
+ int main() {
+ 	int i;
+@@ -10,6 +12,8 @@
+ 		print_int(func(i));
+ 	}
+ 
++	print_ln();
++
+ 	return 0;
+ }
+ 
+@@ -21,3 +25,7 @@
+ 	printf("%d", num);
+ }
+ 
++void print_ln() {
++	printf("\n");
++}
++
diff --git a/t/t4109/patch3.patch b/t/t4109/patch3.patch
new file mode 100644
index 0000000..d696c55
--- /dev/null
+++ b/t/t4109/patch3.patch
@@ -0,0 +1,31 @@
+cat > patch3.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,9 +1,7 @@
+-#include <stdlib.h>
+ #include <stdio.h>
+ 
+ int func(int num);
+ void print_int(int num);
+-void print_ln();
+ 
+ int main() {
+ 	int i;
+@@ -12,8 +10,6 @@
+ 		print_int(func(i));
+ 	}
+ 
+-	print_ln();
+-
+ 	return 0;
+ }
+ 
+@@ -25,7 +21,3 @@
+ 	printf("%d", num);
+ }
+ 
+-void print_ln() {
+-	printf("\n");
+-}
+-
diff --git a/t/t4109/patch4.patch b/t/t4109/patch4.patch
new file mode 100644
index 0000000..4b08590
--- /dev/null
+++ b/t/t4109/patch4.patch
@@ -0,0 +1,30 @@
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,13 +1,14 @@
+ #include <stdio.h>
+ 
+ int func(int num);
+-void print_int(int num);
++int func2(int num);
+ 
+ int main() {
+ 	int i;
+ 
+ 	for (i = 0; i < 10; i++) {
+-		print_int(func(i));
++		printf("%d", func(i));
++		printf("%d", func3(i));
+ 	}
+ 
+ 	return 0;
+@@ -17,7 +18,7 @@
+ 	return num * num;
+ }
+ 
+-void print_int(int num) {
+-	printf("%d", num);
++int func2(int num) {
++	return num * num * num;
+ }
+ 
diff --git a/t/t4119-apply-config.sh b/t/t4119-apply-config.sh
index b540f72..3c73a78 100755
--- a/t/t4119-apply-config.sh
+++ b/t/t4119-apply-config.sh
@@ -19,12 +19,12 @@ test_expect_success setup '
 '
 
 # Also handcraft GNU diff output; note this has trailing whitespace.
-cat >gpatch.file <<\EOF &&
+tr '_' ' ' >gpatch.file <<\EOF &&
 --- file1	2007-02-21 01:04:24.000000000 -0800
 +++ file1+	2007-02-21 01:07:44.000000000 -0800
 @@ -1 +1 @@
 -A
-+B 
++B_
 EOF
 
 sed -e 's|file1|sub/&|' gpatch.file >gpatch-sub.file &&
-- 
1.5.6.rc2.183.g04614

[PATCH 3/4] avoid trailing whitespace in zero-change diffstat lines

[Jeff King]

In some cases, we produce a diffstat line even though no
lines have changed (e.g., because of an exact rename). In
this case, there is no +/- "graph" after the number of
changed lines. However, we output the space separator
unconditionally, meaning that these lines contained a
trailing space character.

This isn't a huge problem, but in cleaning up the output we
are able to eliminate some trailing whitespace from a test
vector.

Signed-off-by: Jeff King <peff@peff.net>
---
I can't imagine any programs actually depend on the trailing space, but
the output change does make this the only contentious patch.

 diff.c                |    3 ++-
 t/t4016-diff-quote.sh |   14 +++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 62fdc54..f77f9e9 100644
--- a/diff.c
+++ b/diff.c
@@ -922,7 +922,8 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 			total = add + del;
 		}
 		show_name(options->file, prefix, name, len, reset, set);
-		fprintf(options->file, "%5d ", added + deleted);
+		fprintf(options->file, "%5d%s", added + deleted,
+				added + deleted ? " " : "");
 		show_graph(options->file, '+', add, add_c, reset);
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh
index 0950250..f07035a 100755
--- a/t/t4016-diff-quote.sh
+++ b/t/t4016-diff-quote.sh
@@ -53,13 +53,13 @@ test_expect_success 'git diff --summary -M HEAD' '
 '
 
 cat >expect <<\EOF
- pathname.1 => "Rpathname\twith HT.0"            |    0 
- pathname.3 => "Rpathname\nwith LF.0"            |    0 
- "pathname\twith HT.3" => "Rpathname\nwith LF.1" |    0 
- pathname.2 => Rpathname with SP.0               |    0 
- "pathname\twith HT.2" => Rpathname with SP.1    |    0 
- pathname.0 => Rpathname.0                       |    0 
- "pathname\twith HT.0" => Rpathname.1            |    0 
+ pathname.1 => "Rpathname\twith HT.0"            |    0
+ pathname.3 => "Rpathname\nwith LF.0"            |    0
+ "pathname\twith HT.3" => "Rpathname\nwith LF.1" |    0
+ pathname.2 => Rpathname with SP.0               |    0
+ "pathname\twith HT.2" => Rpathname with SP.1    |    0
+ pathname.0 => Rpathname.0                       |    0
+ "pathname\twith HT.0" => Rpathname.1            |    0
  7 files changed, 0 insertions(+), 0 deletions(-)
 EOF
 test_expect_success 'git diff --stat -M HEAD' '
-- 
1.5.6.rc2.183.g04614

[PATCH 4/4] enable whitespace checking of test scripts

[Jeff King]

Now that all of the policy violations have been cleaned up,
we can turn this on and start checking incoming patches.

Signed-off-by: Jeff King <peff@peff.net>
---
Check and mate, whitespace.

 t/.gitattributes |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index ab6edbf..1b97c54 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,2 +1 @@
-t[0-9][0-9][0-9][0-9]-*.sh -whitespace
 t[0-9][0-9][0-9][0-9]/* -whitespace
-- 
1.5.6.rc2.183.g04614

Re: [PATCH 1/4] fix whitespace violations in test scripts

[Jeff King]

On Sat, Jun 14, 2008 at 02:51:19AM -0400, Jeff King wrote:

> --- a/t/t1502-rev-parse-parseopt.sh
> +++ b/t/t1502-rev-parse-parseopt.sh
> @@ -5,7 +5,7 @@ test_description='test git rev-parse --parseopt'
>  
>  cat > expect.err <<EOF
>  usage: some-command [options] <args>...
> -    
> +
>      some-command does foo and bar!
>  
>      -h, --help            show the help

<sigh> I thought I had run all of the tests after this, but obviously I
screwed up. This is not a correct change.

I will send out a respun patch in a second.

-Peff

Re: [PATCH 1/4] fix whitespace violations in test scripts

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
> index d24a47d..7cdd70a 100755
> --- a/t/t1502-rev-parse-parseopt.sh
> +++ b/t/t1502-rev-parse-parseopt.sh
> @@ -5,7 +5,7 @@ test_description='test git rev-parse --parseopt'
>  
>  cat > expect.err <<EOF
>  usage: some-command [options] <args>...
> -    
> +
>      some-command does foo and bar!
>  
>      -h, --help            show the help

This part unfortunately falls into the same category as your [3/4].

---

 parse-options.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index acf3fe3..5e56bb5 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -312,8 +312,12 @@ void usage_with_options_internal(const char * const *usagestr,
 	fprintf(stderr, "usage: %s\n", *usagestr++);
 	while (*usagestr && **usagestr)
 		fprintf(stderr, "   or: %s\n", *usagestr++);
-	while (*usagestr)
-		fprintf(stderr, "    %s\n", *usagestr++);
+	while (*usagestr) {
+		if (**usagestr)
+			fprintf(stderr, "    %s", *usagestr);
+		putc('\n', stderr);
+		usagestr++;
+	}
 
 	if (opts->type != OPTION_GROUP)
 		fputc('\n', stderr);

Re: [PATCH 1/4] fix whitespace violations in test scripts

[Jeff King]

On Sat, Jun 14, 2008 at 12:20:45AM -0700, Junio C Hamano wrote:

> This part unfortunately falls into the same category as your [3/4].

I was just about to send the same patch. Hold on, there is one other
bug, and I am about to send the respun series.

-Peff

Re: [PATCH 3/4] avoid trailing whitespace in zero-change diffstat lines

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> In some cases, we produce a diffstat line even though no lines have
> changed (e.g., because of an exact rename). In this case, there is no
> +/- "graph" after the number of changed lines. However, we output the
> space separator unconditionally, meaning that these lines contained a
> trailing space character.
>
> This isn't a huge problem, but in cleaning up the output we are able to
> eliminate some trailing whitespace from a test vector.

This is why I love your patches.  Not merely fixing superficial issues but
doing so with _real thinking_ ;-)

[PATCH v2 1/5] fix whitespace violations in test scripts

[Jeff King]

These violations are simply wrong, but were never caught
because whitespace policy checking is turned off in the test
scripts.

Signed-off-by: Jeff King <peff@peff.net>
---
The original was total crap. t1502 didn't pass (which is now fixed in
3/4), and t3800 didn't pass (which is fixed correctly and lumped into
2/4 now).

 t/t3903-stash.sh        |    2 +-
 t/t4014-format-patch.sh |    6 +++---
 t/t4150-am.sh           |    2 +-
 t/t5540-http-push.sh    |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2d3ee3b..54d99ed 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -41,7 +41,7 @@ test_expect_success 'apply needs clean working directory' '
 	echo 4 > other-file &&
 	git add other-file &&
 	echo 5 > other-file &&
- 	test_must_fail git stash apply
+	test_must_fail git stash apply
 '
 
 test_expect_success 'apply stashed changes' '
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 3583e68..7fe853c 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -98,7 +98,7 @@ test_expect_success 'extra headers' '
 	sed -e "/^$/q" patch2 > hdrs2 &&
 	grep "^To: R. E. Cipient <rcipient@example.com>$" hdrs2 &&
 	grep "^Cc: S. E. Cipient <scipient@example.com>$" hdrs2
-	
+
 '
 
 test_expect_success 'extra headers without newlines' '
@@ -109,7 +109,7 @@ test_expect_success 'extra headers without newlines' '
 	sed -e "/^$/q" patch3 > hdrs3 &&
 	grep "^To: R. E. Cipient <rcipient@example.com>$" hdrs3 &&
 	grep "^Cc: S. E. Cipient <scipient@example.com>$" hdrs3
-	
+
 '
 
 test_expect_success 'extra headers with multiple To:s' '
@@ -170,7 +170,7 @@ test_expect_success 'thread cover-letter' '
 	git checkout side &&
 	git format-patch --cover-letter --thread -o patches/ master &&
 	FIRST_MID=$(grep "Message-Id:" patches/0000-* | sed "s/^[^<]*\(<[^>]*>\).*$/\1/") &&
-	for i in patches/0001-* patches/0002-* patches/0003-* 
+	for i in patches/0001-* patches/0002-* patches/0003-*
 	do
 	  grep "References: $FIRST_MID" $i &&
 	  grep "In-Reply-To: $FIRST_MID" $i || break
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 722ae96..bc98260 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -110,7 +110,7 @@ test_expect_success 'am applies patch correctly' '
 
 GIT_AUTHOR_NAME="Another Thor"
 GIT_AUTHOR_EMAIL="a.thor@example.com"
-GIT_COMMITTER_NAME="Co M Miter" 
+GIT_COMMITTER_NAME="Co M Miter"
 GIT_COMMITTER_EMAIL="c.miter@example.com"
 export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL
 
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 7372439..f15dd03 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -38,7 +38,7 @@ test_expect_success 'setup remote repository' '
 	cd - &&
 	mv test_repo.git $HTTPD_DOCUMENT_ROOT_PATH
 '
-	
+
 test_expect_success 'clone remote repository' '
 	cd "$ROOT_PATH" &&
 	git clone $HTTPD_URL/test_repo.git test_repo_clone
-- 
1.5.6.rc2.183.g04614

[PATCH v2 2/5] mask necessary whitespace policy violations in test scripts

[Jeff King]

All of these violations are necessary parts of the tests
(which are generally checking the behavior of trailing
whitespace, or contain diff fragments with empty lines).

Our solution is two-fold:

  1. Process input with whitespace problems using tr. This
     has the added bonus that it becomes very obvious where
     the bogus whitespace is intended to go.

  2. Move large diff fragments into their own supplemental
     files. This gets rid of the whitespace problem, since
     supplemental files are not checked, and it also makes
     the test script a bit easier to read.

Signed-off-by: Jeff King <peff@peff.net>
---
This now has a fix for t3800 which was incorrectly fixed in the original
1/4.

 t/t3800-mktag.sh           |    4 +-
 t/t4015-diff-whitespace.sh |    8 +-
 t/t4109-apply-multifrag.sh |  132 +------------------------------------------
 t/t4109/patch1.patch       |   28 +++++++++
 t/t4109/patch2.patch       |   30 ++++++++++
 t/t4109/patch3.patch       |   31 ++++++++++
 t/t4109/patch4.patch       |   30 ++++++++++
 t/t4119-apply-config.sh    |    4 +-
 8 files changed, 131 insertions(+), 136 deletions(-)
 create mode 100644 t/t4109/patch1.patch
 create mode 100644 t/t4109/patch2.patch
 create mode 100644 t/t4109/patch3.patch
 create mode 100644 t/t4109/patch4.patch

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index df1fd6f..c851db8 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -241,11 +241,11 @@ check_verify_failure 'disallow spaces in tag email' \
 ############################################################
 # 17. disallow missing tag timestamp
 
-cat >tag.sig <<EOF
+tr '_' ' ' >tag.sig <<EOF
 object $head
 type commit
 tag mytag
-tagger T A Gger <tagger@example.com>  
+tagger T A Gger <tagger@example.com>__
 
 EOF
 
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index ca0302f..b7cc6b2 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -62,16 +62,16 @@ EOF
 
 git update-index x
 
-cat << EOF > x
+tr '_' ' ' << EOF > x
 	whitespace at beginning
 whitespace 	 change
 white space in the middle
-whitespace at end  
+whitespace at end__
 unchanged line
 CR at end
 EOF
 
-tr 'Q' '\015' << EOF > expect
+tr 'Q_' '\015 ' << EOF > expect
 diff --git a/x b/x
 index d99af23..8b32fb5 100644
 --- a/x
@@ -84,7 +84,7 @@ index d99af23..8b32fb5 100644
 +	whitespace at beginning
 +whitespace 	 change
 +white space in the middle
-+whitespace at end  
++whitespace at end__
  unchanged line
 -CR at endQ
 +CR at end
diff --git a/t/t4109-apply-multifrag.sh b/t/t4109-apply-multifrag.sh
index bd40a21..ff5fdf3 100755
--- a/t/t4109-apply-multifrag.sh
+++ b/t/t4109-apply-multifrag.sh
@@ -9,134 +9,10 @@ test_description='git apply test patches with multiple fragments.
 '
 . ./test-lib.sh
 
-# setup
-
-cat > patch1.patch <<\EOF
-diff --git a/main.c b/main.c
-new file mode 100644
---- /dev/null
-+++ b/main.c
-@@ -0,0 +1,23 @@
-+#include <stdio.h>
-+
-+int func(int num);
-+void print_int(int num);
-+
-+int main() {
-+	int i;
-+
-+	for (i = 0; i < 10; i++) {
-+		print_int(func(i));
-+	}
-+
-+	return 0;
-+}
-+
-+int func(int num) {
-+	return num * num;
-+}
-+
-+void print_int(int num) {
-+	printf("%d", num);
-+}
-+
-EOF
-cat > patch2.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,7 +1,9 @@
-+#include <stdlib.h>
- #include <stdio.h>
- 
- int func(int num);
- void print_int(int num);
-+void print_ln();
- 
- int main() {
- 	int i;
-@@ -10,6 +12,8 @@
- 		print_int(func(i));
- 	}
- 
-+	print_ln();
-+
- 	return 0;
- }
- 
-@@ -21,3 +25,7 @@
- 	printf("%d", num);
- }
- 
-+void print_ln() {
-+	printf("\n");
-+}
-+
-EOF
-cat > patch3.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,9 +1,7 @@
--#include <stdlib.h>
- #include <stdio.h>
- 
- int func(int num);
- void print_int(int num);
--void print_ln();
- 
- int main() {
- 	int i;
-@@ -12,8 +10,6 @@
- 		print_int(func(i));
- 	}
- 
--	print_ln();
--
- 	return 0;
- }
- 
-@@ -25,7 +21,3 @@
- 	printf("%d", num);
- }
- 
--void print_ln() {
--	printf("\n");
--}
--
-EOF
-cat > patch4.patch <<\EOF
-diff --git a/main.c b/main.c
---- a/main.c
-+++ b/main.c
-@@ -1,13 +1,14 @@
- #include <stdio.h>
- 
- int func(int num);
--void print_int(int num);
-+int func2(int num);
- 
- int main() {
- 	int i;
- 
- 	for (i = 0; i < 10; i++) {
--		print_int(func(i));
-+		printf("%d", func(i));
-+		printf("%d", func3(i));
- 	}
- 
- 	return 0;
-@@ -17,7 +18,7 @@
- 	return num * num;
- }
- 
--void print_int(int num) {
--	printf("%d", num);
-+int func2(int num) {
-+	return num * num * num;
- }
- 
-EOF
+cp ../t4109/patch1.patch .
+cp ../t4109/patch2.patch .
+cp ../t4109/patch3.patch .
+cp ../t4109/patch4.patch .
 
 test_expect_success "S = git apply (1)" \
     'git apply patch1.patch patch2.patch'
diff --git a/t/t4109/patch1.patch b/t/t4109/patch1.patch
new file mode 100644
index 0000000..1d411fc
--- /dev/null
+++ b/t/t4109/patch1.patch
@@ -0,0 +1,28 @@
+diff --git a/main.c b/main.c
+new file mode 100644
+--- /dev/null
++++ b/main.c
+@@ -0,0 +1,23 @@
++#include <stdio.h>
++
++int func(int num);
++void print_int(int num);
++
++int main() {
++	int i;
++
++	for (i = 0; i < 10; i++) {
++		print_int(func(i));
++	}
++
++	return 0;
++}
++
++int func(int num) {
++	return num * num;
++}
++
++void print_int(int num) {
++	printf("%d", num);
++}
++
diff --git a/t/t4109/patch2.patch b/t/t4109/patch2.patch
new file mode 100644
index 0000000..8c6b06d
--- /dev/null
+++ b/t/t4109/patch2.patch
@@ -0,0 +1,30 @@
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,7 +1,9 @@
++#include <stdlib.h>
+ #include <stdio.h>
+ 
+ int func(int num);
+ void print_int(int num);
++void print_ln();
+ 
+ int main() {
+ 	int i;
+@@ -10,6 +12,8 @@
+ 		print_int(func(i));
+ 	}
+ 
++	print_ln();
++
+ 	return 0;
+ }
+ 
+@@ -21,3 +25,7 @@
+ 	printf("%d", num);
+ }
+ 
++void print_ln() {
++	printf("\n");
++}
++
diff --git a/t/t4109/patch3.patch b/t/t4109/patch3.patch
new file mode 100644
index 0000000..d696c55
--- /dev/null
+++ b/t/t4109/patch3.patch
@@ -0,0 +1,31 @@
+cat > patch3.patch <<\EOF
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,9 +1,7 @@
+-#include <stdlib.h>
+ #include <stdio.h>
+ 
+ int func(int num);
+ void print_int(int num);
+-void print_ln();
+ 
+ int main() {
+ 	int i;
+@@ -12,8 +10,6 @@
+ 		print_int(func(i));
+ 	}
+ 
+-	print_ln();
+-
+ 	return 0;
+ }
+ 
+@@ -25,7 +21,3 @@
+ 	printf("%d", num);
+ }
+ 
+-void print_ln() {
+-	printf("\n");
+-}
+-
diff --git a/t/t4109/patch4.patch b/t/t4109/patch4.patch
new file mode 100644
index 0000000..4b08590
--- /dev/null
+++ b/t/t4109/patch4.patch
@@ -0,0 +1,30 @@
+diff --git a/main.c b/main.c
+--- a/main.c
++++ b/main.c
+@@ -1,13 +1,14 @@
+ #include <stdio.h>
+ 
+ int func(int num);
+-void print_int(int num);
++int func2(int num);
+ 
+ int main() {
+ 	int i;
+ 
+ 	for (i = 0; i < 10; i++) {
+-		print_int(func(i));
++		printf("%d", func(i));
++		printf("%d", func3(i));
+ 	}
+ 
+ 	return 0;
+@@ -17,7 +18,7 @@
+ 	return num * num;
+ }
+ 
+-void print_int(int num) {
+-	printf("%d", num);
++int func2(int num) {
++	return num * num * num;
+ }
+ 
diff --git a/t/t4119-apply-config.sh b/t/t4119-apply-config.sh
index b540f72..3c73a78 100755
--- a/t/t4119-apply-config.sh
+++ b/t/t4119-apply-config.sh
@@ -19,12 +19,12 @@ test_expect_success setup '
 '
 
 # Also handcraft GNU diff output; note this has trailing whitespace.
-cat >gpatch.file <<\EOF &&
+tr '_' ' ' >gpatch.file <<\EOF &&
 --- file1	2007-02-21 01:04:24.000000000 -0800
 +++ file1+	2007-02-21 01:07:44.000000000 -0800
 @@ -1 +1 @@
 -A
-+B 
++B_
 EOF
 
 sed -e 's|file1|sub/&|' gpatch.file >gpatch-sub.file &&
-- 
1.5.6.rc2.183.g04614

[PATCH v2 3/5] avoid whitespace on empty line in automatic usage message

[Jeff King]

When outputting a usage message with a blank line in the
header, we would output a line with four spaces. Make this
truly a blank line.

This helps us remove trailing whitespace from a test vector.

Signed-off-by: Jeff King <peff@peff.net>
---
This is equivalent to the fix you just posted.

 parse-options.c               |    8 ++++++--
 t/t1502-rev-parse-parseopt.sh |    2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index acf3fe3..8071711 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -312,8 +312,12 @@ void usage_with_options_internal(const char * const *usagestr,
 	fprintf(stderr, "usage: %s\n", *usagestr++);
 	while (*usagestr && **usagestr)
 		fprintf(stderr, "   or: %s\n", *usagestr++);
-	while (*usagestr)
-		fprintf(stderr, "    %s\n", *usagestr++);
+	while (*usagestr) {
+		fprintf(stderr, "%s%s\n",
+				**usagestr ? "    " : "",
+				*usagestr);
+		usagestr++;
+	}
 
 	if (opts->type != OPTION_GROUP)
 		fputc('\n', stderr);
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index d24a47d..7cdd70a 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -5,7 +5,7 @@ test_description='test git rev-parse --parseopt'
 
 cat > expect.err <<EOF
 usage: some-command [options] <args>...
-    
+
     some-command does foo and bar!
 
     -h, --help            show the help
-- 
1.5.6.rc2.183.g04614

[PATCH v2 4/5] avoid trailing whitespace in zero-change diffstat lines

[Jeff King]

In some cases, we produce a diffstat line even though no
lines have changed (e.g., because of an exact rename). In
this case, there is no +/- "graph" after the number of
changed lines. However, we output the space separator
unconditionally, meaning that these lines contained a
trailing space character.

This isn't a huge problem, but in cleaning up the output we
are able to eliminate some trailing whitespace from a test
vector.

Signed-off-by: Jeff King <peff@peff.net>
---
This is identical to the original 3/4.

 diff.c                |    3 ++-
 t/t4016-diff-quote.sh |   14 +++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 62fdc54..f77f9e9 100644
--- a/diff.c
+++ b/diff.c
@@ -922,7 +922,8 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 			total = add + del;
 		}
 		show_name(options->file, prefix, name, len, reset, set);
-		fprintf(options->file, "%5d ", added + deleted);
+		fprintf(options->file, "%5d%s", added + deleted,
+				added + deleted ? " " : "");
 		show_graph(options->file, '+', add, add_c, reset);
 		show_graph(options->file, '-', del, del_c, reset);
 		fprintf(options->file, "\n");
diff --git a/t/t4016-diff-quote.sh b/t/t4016-diff-quote.sh
index 0950250..f07035a 100755
--- a/t/t4016-diff-quote.sh
+++ b/t/t4016-diff-quote.sh
@@ -53,13 +53,13 @@ test_expect_success 'git diff --summary -M HEAD' '
 '
 
 cat >expect <<\EOF
- pathname.1 => "Rpathname\twith HT.0"            |    0 
- pathname.3 => "Rpathname\nwith LF.0"            |    0 
- "pathname\twith HT.3" => "Rpathname\nwith LF.1" |    0 
- pathname.2 => Rpathname with SP.0               |    0 
- "pathname\twith HT.2" => Rpathname with SP.1    |    0 
- pathname.0 => Rpathname.0                       |    0 
- "pathname\twith HT.0" => Rpathname.1            |    0 
+ pathname.1 => "Rpathname\twith HT.0"            |    0
+ pathname.3 => "Rpathname\nwith LF.0"            |    0
+ "pathname\twith HT.3" => "Rpathname\nwith LF.1" |    0
+ pathname.2 => Rpathname with SP.0               |    0
+ "pathname\twith HT.2" => Rpathname with SP.1    |    0
+ pathname.0 => Rpathname.0                       |    0
+ "pathname\twith HT.0" => Rpathname.1            |    0
  7 files changed, 0 insertions(+), 0 deletions(-)
 EOF
 test_expect_success 'git diff --stat -M HEAD' '
-- 
1.5.6.rc2.183.g04614

[PATCH v2 5/5] enable whitespace checking of test scripts

[Jeff King]

Now that all of the policy violations have been cleaned up,
we can turn this on and start checking incoming patches.

Signed-off-by: Jeff King <peff@peff.net>
---
This is identical to the original 4/4.

 t/.gitattributes |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/t/.gitattributes b/t/.gitattributes
index ab6edbf..1b97c54 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,2 +1 @@
-t[0-9][0-9][0-9][0-9]-*.sh -whitespace
 t[0-9][0-9][0-9][0-9]/* -whitespace
-- 
1.5.6.rc2.183.g04614

Re: [PATCH 3/4] avoid trailing whitespace in zero-change diffstat lines

[Jeff King]

On Sat, Jun 14, 2008 at 12:22:38AM -0700, Junio C Hamano wrote:

> > This isn't a huge problem, but in cleaning up the output we are able to
> > eliminate some trailing whitespace from a test vector.
> 
> This is why I love your patches.  Not merely fixing superficial issues but
> doing so with _real thinking_ ;-)

Heh. I am still trying to remove the brown paper bag from sending
patches to the test scripts that don't even pass!

-Peff