For Real - Fixed pluralization in diff reports

[I must have accidentally removed the "s" in "deletions" before. I just rebuilt everything and remade the patch. All looks well this time. This is my first submitted patch to anything so my fingers are still learning.]

I got irritated by the

     1 files changed, 0 insertions(+), 1 deletions(-)

lack of pluralization so I fixed it. Now it says

     1 file changed, 0 insertions(+), 1 deletion(-)

and so forth.

---
 diff.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

index 93ef9a2..a179b24 100644
@@ -1465,8 +1465,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	}
 	fprintf(options->file, "%s", line_prefix);
 	fprintf(options->file,
-	       " %d files changed, %d insertions(+), %d deletions(-)\n",
-	       total_files, adds, dels);
+	       " %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+	       total_files, total_files == 1 ? "" : "s", adds, adds == 1 ? "" : "s", dels,
+		dels == 1 ? "" : "s");
 }
 
 static void show_shortstats(struct diffstat_t *data, struct 

diff_options *options)
@@ -1496,8 +1497,9 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option
options->output_prefix_data);
fprintf(options->file, "%s", msg->buf);
}
- fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n",
- total_files, adds, dels);
+ fprintf(options->file, " %d file%s changed, %d insertion%s(+), %d deletion%s(-)\n",
+ total_files, total_files == 1 ? "" : "s", adds, adds == 1 ? "" : "s", dels,
+ dels == 1 ? "" : "s");
}

static void show_numstat(struct diffstat_t *data, struct diff_options
*options)
-- 1.7.6.351.gb35ac.dirty

Are you sure this does not break any tests? t3508.2 for example hardcodes "1 insertions" and does textual compare. I have not run the tests but I suspect it will fail.

Duy

I ran the tests. The only result in red that I saw that didn't appear to be caused "by breakage" was

not ok - 3 mktemp to unwritable directory prints filename

Since I didn't do anything relating to mktemp I thought this was probably bogus.

I'm new to this so I don't know how to find the t3508.2. If you can give me a hint I'd be glad to look at this in more detail.

Jon

Your patch is line-wrapped. Please see
Documentation/SubmittingPatches, it may help.

I manually applied your patch and run t3508 alone. t3508.2 is the second test in t3502, by the way. t/README gives more information on tests of git.

pclouds@do ~/w/git/t $ ./t3508-cherry-pick-many-commits.sh -v Initialized empty Git repository in /home/pclouds/w/git/t/trash directory.t3508-cherry-pick-many-commits/.git/ expecting success:

	echo first > file1 &&
	git add file1 &&
	test_tick &&
	git commit -m "first" &&
	git tag first &&

	git checkout -b other &&
	for val in second third fourth
	do
		echo $val >> file1 &&
		git add file1 &&
		test_tick &&
		git commit -m "$val" &&
		git tag $val
	done

[master (root-commit) 0c72e4f] first
Author: A U Thor <[email protected]>

 1 file changed, 1 insertion(+), 0 deletions(-)
 create mode 100644 file1
[other 453a047] second
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)
[other e85abe2] third
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)
[other 94d3184] fourth
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)

ok 1 - setup

expecting success:

	cat <<-\EOF >expected &&
	[master OBJID] second
	 Author: A U Thor <[email protected]>
	 1 files changed, 1 insertions(+), 0 deletions(-)
	[master OBJID] third
	 Author: A U Thor <[email protected]>
	 1 files changed, 1 insertions(+), 0 deletions(-)
	[master OBJID] fourth
	 Author: A U Thor <[email protected]>
	 1 files changed, 1 insertions(+), 0 deletions(-)
	EOF

	git checkout -f master &&
	git reset --hard first &&
	test_tick &&
	git cherry-pick first..fourth >actual &&
	git diff --quiet other &&
	git diff --quiet HEAD other &&

	sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
	test_cmp expected actual.fuzzy &&
	check_head_differs_from fourth

HEAD is now at 0c72e4f first

@@ -1,9 +1,9 @@
 [master OBJID] second
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)
 [master OBJID] third
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)
 [master OBJID] fourth
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)

not ok - 2 cherry-pick first..fourth works
#
# cat <<-\EOF >expected &&
# [master OBJID] second
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# [master OBJID] third
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# [master OBJID] fourth
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# EOF
#
# git checkout -f master &&
# git reset --hard first &&
# test_tick &&
# git cherry-pick first..fourth >actual &&
# git diff --quiet other &&
# git diff --quiet HEAD other &&
#
# sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
# test_cmp expected actual.fuzzy &&
# check_head_differs_from fourth
#

expecting success:
cat <<-\EOF >expected &&
Trying simple merge.
[master OBJID] second
Author: A U Thor <[email protected]>
1 files changed, 1 insertions(+), 0 deletions(-)
Trying simple merge.
[master OBJID] third
Author: A U Thor <[email protected]>
1 files changed, 1 insertions(+), 0 deletions(-)
Trying simple merge.
[master OBJID] fourth
Author: A U Thor <[email protected]>
1 files changed, 1 insertions(+), 0 deletions(-)
EOF

git checkout -f master &&
git reset --hard first &&
test_tick &&
git cherry-pick --strategy resolve first..fourth >actual && git diff --quiet other &&
git diff --quiet HEAD other &&
sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy && test_cmp expected actual.fuzzy && check_head_differs_from fourth

HEAD is now at 0c72e4f first

@@ -1,12 +1,12 @@
 Trying simple merge.
 [master OBJID] second
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)
 Trying simple merge.
 [master OBJID] third
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)
 Trying simple merge.
 [master OBJID] fourth
  Author: A U Thor <[email protected]>
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+), 0 deletions(-)

not ok - 3 cherry-pick --strategy resolve first..fourth works #
# cat <<-\EOF >expected &&
# Trying simple merge.
# [master OBJID] second
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# Trying simple merge.
# [master OBJID] third
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# Trying simple merge.
# [master OBJID] fourth
# Author: A U Thor <[email protected]>
# 1 files changed, 1 insertions(+), 0 deletions(-)
# EOF
#
# git checkout -f master &&
# git reset --hard first &&
# test_tick &&
# git cherry-pick --strategy resolve first..fourth >actual &&
# git diff --quiet other &&
# git diff --quiet HEAD other &&
# sed -e "s/$_x05[0-9a-f][0-9a-f]/OBJID/" <actual >actual.fuzzy &&
# test_cmp expected actual.fuzzy &&
# check_head_differs_from fourth
#

expecting success:

	git checkout -f master &&
	git reset --hard first &&
	test_tick &&
	git cherry-pick --ff first..fourth &&
	git diff --quiet other &&
	git diff --quiet HEAD other &&
	check_head_equals fourth

HEAD is now at 0c72e4f first
ok 4 - cherry-pick --ff first..fourth works

expecting success:

	git checkout -f master &&
	git reset --hard first &&
	test_tick &&
	git cherry-pick -n first..fourth &&
	git diff --quiet other &&
	git diff --cached --quiet other &&
	git diff --quiet HEAD first

HEAD is now at 0c72e4f first
ok 5 - cherry-pick -n first..fourth works

expecting success:

	git checkout -f master &&
	git reset --hard fourth &&
	test_tick &&
	git revert first..fourth &&
	git diff --quiet first &&
	git diff --cached --quiet first &&
	git diff --quiet HEAD first

HEAD is now at 94d3184 fourth
[master c20509a] Revert "fourth"
Author: A U Thor <[email protected]>

 1 file changed, 0 insertions(+), 1 deletion(-)
[master 5c15a57] Revert "third"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)
[master 76d1a2e] Revert "second"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)

ok 6 - revert first..fourth works

expecting success:

	git checkout -f master &&
	git reset --hard fourth &&
	test_tick &&
	git revert ^first fourth &&
	git diff --quiet first &&
	git diff --cached --quiet first &&
	git diff --quiet HEAD first

HEAD is now at 94d3184 fourth
[master aa52aa6] Revert "fourth"
Author: A U Thor <[email protected]>

 1 file changed, 0 insertions(+), 1 deletion(-)
[master 7128690] Revert "third"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)
[master 2b0bc01] Revert "second"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)

ok 7 - revert ^first fourth works

expecting success:

	git checkout -f master &&
	git reset --hard fourth &&
	test_tick &&
	git revert fourth fourth~1 fourth~2 &&
	git diff --quiet first &&
	git diff --cached --quiet first &&
	git diff --quiet HEAD first

HEAD is now at 94d3184 fourth
[master e7406a3] Revert "fourth"
Author: A U Thor <[email protected]>

 1 file changed, 0 insertions(+), 1 deletion(-)
[master 0039f72] Revert "third"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)
[master 6fceae4] Revert "second"
 Author: A U Thor <[email protected]>
 1 file changed, 0 insertions(+), 1 deletion(-)

ok 8 - revert fourth fourth~1 fourth~2 works

expecting success:

	git checkout -f master &&
	git reset --hard first &&
	test_tick &&
	git cherry-pick -3 fourth &&
	git diff --quiet other &&
	git diff --quiet HEAD other &&
	check_head_differs_from fourth

HEAD is now at 0c72e4f first
[master de54420] second
Author: A U Thor <[email protected]>

 1 file changed, 1 insertion(+), 0 deletions(-)
[master 5fbc3d8] third
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)
[master 0f9850f] fourth
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)

ok 9 - cherry-pick -3 fourth works

expecting success:

	git checkout -f master &&
	git reset --hard first &&
	test_tick &&
	git rev-list --reverse first..fourth | git cherry-pick --stdin &&
	git diff --quiet other &&
	git diff --quiet HEAD other &&
	check_head_differs_from fourth

HEAD is now at 0c72e4f first
[master 058f589] second
Author: A U Thor <[email protected]>

 1 file changed, 1 insertion(+), 0 deletions(-)
[master 013cb83] third
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)
[master 79f9008] fourth
 Author: A U Thor <[email protected]>
 1 file changed, 1 insertion(+), 0 deletions(-)

ok 10 - cherry-pick --stdin works

# failed 2 among 10 test(s)
1..10

Duy

Heya,

Also, this is rather detrimental to the i18n effort methinks?

Cheers,

Sverre Rabbelier

If the goal if the i18n effort is also to produce grammatically correct output in all the supported languages then the tests that my patch would break would have to be rewritten anyway.

Jon

I think he means that auto-pluralization like this cannot be done in an i18n world, as many languages do not simply add "s". Your patch would have to use ngettext, something like this (totally untested and just copying a similar spot in suggest_reattach, as I have never done any i18n myself):

fprintf(options->file,

    Q_(" %d file changed,",
       " %d files changed",
       total_files),
    Q_(" %d insertion(+)",
       " %d insertions(+)",
       adds),
    Q_(" %d deletion(-)",
       " %d deletions(-)",
       dels),
    "\n",
    total_files, adds, dels);

And that gives translators a chance to specify the entire singular and plural versions separately.

-Peff

[snip]

I entirely agree. My point is only that the various tests that expect the current behavior will have to be changed whether the implementation of correct plurals uses my inferior method or the way more correct i18n method.

Jon

Heya,

Wouldn't it be a nice hack if we just solved problem through i18n then? Have all the plumbing see the current wording, but through i18n change it to something grammatically correct for the porcelain. Probably not possible, but a nice daydream :).

Cheers,

Sverre Rabbelier

I thought there was still some question of whether this text was something that should be script-parseable. If it is, then it shouldn't be i18n'd at all, nor should we lightly change the format with pluralization magic. And if it isn't, then we should definitely go the full i18n route. So in either case, the original patch isn't appropriate.

I don't have a strong opinion myself. I tend to lean towards i18n-ing it, because any scripts should be using --numstat to parse, anyway. OTOH, as Junio pointed out, we are matching the output of much older tools, so pre-git scripts might be written to read the --stat format. I've never seen such a script, and I have no idea how many there really are.

-Peff

Besides, as it was already said, this is an API.

That's not it.

The problem is that above code assumes that plural form can be formed by adding suffix, and it assumes that is only one plural form... both assumptions does not hold for non-English.

C.f. "Additional functions for plural forms" chapter in gettext info
page: http://www.gnu.org/s/hello/manual/gettext/Plural-forms.html

Jakub Narębski

I pretty much only looked at that one file and it
didn't look at quick glance like an API was being
used to internationalize git.

We can discuss the correct way to implement a change like this but the fact remains that whatever the implementation, the issue that Junio raised will remain. That is, what to do about all the places that presume the old incorrect output.

This problem isn't specific to git. I can easily imagine other open source projects that face, or will face, this problem.

Jon