[PATCH 1/2] i18n: leave \n out of translated diffstat

[PATCH 1/2] i18n: leave \n out of translated diffstat

[Nguyễn Thái Ngọc Duy]

GETTEXT_POISON scrapes everything in translated strings, including \n.
t4205.12 however needs this \n in matching the end result. Keep this
\n out of translation to make t4205.12 happy.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 1a594df..923b6aa 100644
--- a/diff.c
+++ b/diff.c
@@ -1397,7 +1397,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 
 	if (!files) {
 		assert(insertions == 0 && deletions == 0);
-		return fputs(_(" 0 files changed\n"), fp);
+		return fprintf(fp, "%s\n", _(" 0 files changed"));
 	}
 
 	strbuf_addf(&sb,
-- 
1.7.8

[PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Nguyễn Thái Ngọc Duy]

This fixes all GETTEXT_POISON breakages caused by recent i18n changes.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t0006-date.sh                 |    6 +++---
 t/t0040-parse-options.sh        |    8 ++++----
 t/t1300-repo-config.sh          |    2 +-
 t/t1502-rev-parse-parseopt.sh   |    2 +-
 t/t2006-checkout-index-basic.sh |    4 ++--
 t/t2107-update-index-basic.sh   |    4 ++--
 t/t3004-ls-files-basic.sh       |    4 ++--
 t/t3200-branch.sh               |    4 ++--
 t/t3501-revert-cherry-pick.sh   |    4 ++--
 t/t4006-diff-mode.sh            |    8 ++++----
 t/t4012-diff-binary.sh          |    4 ++--
 t/t4120-apply-popt.sh           |    4 ++--
 t/t4133-apply-filenames.sh      |    2 +-
 t/t4200-rerere.sh               |    4 ++--
 t/t4202-log.sh                  |    2 +-
 t/t5300-pack-object.sh          |    2 +-
 t/t5505-remote.sh               |   16 ++++++++--------
 t/t6500-gc.sh                   |    4 ++--
 t/t7508-status.sh               |    2 +-
 t/t7600-merge.sh                |    2 +-
 20 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 1d29810..3de3c8f 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -9,7 +9,7 @@ TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
 check_show() {
 	t=$(($TEST_DATE_NOW - $1))
 	echo "$t -> $2" >expect
-	test_expect_${3:-success} "relative date ($2)" "
+	test_expect_${3:-success} C_LOCALE_OUTPUT "relative date ($2)" "
 	test-date show $t >actual &&
 	test_cmp expect actual
 	"
@@ -29,7 +29,7 @@ check_show 62985600 '2 years ago'
 
 check_parse() {
 	echo "$1 -> $2" >expect
-	test_expect_${4:-success} "parse date ($1${3:+ TZ=$3})" "
+	test_expect_${4:-success} C_LOCALE_OUTPUT "parse date ($1${3:+ TZ=$3})" "
 	TZ=${3:-$TZ} test-date parse '$1' >actual &&
 	test_cmp expect actual
 	"
@@ -50,7 +50,7 @@ check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
 
 check_approxidate() {
 	echo "$1 -> $2 +0000" >expect
-	test_expect_${3:-success} "parse approxidate ($1)" "
+	test_expect_${3:-success} C_LOCALE_OUTPUT "parse approxidate ($1)" "
 	test-date approxidate '$1' >actual &&
 	test_cmp expect actual
 	"
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index e3f354a..790428d 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -48,7 +48,7 @@ Standard options
 
 EOF
 
-test_expect_success 'test help' '
+test_expect_success C_LOCALE_OUTPUT 'test help' '
 	test_must_fail test-parse-options -h > output 2> output.err &&
 	test ! -s output.err &&
 	test_cmp expect output
@@ -104,8 +104,8 @@ test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
 test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
 test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
 
-test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
-test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
+test_expect_success C_LOCALE_OUTPUT 'OPT_BOOL() no negation #1' 'check_unknown --fear'
+test_expect_success C_LOCALE_OUTPUT 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
 
 test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
 
@@ -308,7 +308,7 @@ cat > expect <<EOF
 Callback: "not set", 1
 EOF
 
-test_expect_success 'OPT_CALLBACK() and callback errors work' '
+test_expect_success C_LOCALE_OUTPUT 'OPT_CALLBACK() and callback errors work' '
 	test_must_fail test-parse-options --no-length > output 2> output.err &&
 	test_cmp expect output &&
 	test_cmp expect.err output.err
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index a477453..13953dc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -389,7 +389,7 @@ test_expect_success 'get bool variable with empty value' \
 	'git config --bool emptyvalue.variable > output &&
 	 cmp output expect'
 
-test_expect_success 'no arguments, but no crash' '
+test_expect_success C_LOCALE_OUTPUT 'no arguments, but no crash' '
 	test_must_fail git config >output 2>&1 &&
 	grep usage output
 '
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 1efd7f7..2b283ba 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -39,7 +39,7 @@ Extras
 extra1    line above used to cause a segfault but no longer does
 EOF
 
-test_expect_success 'test --parseopt help output' '
+test_expect_success C_LOCALE_OUTPUT 'test --parseopt help output' '
 	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec &&
 	test_cmp expect output
 '
diff --git a/t/t2006-checkout-index-basic.sh b/t/t2006-checkout-index-basic.sh
index b855983..3f75462 100755
--- a/t/t2006-checkout-index-basic.sh
+++ b/t/t2006-checkout-index-basic.sh
@@ -5,12 +5,12 @@ test_description='basic checkout-index tests
 
 . ./test-lib.sh
 
-test_expect_success 'checkout-index --gobbledegook' '
+test_expect_success C_LOCALE_OUTPUT 'checkout-index --gobbledegook' '
 	test_expect_code 129 git checkout-index --gobbledegook 2>err &&
 	grep "[Uu]sage" err
 '
 
-test_expect_success 'checkout-index -h in broken repository' '
+test_expect_success C_LOCALE_OUTPUT 'checkout-index -h in broken repository' '
 	mkdir broken &&
 	(
 		cd broken &&
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 809fafe..36f2ebf 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -13,12 +13,12 @@ test_expect_success 'update-index --nonsense fails' '
 	test -s msg
 '
 
-test_expect_success 'update-index --nonsense dumps usage' '
+test_expect_success C_LOCALE_OUTPUT 'update-index --nonsense dumps usage' '
 	test_expect_code 129 git update-index --nonsense 2>err &&
 	grep "[Uu]sage: git update-index" err
 '
 
-test_expect_success 'update-index -h with corrupt index' '
+test_expect_success C_LOCALE_OUTPUT 'update-index -h with corrupt index' '
 	mkdir broken &&
 	(
 		cd broken &&
diff --git a/t/t3004-ls-files-basic.sh b/t/t3004-ls-files-basic.sh
index 490e052..2feeb60 100755
--- a/t/t3004-ls-files-basic.sh
+++ b/t/t3004-ls-files-basic.sh
@@ -20,12 +20,12 @@ test_expect_success 'ls-files with nonexistent path' '
 	test_cmp empty actual
 '
 
-test_expect_success 'ls-files with nonsense option' '
+test_expect_success C_LOCALE_OUTPUT 'ls-files with nonsense option' '
 	test_expect_code 129 git ls-files --nonsense 2>actual &&
 	grep "[Uu]sage: git ls-files" actual
 '
 
-test_expect_success 'ls-files -h in corrupt repository' '
+test_expect_success C_LOCALE_OUTPUT 'ls-files -h in corrupt repository' '
 	mkdir broken &&
 	(
 		cd broken &&
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a17f8b2..4b24cf8 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -23,7 +23,7 @@ test_expect_success \
      test_path_is_missing .git/refs/heads/--help
 '
 
-test_expect_success 'branch -h in broken repository' '
+test_expect_success C_LOCALE_OUTPUT 'branch -h in broken repository' '
 	mkdir broken &&
 	(
 		cd broken &&
@@ -71,7 +71,7 @@ test_expect_success \
         git branch -d l/m &&
         git branch l'
 
-test_expect_success \
+test_expect_success C_LOCALE_OUTPUT \
     'git branch -m dumps usage' \
        'test_expect_code 129 git branch -m 2>err &&
 	grep "[Uu]sage: git branch" err'
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 595d2ff..9b4b860 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -41,7 +41,7 @@ test_expect_success setup '
 	git tag rename2
 '
 
-test_expect_success 'cherry-pick --nonsense' '
+test_expect_success C_LOCALE_OUTPUT 'cherry-pick --nonsense' '
 
 	pos=$(git rev-parse HEAD) &&
 	git diff --exit-code HEAD &&
@@ -50,7 +50,7 @@ test_expect_success 'cherry-pick --nonsense' '
 	grep '[Uu]sage:' msg
 '
 
-test_expect_success 'revert --nonsense' '
+test_expect_success C_LOCALE_OUTPUT 'revert --nonsense' '
 
 	pos=$(git rev-parse HEAD) &&
 	git diff --exit-code HEAD &&
diff --git a/t/t4006-diff-mode.sh b/t/t4006-diff-mode.sh
index 7a3e1f9..135327e 100755
--- a/t/t4006-diff-mode.sh
+++ b/t/t4006-diff-mode.sh
@@ -32,26 +32,26 @@ test_expect_success 'prepare binary file' '
 	git commit -m binbin
 '
 
-test_expect_success '--stat output after text chmod' '
+test_expect_success C_LOCALE_OUTPUT '--stat output after text chmod' '
 	test_chmod -x rezrov &&
 	echo " 0 files changed" >expect &&
 	git diff HEAD --stat >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '--shortstat output after text chmod' '
+test_expect_success C_LOCALE_OUTPUT '--shortstat output after text chmod' '
 	git diff HEAD --shortstat >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '--stat output after binary chmod' '
+test_expect_success C_LOCALE_OUTPUT '--stat output after binary chmod' '
 	test_chmod +x binbin &&
 	echo " 0 files changed" >expect &&
 	git diff HEAD --stat >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '--shortstat output after binary chmod' '
+test_expect_success C_LOCALE_OUTPUT '--shortstat output after binary chmod' '
 	git diff HEAD --shortstat >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 6cebb39..6764a7c 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -62,7 +62,7 @@ test_expect_success 'apply --numstat understands diff --binary format' '
 
 # apply needs to be able to skip the binary material correctly
 # in order to report the line number of a corrupt patch.
-test_expect_success 'apply detecting corrupt patch correctly' \
+test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' \
 	'git diff | sed -e 's/-CIT/xCIT/' >broken &&
 	 if git apply --stat --summary broken 2>detected
 	 then
@@ -76,7 +76,7 @@ test_expect_success 'apply detecting corrupt patch correctly' \
 	 detected=`sed -ne "${detected}p" broken` &&
 	 test "$detected" = xCIT'
 
-test_expect_success 'apply detecting corrupt patch correctly' \
+test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' \
 	'git diff --binary | sed -e 's/-CIT/xCIT/' >broken &&
 	 if git apply --stat --summary broken 2>detected
 	 then
diff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh
index a33d510..90d5313 100755
--- a/t/t4120-apply-popt.sh
+++ b/t/t4120-apply-popt.sh
@@ -29,7 +29,7 @@ test_expect_success 'apply git diff with -p2' '
 	git apply -p2 patch.file
 '
 
-test_expect_success 'apply with too large -p' '
+test_expect_success C_LOCALE_OUTPUT 'apply with too large -p' '
 	cp file1.saved file1 &&
 	test_must_fail git apply --stat -p3 patch.file 2>err &&
 	grep "removing 3 leading" err
@@ -51,7 +51,7 @@ test_expect_success 'apply (-p2) traditional diff with funny filenames' '
 	test_cmp expected file1
 '
 
-test_expect_success 'apply with too large -p and fancy filename' '
+test_expect_success C_LOCALE_OUTPUT 'apply with too large -p and fancy filename' '
 	cp file1.saved file1 &&
 	test_must_fail git apply --stat -p3 patch.escaped 2>err &&
 	grep "removing 3 leading" err
diff --git a/t/t4133-apply-filenames.sh b/t/t4133-apply-filenames.sh
index 94da990..94b0233 100755
--- a/t/t4133-apply-filenames.sh
+++ b/t/t4133-apply-filenames.sh
@@ -28,7 +28,7 @@ index d00491f..0000000
 EOF
 '
 
-test_expect_success 'apply diff with inconsistent filenames in headers' '
+test_expect_success C_LOCALE_OUTPUT 'apply diff with inconsistent filenames in headers' '
 	test_must_fail git apply bad1.patch 2>err &&
 	grep "inconsistent new filename" err &&
 	test_must_fail git apply bad2.patch 2>err &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 36255d6..937b217 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -377,7 +377,7 @@ test_expect_success 'explicit rerere --rerere-autoupdate overrides' '
 	test_cmp expected.unresolved actual2
 '
 
-test_expect_success 'rerere --no-no-rerere-autoupdate' '
+test_expect_success C_LOCALE_OUTPUT 'rerere --no-no-rerere-autoupdate' '
 	git rm -fr --cached . &&
 	git update-index --index-info <failedmerge &&
 	cp file3.conflict file3 &&
@@ -386,7 +386,7 @@ test_expect_success 'rerere --no-no-rerere-autoupdate' '
 	test_must_fail git update-index --refresh
 '
 
-test_expect_success 'rerere -h' '
+test_expect_success C_LOCALE_OUTPUT 'rerere -h' '
 	test_must_fail git rerere -h >help &&
 	grep [Uu]sage help
 '
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 71be59d..dc7d2e1 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -800,7 +800,7 @@ sanitize_output () {
 	    -e 's/, 1 insertions(+)/, 1 insertion(+)/'
 }
 
-test_expect_success 'log --graph with diff and stats' '
+test_expect_success C_LOCALE_OUTPUT 'log --graph with diff and stats' '
 	git log --graph --pretty=short --stat -p >actual &&
 	sanitize_output >actual.sanitized <actual &&
 	test_cmp expect actual.sanitized
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index d9d856b..da54924 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -413,7 +413,7 @@ test_expect_success \
      cp -f	.git/objects/9d/235ed07cd19811a6ceb342de82f190e49c9f68 \
 		.git/objects/c8/2de19312b6c3695c0c18f70709a6c535682a67'
 
-test_expect_success \
+test_expect_success C_LOCALE_OUTPUT \
     'make sure index-pack detects the SHA1 collision' \
     'test_must_fail git index-pack -o bad.idx test-3.pack 2>msg &&
      grep "SHA1 COLLISION FOUND" msg'
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index e8af615..f632262 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -24,7 +24,7 @@ setup_repository () {
 tokens_match () {
 	echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
 	echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
-	test_cmp expect actual
+	test_i18ncmp expect actual
 }
 
 check_remote_track () {
@@ -131,8 +131,8 @@ EOF
 	git remote rm oops 2>actual2 &&
 	git branch -d foobranch &&
 	git tag -d footag &&
-	test_cmp expect1 actual1 &&
-	test_cmp expect2 actual2
+	test_i18ncmp expect1 actual1 &&
+	test_i18ncmp expect2 actual2
 )
 '
 
@@ -192,7 +192,7 @@ test_expect_success 'show' '
 	 git config --add remote.two.push refs/heads/master:refs/heads/another &&
 	 git remote show origin two > output &&
 	 git branch -d rebase octopus &&
-	 test_cmp expect output)
+	 test_i18ncmp expect output)
 '
 
 cat > test/expect << EOF
@@ -217,7 +217,7 @@ test_expect_success 'show -n' '
 	 cd test &&
 	 git remote show -n origin > output &&
 	 mv ../one.unreachable ../one &&
-	 test_cmp expect output)
+	 test_i18ncmp expect output)
 '
 
 test_expect_success 'prune' '
@@ -255,7 +255,7 @@ EOF
 test_expect_success 'set-head --auto fails w/multiple HEADs' '
 	(cd test &&
 	 test_must_fail git remote set-head --auto two >output 2>&1 &&
-	test_cmp expect output)
+	test_i18ncmp expect output)
 '
 
 cat >test/expect <<EOF
@@ -285,7 +285,7 @@ test_expect_success 'prune --dry-run' '
 	 test_must_fail git rev-parse refs/remotes/origin/side &&
 	(cd ../one &&
 	 git branch -m side side2) &&
-	 test_cmp expect output)
+	 test_i18ncmp expect output)
 '
 
 test_expect_success 'add --mirror && prune' '
@@ -694,7 +694,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	 test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
 '
 
-test_expect_success 'remote prune to cause a dangling symref' '
+test_expect_success C_LOCALE_OUTPUT 'remote prune to cause a dangling symref' '
 	git clone one seven &&
 	(
 		cd one &&
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 82f3639..00a7a5b 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -9,12 +9,12 @@ test_expect_success 'gc empty repository' '
 	git gc
 '
 
-test_expect_success 'gc --gobbledegook' '
+test_expect_success C_LOCALE_OUTPUT 'gc --gobbledegook' '
 	test_expect_code 129 git gc --nonsense 2>err &&
 	grep "[Uu]sage: git gc" err
 '
 
-test_expect_success 'gc -h with invalid configuration' '
+test_expect_success C_LOCALE_OUTPUT 'gc -h with invalid configuration' '
 	mkdir broken &&
 	(
 		cd broken &&
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 28e1848..c3ca2ef 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -59,7 +59,7 @@ test_expect_success 'status (1)' '
 	test_i18ngrep "use \"git rm --cached <file>\.\.\.\" to unstage" output
 '
 
-test_expect_success 'status --column' '
+test_expect_success C_LOCALE_OUTPUT 'status --column' '
 	COLUMNS=50 git status --column="column dense" >output &&
 	cat >expect <<\EOF &&
 # On branch master
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9e27bbf..eebab61 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -149,7 +149,7 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge
 '
 
-test_expect_success 'merge -h with invalid index' '
+test_expect_success C_LOCALE_OUTPUT 'merge -h with invalid index' '
 	mkdir broken &&
 	(
 		cd broken &&
-- 
1.7.8

Re: [PATCH 1/2] i18n: leave \n out of translated diffstat

[Jonathan Nieder]

Hi,

Nguyễn Thái Ngọc Duy wrote:

> GETTEXT_POISON scrapes everything in translated strings, including \n.
> t4205.12 however needs this \n in matching the end result. Keep this
> \n out of translation to make t4205.12 happy.
[...]
> --- a/diff.c
> +++ b/diff.c
> @@ -1397,7 +1397,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>  
>  	if (!files) {
>  		assert(insertions == 0 && deletions == 0);
> -		return fputs(_(" 0 files changed\n"), fp);
> +		return fprintf(fp, "%s\n", _(" 0 files changed"));

More importantly, this makes the string more similar to other
translated strings in the same file and saves translators from having
to remember to worry about appropriate whitespace at the end of the
string.

Actually, it should be possible to make their lives even easier.  How
about this?

diff --git i/diff.c w/diff.c
index 1a594df4..c53eea50 100644
--- i/diff.c
+++ w/diff.c
@@ -1395,11 +1395,6 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 	struct strbuf sb = STRBUF_INIT;
 	int ret;
 
-	if (!files) {
-		assert(insertions == 0 && deletions == 0);
-		return fputs(_(" 0 files changed\n"), fp);
-	}
-
 	strbuf_addf(&sb,
 		    Q_(" %d file changed", " %d files changed", files),
 		    files);

Re: [PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Jonathan Nieder]

Nguyễn Thái Ngọc Duy wrote:

> This fixes all GETTEXT_POISON breakages caused by recent i18n changes.

First, thanks much for this.

Lots of these could be fixed in a more targetted way by using
test_i18ngrep, but the C_LOCALE_OUTPUT prereq works just as well as a
way to double-check that the newly translated strings are hopefully
not disrupting any functionality people rely on.

[...]
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -9,7 +9,7 @@ TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
>  check_show() {
>  	t=$(($TEST_DATE_NOW - $1))
>  	echo "$t -> $2" >expect
> -	test_expect_${3:-success} "relative date ($2)" "
> +	test_expect_${3:-success} C_LOCALE_OUTPUT "relative date ($2)" "
>  	test-date show $t >actual &&
>  	test_cmp expect actual
>  	"

Could use test_i18ncmp so we catch if test-date crashes, but anyway,
yeah, we can't expect to be able to meaningfully test date formatting
in another language.


> @@ -29,7 +29,7 @@ check_show 62985600 '2 years ago'
>  
>  check_parse() {
>  	echo "$1 -> $2" >expect
> -	test_expect_${4:-success} "parse date ($1${3:+ TZ=$3})" "
> +	test_expect_${4:-success} C_LOCALE_OUTPUT "parse date ($1${3:+ TZ=$3})" "
>  	TZ=${3:-$TZ} test-date parse '$1' >actual &&
>  	test_cmp expect actual
>  	"

This one is less pleasant.  Could test-date be convinced to produce
machine-readable output (e.g., seconds since the epoque)?  I'd be
especially concerned to check that the parser still accepts the
same input strings in other locales.

> @@ -50,7 +50,7 @@ check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
>  
>  check_approxidate() {
>  	echo "$1 -> $2 +0000" >expect
> -	test_expect_${3:-success} "parse approxidate ($1)" "
> +	test_expect_${3:-success} C_LOCALE_OUTPUT "parse approxidate ($1)" "
>  	test-date approxidate '$1' >actual &&
>  	test_cmp expect actual
>  	"

Likewise.

[...]
> --- a/t/t0040-parse-options.sh
> +++ b/t/t0040-parse-options.sh
> @@ -48,7 +48,7 @@ Standard options
>  
>  EOF
>  
> -test_expect_success 'test help' '
> +test_expect_success C_LOCALE_OUTPUT 'test help' '
>  	test_must_fail test-parse-options -h > output 2> output.err &&
>  	test ! -s output.err &&
>  	test_cmp expect output

Sensible.

> @@ -104,8 +104,8 @@ test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
>  test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
>  test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
>  
> -test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
> -test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
> +test_expect_success C_LOCALE_OUTPUT 'OPT_BOOL() no negation #1' 'check_unknown --fear'
> +test_expect_success C_LOCALE_OUTPUT 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'

Simpler to do

      --- i/t/t0040-parse-options.sh
      +++ w/t/t0040-parse-options.sh
      @@ -89,7 +89,7 @@ check_unknown() {
	      cat expect.err >>expect &&
	      test_must_fail test-parse-options $* >output 2>output.err &&
	      test ! -s output &&
      -	test_cmp expect output.err
      +	test_i18ncmp expect output.err
       }
       
       test_expect_success 'OPT_BOOL() #1' 'check boolean: 1 --yes'

[...]
> @@ -308,7 +308,7 @@ cat > expect <<EOF
>  Callback: "not set", 1
>  EOF
>  
> -test_expect_success 'OPT_CALLBACK() and callback errors work' '
> +test_expect_success C_LOCALE_OUTPUT 'OPT_CALLBACK() and callback errors work' '
>  	test_must_fail test-parse-options --no-length > output 2> output.err &&
>  	test_cmp expect output &&
>  	test_cmp expect.err output.err

Using test_i18ncmp on stderr would allow us to keep checking stdout.

[...]
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -389,7 +389,7 @@ test_expect_success 'get bool variable with empty value' \
>  	'git config --bool emptyvalue.variable > output &&
>  	 cmp output expect'
>  
> -test_expect_success 'no arguments, but no crash' '
> +test_expect_success C_LOCALE_OUTPUT 'no arguments, but no crash' '
>  	test_must_fail git config >output 2>&1 &&
>  	grep usage output
>  '

It's mostly about "no crash", so test_i18ngrep would be more
comforting.

[...]
> --- a/t/t1502-rev-parse-parseopt.sh
> +++ b/t/t1502-rev-parse-parseopt.sh
> @@ -39,7 +39,7 @@ Extras
>  extra1    line above used to cause a segfault but no longer does
>  EOF
>  
> -test_expect_success 'test --parseopt help output' '
> +test_expect_success C_LOCALE_OUTPUT 'test --parseopt help output' '
>  	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec &&
>  	test_cmp expect output
>  '

Seems fine (though test_i18ncmp is more targetted).

> --- a/t/t2006-checkout-index-basic.sh
> +++ b/t/t2006-checkout-index-basic.sh
> @@ -5,12 +5,12 @@ test_description='basic checkout-index tests
>  
>  . ./test-lib.sh
>  
> -test_expect_success 'checkout-index --gobbledegook' '
> +test_expect_success C_LOCALE_OUTPUT 'checkout-index --gobbledegook' '
[...]
> -test_expect_success 'checkout-index -h in broken repository' '
> +test_expect_success C_LOCALE_OUTPUT 'checkout-index -h in broken repository' '
[...]
> +++ b/t/t2107-update-index-basic.sh
[...]
> -test_expect_success 'update-index --nonsense dumps usage' '
> +test_expect_success C_LOCALE_OUTPUT 'update-index --nonsense dumps usage' '
[...]
> -test_expect_success 'update-index -h with corrupt index' '
> +test_expect_success C_LOCALE_OUTPUT 'update-index -h with corrupt index' '
[...]
> +++ b/t/t3004-ls-files-basic.sh
[...]
> -test_expect_success 'ls-files with nonsense option' '
> +test_expect_success C_LOCALE_OUTPUT 'ls-files with nonsense option' '
[...]
> -test_expect_success 'ls-files -h in corrupt repository' '
> +test_expect_success C_LOCALE_OUTPUT 'ls-files -h in corrupt repository' '
[...]
> +++ b/t/t3200-branch.sh
[...]
> -test_expect_success 'branch -h in broken repository' '
> +test_expect_success C_LOCALE_OUTPUT 'branch -h in broken repository' '
[...]
> -test_expect_success \
> +test_expect_success C_LOCALE_OUTPUT \
>      'git branch -m dumps usage' \
[...]
> +++ b/t/t3501-revert-cherry-pick.sh
[...]
> -test_expect_success 'cherry-pick --nonsense' '
> +test_expect_success C_LOCALE_OUTPUT 'cherry-pick --nonsense' '
[...]
> -test_expect_success 'revert --nonsense' '
> +test_expect_success C_LOCALE_OUTPUT 'revert --nonsense' '

Likewise.

> --- a/t/t4006-diff-mode.sh
> +++ b/t/t4006-diff-mode.sh
> @@ -32,26 +32,26 @@ test_expect_success 'prepare binary file' '
>  	git commit -m binbin
>  '
>  
> -test_expect_success '--stat output after text chmod' '
> +test_expect_success C_LOCALE_OUTPUT '--stat output after text chmod' '
>  	test_chmod -x rezrov &&
>  	echo " 0 files changed" >expect &&
>  	git diff HEAD --stat >actual &&
>  	test_cmp expect actual
>  '

Yeah, sensible enough.

It would be possible to recover some of the test by adding another
that does --numstat.

[...]
> --- a/t/t4012-diff-binary.sh
> +++ b/t/t4012-diff-binary.sh
> @@ -62,7 +62,7 @@ test_expect_success 'apply --numstat understands diff --binary format' '
>  
>  # apply needs to be able to skip the binary material correctly
>  # in order to report the line number of a corrupt patch.
> -test_expect_success 'apply detecting corrupt patch correctly' \
> +test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' \
>  	'git diff | sed -e 's/-CIT/xCIT/' >broken &&
>  	 if git apply --stat --summary broken 2>detected
>  	 then

This is an old one and uses a weird style.  I guess I'd go for

	git diff >patch &&
	sed -e "s/-CIT/xCIT/" <patch >broken &&
	test_must_fail git apply --stat --summary broken 2>detected &&

	if test_have_prereq C_LOCALE_OUTPUT
	then
		l=$(sed -n "s/.*fatal.*at line \([0-9]*\).*/\1/p" <detected) &&
		echo xCIT >expect &&
		sed -n "$l p" <broken >actual &&
		test_cmp expect actual
	fi

A translator is free to use %Id for the line number, so it's hard to
parse this in an arbitrary locale.

[...]
> -test_expect_success 'apply detecting corrupt patch correctly' \
> +test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt patch correctly' \

Likewise.

[...]
> --- a/t/t4120-apply-popt.sh
> +++ b/t/t4120-apply-popt.sh
[...]
> -test_expect_success 'apply with too large -p' '
> +test_expect_success C_LOCALE_OUTPUT 'apply with too large -p' '
>  	cp file1.saved file1 &&
>  	test_must_fail git apply --stat -p3 patch.file 2>err &&
>  	grep "removing 3 leading" err

It's mostly about making sure "git apply" fails gracefully and the
functionality involves parsing (which can get screwed up by locale
settings), so keeping the test running in other locales using
test_i18ngrep would be a comfort.  

[...]
> -test_expect_success 'apply with too large -p and fancy filename' '
> +test_expect_success C_LOCALE_OUTPUT 'apply with too large -p and fancy filename' '

Likewise.

> --- a/t/t4133-apply-filenames.sh
> +++ b/t/t4133-apply-filenames.sh
> @@ -28,7 +28,7 @@ index d00491f..0000000
[...]
> -test_expect_success 'apply diff with inconsistent filenames in headers' '
> +test_expect_success C_LOCALE_OUTPUT 'apply diff with inconsistent filenames in headers' '
>  	test_must_fail git apply bad1.patch 2>err &&
>  	grep "inconsistent new filename" err &&
>  	test_must_fail git apply bad2.patch 2>err &&
[...]
> +++ b/t/t4200-rerere.sh
[...]
> +test_expect_success C_LOCALE_OUTPUT 'rerere --no-no-rerere-autoupdate' '
[...]
> +test_expect_success C_LOCALE_OUTPUT 'rerere -h' '
[...]
> +++ b/t/t4202-log.sh
[...]
> +test_expect_success C_LOCALE_OUTPUT 'log --graph with diff and stats' '

Seems fine.

> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -413,7 +413,7 @@ test_expect_success \
[...]
> -test_expect_success \
> +test_expect_success C_LOCALE_OUTPUT \
>      'make sure index-pack detects the SHA1 collision' \
>      'test_must_fail git index-pack -o bad.idx test-3.pack 2>msg &&
>       grep "SHA1 COLLISION FOUND" msg'

I'd drop the grep, personally.

[...]
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -24,7 +24,7 @@ setup_repository () {
>  tokens_match () {
>  	echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
>  	echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
> -	test_cmp expect actual
> +	test_i18ncmp expect actual

Sensible.

[...]
> @@ -694,7 +694,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
>  	 test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
>  '
>  
> -test_expect_success 'remote prune to cause a dangling symref' '
> +test_expect_success C_LOCALE_OUTPUT 'remote prune to cause a dangling symref' '
>  	git clone one seven &&

test_i18ngrep message

and

test_i18ngrep ! message

work well for this kind of thing.

[...]
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
[...]
> +test_expect_success C_LOCALE_OUTPUT 'gc --gobbledegook' '
[...]
> +test_expect_success C_LOCALE_OUTPUT 'gc -h with invalid configuration' '
[...]
> +++ b/t/t7508-status.sh
[...]
> +test_expect_success C_LOCALE_OUTPUT 'status --column' '
[...]
> +test_expect_success C_LOCALE_OUTPUT 'merge -h with invalid index' '

Sane.

Hope that helps,
Jonathan

Re: [PATCH 1/2] i18n: leave \n out of translated diffstat

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> GETTEXT_POISON scrapes everything in translated strings, including \n.
>> t4205.12 however needs this \n in matching the end result. Keep this
>> \n out of translation to make t4205.12 happy.
> [...]
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -1397,7 +1397,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>>  
>>  	if (!files) {
>>  		assert(insertions == 0 && deletions == 0);
>> -		return fputs(_(" 0 files changed\n"), fp);
>> +		return fprintf(fp, "%s\n", _(" 0 files changed"));
>
> More importantly, this makes the string more similar to other
> translated strings in the same file and saves translators from having
> to remember to worry about appropriate whitespace at the end of the
> string.
>
> Actually, it should be possible to make their lives even easier.  How
> about this?
>
> diff --git i/diff.c w/diff.c
> index 1a594df4..c53eea50 100644
> --- i/diff.c
> +++ w/diff.c
> @@ -1395,11 +1395,6 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>  	struct strbuf sb = STRBUF_INIT;
>  	int ret;
>  
> -	if (!files) {
> -		assert(insertions == 0 && deletions == 0);
> -		return fputs(_(" 0 files changed\n"), fp);
> -	}
> -
>  	strbuf_addf(&sb,
>  		    Q_(" %d file changed", " %d files changed", files),
>  		    files);

Yeah, I think that is going in the right direction.

Thanks for careful reviews.

Re: [PATCH 1/2] i18n: leave \n out of translated diffstat

[Nguyen Thai Ngoc Duy]

On Mon, Jun 25, 2012 at 11:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Hi,
>>
>> Nguyễn Thái Ngọc Duy wrote:
>>
>>> GETTEXT_POISON scrapes everything in translated strings, including \n.
>>> t4205.12 however needs this \n in matching the end result. Keep this
>>> \n out of translation to make t4205.12 happy.
>> [...]
>>> --- a/diff.c
>>> +++ b/diff.c
>>> @@ -1397,7 +1397,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>>>
>>>      if (!files) {
>>>              assert(insertions == 0 && deletions == 0);
>>> -            return fputs(_(" 0 files changed\n"), fp);
>>> +            return fprintf(fp, "%s\n", _(" 0 files changed"));
>>
>> More importantly, this makes the string more similar to other
>> translated strings in the same file and saves translators from having
>> to remember to worry about appropriate whitespace at the end of the
>> string.
>>
>> Actually, it should be possible to make their lives even easier.  How
>> about this?
>>
>> diff --git i/diff.c w/diff.c
>> index 1a594df4..c53eea50 100644
>> --- i/diff.c
>> +++ w/diff.c
>> @@ -1395,11 +1395,6 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
>>       struct strbuf sb = STRBUF_INIT;
>>       int ret;
>>
>> -     if (!files) {
>> -             assert(insertions == 0 && deletions == 0);
>> -             return fputs(_(" 0 files changed\n"), fp);
>> -     }
>> -
>>       strbuf_addf(&sb,
>>                   Q_(" %d file changed", " %d files changed", files),
>>                   files);
>
> Yeah, I think that is going in the right direction.

That prints "0 files changed, 0 insertions(+), 0 deletions(-)" instead
of "0 files changed". Two more changed lines to make it the latter. I
don't mind either way, just checking which way you two and the list
prefer before I submit another patch.
-- 
Duy

Re: [PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Nguyen Thai Ngoc Duy]

On Sun, Jun 24, 2012 at 11:28 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyễn Thái Ngọc Duy wrote:
>
>> This fixes all GETTEXT_POISON breakages caused by recent i18n changes.
>
> First, thanks much for this.
>
> Lots of these could be fixed in a more targetted way by using
> test_i18ngrep, but the C_LOCALE_OUTPUT prereq works just as well as a
> way to double-check that the newly translated strings are hopefully
> not disrupting any functionality people rely on.

I probably still don't grasp the principles behind gettext-poison.
Pretending success quietly in test_i18ngrep and test_i18ncmp makes me
uncomfortable. At least C_LOCAL_OUTPUT clearly shows the test is
skipped, which is why I prefer it over test_i18n*. Letting it run to
catch crashes is good, but not enough (it's more likely to crash with
wrong printf format specifiers for example, which gettext-poison won't
catch). Will update it according to your comments.

> [...]
>> --- a/t/t0006-date.sh
>> +++ b/t/t0006-date.sh
>> @@ -9,7 +9,7 @@ TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
>>  check_show() {
>>       t=$(($TEST_DATE_NOW - $1))
>>       echo "$t -> $2" >expect
>> -     test_expect_${3:-success} "relative date ($2)" "
>> +     test_expect_${3:-success} C_LOCALE_OUTPUT "relative date ($2)" "
>>       test-date show $t >actual &&
>>       test_cmp expect actual
>>       "
>
> Could use test_i18ncmp so we catch if test-date crashes, but anyway,
> yeah, we can't expect to be able to meaningfully test date formatting
> in another language.

This reminds me of an inkscape crash I encountered long ago because of
a mistake in Vietnamese translation. Too bad we can't test all locales
for similar bugs (or we would need to i18n-ize expected results in the
test suite too)
-- 
Duy

Re: [PATCH 1/2] i18n: leave \n out of translated diffstat

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> That prints "0 files changed, 0 insertions(+), 0 deletions(-)" instead
> of "0 files changed". Two more changed lines to make it the latter.

Sloppy of me.  Yes, I meant

diff --git i/diff.c w/diff.c
index 1a594df4..287f310d 100644
--- i/diff.c
+++ w/diff.c
@@ -1395,11 +1395,6 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 	struct strbuf sb = STRBUF_INIT;
 	int ret;
 
-	if (!files) {
-		assert(insertions == 0 && deletions == 0);
-		return fputs(_(" 0 files changed\n"), fp);
-	}
-
 	strbuf_addf(&sb,
 		    Q_(" %d file changed", " %d files changed", files),
 		    files);
@@ -1412,7 +1407,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 	 * is probably less confusing (i.e skip over "2 files changed
 	 * but nothing about added/removed lines? Is this a bug in Git?").
 	 */
-	if (insertions || deletions == 0) {
+	if (insertions || (files && deletions == 0)) {
 		/*
 		 * TRANSLATORS: "+" in (+) is a line addition marker;
 		 * do not translate it.
@@ -1423,7 +1418,7 @@ int print_stat_summary(FILE *fp, int files, int insertions, int deletions)
 			    insertions);
 	}
 
-	if (deletions || insertions == 0) {
+	if (deletions || (files && insertions == 0)) {
 		/*
 		 * TRANSLATORS: "-" in (-) is a line removal marker;
 		 * do not translate it.

Re: [PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:
> On Sun, Jun 24, 2012 at 11:28 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nguyễn Thái Ngọc Duy wrote:

>>> -     test_expect_${3:-success} "relative date ($2)" "
>>> +     test_expect_${3:-success} C_LOCALE_OUTPUT "relative date ($2)" "
>>>       test-date show $t >actual &&
>>>       test_cmp expect actual
>>>       "
>>
>> Could use test_i18ncmp so we catch if test-date crashes, but anyway,
>> yeah, we can't expect to be able to meaningfully test date formatting
>> in another language.
>
> This reminds me of an inkscape crash I encountered long ago because of
> a mistake in Vietnamese translation. Too bad we can't test all locales
> for similar bugs (or we would need to i18n-ize expected results in the
> test suite too)

Sure we can. :)  We just need to make sure we are using test_i18ncmp
where appropriate so the tests run and check as much as they can
without relying on details that are left to the translators'
disgression.

But yes, the test harness doesn't know how to do that yet.  I'm hoping
that soon-ish there will be an option to run tests in your chosen
language, so the test result can more closely reflect bugs one is
likely to run into in practice (and so developers can more easily test
the edge cases such as Turkic locales where tolower('I') != 'i').
Sorry I haven't had enough time to bring this kind of thing into
reality, and thanks for stepping in with the partial work in that
direction before.

Jonathan

Re: [PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Nguyen Thai Ngoc Duy]

On Mon, Jun 25, 2012 at 6:32 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>> --- a/t/t0006-date.sh
>>> +++ b/t/t0006-date.sh
>>> @@ -9,7 +9,7 @@ TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
>>>  check_show() {
>>>       t=$(($TEST_DATE_NOW - $1))
>>>       echo "$t -> $2" >expect
>>> -     test_expect_${3:-success} "relative date ($2)" "
>>> +     test_expect_${3:-success} C_LOCALE_OUTPUT "relative date ($2)" "
>>>       test-date show $t >actual &&
>>>       test_cmp expect actual
>>>       "
>>
>> Could use test_i18ncmp so we catch if test-date crashes, but anyway,
>> yeah, we can't expect to be able to meaningfully test date formatting
>> in another language.
>
> This reminds me of an inkscape crash I encountered long ago because of
> a mistake in Vietnamese translation. Too bad we can't test all locales
> for similar bugs (or we would need to i18n-ize expected results in the
> test suite too)

What if we allow to run the test suite in non-C locales and skip the
tests that require C_LOCALE_OUTPUT? In non-C locales, we won't set
C_LOCALE_OUTPUT prerequisite. This allows to test a localized git with
~90% of the test suite. Would it work?
-- 
Duy

Re: [PATCH 2/2] Add C_LOCALE_OUTPUT prereq to test cases that require English text matching

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> What if we allow to run the test suite in non-C locales and skip the
> tests that require C_LOCALE_OUTPUT? In non-C locales, we won't set
> C_LOCALE_OUTPUT prerequisite. This allows to test a localized git with
> ~90% of the test suite. Would it work?

Yes, of course, modulo bugs. :)  I guess one thing to watch out for
would be assumptions about non-git utilities in the test suite --- for
example, some commands such as "sed" have a different idea of what a
character is in other locales.  But the main ingredient needed is just
UI (a test option or envvar).