[PATCH/WIP] git-svn: tweak log format to match "svn log" output

[PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Jonathan Nieder]

From: Jens Seidel <jensseidel@users.sf.net>

Tweak "git svn log" output to more closely match what svn produces.
In particular, if Locale::gettext is available, use that to produce
localized output using svn’s translations.

So now instead of 

 git:
 r2105 | viktor | 2010-03-19 13:40:54 +0100 (Fr, 19 Mär 2010) | 2 lines
 
 svn:
 r2105 | viktor | 2010-03-19 13:40:54 +0100 (Fr, 19. Mär 2010) | 1 Zeile

we get the much closer

 git:
 r2105 | viktor | 2010-03-19 13:40:54 +0100 (Fr, 19. Mär 2010) | 2 Zeilen
 
 svn:
 r2105 | viktor | 2010-03-19 13:40:54 +0100 (Fr, 19. Mär 2010) | 1 Zeile

and a script for munging to compare logs would only have to fuzz
out the extra blank lines in git svn’s commit log.

[jn: made Locale::gettext dependency optional; added a test script]

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jens Seidel wrote at http://bugs.debian.org/578915:
>> Jens Seidel wrote:

> Ah, git svn ignores any locale, this explains the problem.
>
>>> The dot stands for an ordinal number (e.g. "19." == "19th").
>>> Second: "line(s)" is not translated. I can workaround by starting git/svn in an
>>> English locale ...
>
> I tried to fix both.
>
>>> Third: the git log often contains an addional empty line.
>
> This remains open. Probably that can be solved by setting a git config
> option and resyncing all git svn history from git.

Sorry to leave this hanging for so long.  I think it is an improvement
as is, but I have not tested in the following scenarios:

 - Locale::gettext not available
 - subversion not translated
 - subversion translated but not to the current language

And the extra blank line at the end of log messages remains unsolved.

Thoughts?

diff --git i/git-svn.perl w/git-svn.perl
index c416358..55dc50d 100755
--- i/git-svn.perl
+++ w/git-svn.perl
@@ -45,6 +45,7 @@ sub _req_svn {
 	}
 }
 my $can_compress = eval { require Compress::Zlib; 1};
+my $can_localize = eval { require Locale::gettext; 1};
 push @Git::SVN::Ra::ISA, 'SVN::Ra';
 push @SVN::Git::Editor::ISA, 'SVN::Delta::Editor';
 push @SVN::Git::Fetcher::ISA, 'SVN::Delta::Editor';
@@ -5535,7 +5536,14 @@ sub format_svn_date {
 	my $gm = timelocal(gmtime($t));
 	my $sign = qw( + + - )[ $t <=> $gm ];
 	my $gmoff = sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
-	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
+	my $format;
+	if ($can_localize) {
+		my $d = Locale::gettext->domain("subversion");
+		$format = "%Y-%m-%d %H:%M:%S $gmoff" . $d->get(" (%a, %d %b %Y)");
+	} else {
+		$format = "%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)";
+	}
+	return strftime($format, localtime($t));
 }
 
 sub parse_git_date {
@@ -5631,9 +5639,16 @@ sub show_commit_normal {
 	my ($c) = @_;
 	print commit_log_separator, "r$c->{r} | ";
 	print "$c->{c} | " if $show_commit;
-	print "$c->{a} | ", format_svn_date($c->{t_utc}), ' | ';
+	print "$c->{a} | ", format_svn_date($c->{t_utc});
 	my $nr_line = 0;
+	my $sing_fmt = " | %d line";
+	my $plur_fmt = " | %d lines";
 
+	if ($can_localize) {
+		my $d = Locale::gettext->domain("subversion");
+		$sing_fmt = $d->nget(" | %d line", " | %d lines", 1);
+		$plur_fmt = $d->nget(" | %d line", " | %d lines", 10);
+	}
 	if (my $l = $c->{l}) {
 		while ($l->[$#$l] eq "\n" && $#$l > 0
 		                          && $l->[($#$l - 1)] eq "\n") {
@@ -5641,20 +5656,23 @@ sub show_commit_normal {
 		}
 		$nr_line = scalar @$l;
 		if (!$nr_line) {
-			print "1 line\n\n\n";
+			print sprintf($sing_fmt, 1), "\n\n\n";
 		} else {
 			if ($nr_line == 1) {
-				$nr_line = '1 line';
+				print sprintf($sing_fmt, $nr_line), "\n";
 			} else {
-				$nr_line .= ' lines';
+				print sprintf($plur_fmt, $nr_line), "\n";
 			}
-			print $nr_line, "\n";
 			show_commit_changed_paths($c);
 			print "\n";
 			print $_ foreach @$l;
 		}
 	} else {
-		print "1 line\n";
+		if ($nr_line == 1) {
+			print sprintf($sing_fmt, $nr_line), "\n";
+		} else {
+			print sprintf($plur_fmt, $nr_line), "\n";
+		}
 		show_commit_changed_paths($c);
 		print "\n";
 
diff --git i/t/t9116-git-svn-log.sh w/t/t9116-git-svn-log.sh
index 0374a74..586f64b 100755
--- i/t/t9116-git-svn-log.sh
+++ w/t/t9116-git-svn-log.sh
@@ -16,6 +16,7 @@ test_expect_success 'setup repository and import' '
 		done && \
 		svn_cmd import -m test . "$svnrepo"
 		cd .. &&
+	svn_cmd checkout "$svnrepo"/branches/a checkout &&
 	git svn init "$svnrepo" -T trunk -b branches -t tags &&
 	git svn fetch &&
 	git reset --hard trunk &&
@@ -36,7 +37,38 @@ test_expect_success 'setup repository and import' '
 	git commit -a -m spy &&
 	echo try >> README &&
 	git commit -a -m try &&
-	git svn dcommit
+	git svn dcommit &&
+	(
+		cd checkout &&
+		svn_cmd update
+	) &&
+
+	if test -n "$ORIGINAL_LANG" && test "$ORIGINAL_LANG" != C
+	then
+		test_set_prereq NONCLOCALE
+	fi
+	'
+
+test_expect_failure 'log matches svn log' '
+	git reset --hard a &&
+	(
+		cd checkout &&
+		svn_cmd log >../expected
+	) &&
+	git svn log >actual &&
+	test_cmp expected actual
+	'
+
+test_expect_failure NONCLOCALE 'log matches svn log, original locale' '
+	(
+		LC_ALL=$ORIGINAL_LANG &&
+		(
+			cd checkout &&
+			svn_cmd log >../expected
+		) &&
+		git svn log >actual
+	) &&
+	test_cmp expected actual
 	'
 
 test_expect_success 'run log' "
diff --git i/t/test-lib.sh w/t/test-lib.sh
index e5523dd..62aa48c 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -34,6 +34,9 @@ esac
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
 
+# Keep the original locale for tests
+ORIGINAL_LANG=$LANG
+
 # For repeatability, reset the environment to known value.
 LANG=C
 LC_ALL=C
-- 

Re: [PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Eric Wong]

Jonathan Nieder <jrnieder@gmail.com> wrote:
> From: Jens Seidel <jensseidel@users.sf.net>
> 
> Tweak "git svn log" output to more closely match what svn produces.
> In particular, if Locale::gettext is available, use that to produce
> localized output using svn’s translations.

<snip>

> [jn: made Locale::gettext dependency optional; added a test script]
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Jens Seidel wrote at http://bugs.debian.org/578915:
> >> Jens Seidel wrote:
> 
> > Ah, git svn ignores any locale, this explains the problem.
> >
> >>> The dot stands for an ordinal number (e.g. "19." == "19th").
> >>> Second: "line(s)" is not translated. I can workaround by starting git/svn in an
> >>> English locale ...
> >
> > I tried to fix both.
> >
> >>> Third: the git log often contains an addional empty line.
>
> > This remains open. Probably that can be solved by setting a git config
> > option and resyncing all git svn history from git.

Hi Jens, Jonathan,

I just made an additional patch that should solve the problem
(see my reply to this email).

> Sorry to leave this hanging for so long.  I think it is an improvement
> as is, but I have not tested in the following scenarios:
> 
>  - Locale::gettext not available
>  - subversion not translated
>  - subversion translated but not to the current language
> 
> And the extra blank line at the end of log messages remains unsolved.
> 
> Thoughts?

Everything looks reasonable to me.  gettext should just fall back to the
original string when a translation is not available.

I've squashed the following cleanup on top of Jens' original change:

diff --git a/git-svn.perl b/git-svn.perl
index 55dc50d..5252722 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5536,14 +5536,12 @@ sub format_svn_date {
 	my $gm = timelocal(gmtime($t));
 	my $sign = qw( + + - )[ $t <=> $gm ];
 	my $gmoff = sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
-	my $format;
+	my $format = " (%a, %d %b %Y)";
 	if ($can_localize) {
 		my $d = Locale::gettext->domain("subversion");
-		$format = "%Y-%m-%d %H:%M:%S $gmoff" . $d->get(" (%a, %d %b %Y)");
-	} else {
-		$format = "%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)";
+		$format = $d->get($format);
 	}
-	return strftime($format, localtime($t));
+	return strftime("%Y-%m-%d %H:%M:%S $gmoff$format", localtime($t));
 }
 
 sub parse_git_date {
@@ -5646,8 +5644,8 @@ sub show_commit_normal {
 
 	if ($can_localize) {
 		my $d = Locale::gettext->domain("subversion");
-		$sing_fmt = $d->nget(" | %d line", " | %d lines", 1);
-		$plur_fmt = $d->nget(" | %d line", " | %d lines", 10);
+		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
+		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
 	}
 	if (my $l = $c->{l}) {
 		while ($l->[$#$l] eq "\n" && $#$l > 0
-- 
Eric Wong

[PATCH 2/1] git svn: remove extraneous newline from log output

[Eric Wong]

This is to match the output of "svn log", which does not
add an extra newline before the next log entry.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---

The following are pushed to the master git svn repo at
git://git.bogomips.org/git-svn   Let me know if everything looks
alright and I'll ask Junio to pull.

	Eric Wong (1):
	      git svn: remove extraneous newline from log output

	Jens Seidel (1):
	      git-svn: tweak log format to match "svn log" output

 git-svn.perl           |    3 +++
 t/t9116-git-svn-log.sh |    4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 5252722..f9b7711 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5743,6 +5743,9 @@ sub cmd_show_log {
 			push @{$c->{stat}}, $_;
 			$stat = undef;
 		} elsif (/^${esc_color}    (git-svn-id:.+)$/o) {
+			if (@{$c->{l}} && $c->{l}->[-1] eq "\n") {
+				pop @{$c->{l}};
+			}
 			($c->{url}, $c->{r}, undef) = ::extract_metadata($1);
 		} elsif (s/^${esc_color}    //o) {
 			push @{$c->{l}}, $_;
diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
index 4394fb2..586f64b 100755
--- a/t/t9116-git-svn-log.sh
+++ b/t/t9116-git-svn-log.sh
@@ -49,7 +49,7 @@ test_expect_success 'setup repository and import' '
 	fi
 	'
 
-test_expect_failure 'log matches svn log' '
+test_expect_success 'log matches svn log' '
 	git reset --hard a &&
 	(
 		cd checkout &&
@@ -59,7 +59,7 @@ test_expect_failure 'log matches svn log' '
 	test_cmp expected actual
 	'
 
-test_expect_failure NONCLOCALE 'log matches svn log, original locale' '
+test_expect_success NONCLOCALE 'log matches svn log, original locale' '
 	(
 		LC_ALL=$ORIGINAL_LANG &&
 		(
-- 
Eric Wong

Re: [PATCH 2/1] git svn: remove extraneous newline from log output

[Jens Seidel]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

Hi Eric,

On Sun, Jul 25, 2010 at 08:13:44AM +0000, Eric Wong wrote:
> This is to match the output of "svn log", which does not
> add an extra newline before the next log entry.

thanks for the patch but it doesn't work in the general case.

I attached a sample Subversion repository dump where the output of
git svn log and svn log still differs by empty lines.

I also noticed the following error:
Can't use an undefined value as an ARRAY reference at                                                                                            
/usr/lib/git-core/git-svn line 5717, <$fh> line 53.

I'm not sure whether I introduced this error once I patched
git-svn ...
The affected line is the second one in:
           } elsif (/^${esc_color}    (git-svn-id:.+)$/o) {
                        if (@{$c->{l}} && $c->{l}->[-1] eq "\n") {
                                pop @{$c->{l}};
                        }
(is -1 valid?)

Jens 

[-- Attachment #2: svn.log --]
[-- Type: text/plain, Size: 851 bytes --]

------------------------------------------------------------------------
r5 | jens | 2010-07-25 19:08:18 +0200 (So, 25. Jul 2010) | 4 lines

2 trailing newlines



------------------------------------------------------------------------
r4 | jens | 2010-07-25 19:07:42 +0200 (So, 25. Jul 2010) | 3 lines

trailing newline


------------------------------------------------------------------------
r3 | jens | 2010-07-25 19:06:58 +0200 (So, 25. Jul 2010) | 3 lines

one linebreak
line 2

------------------------------------------------------------------------
r2 | jens | 2010-07-25 19:06:05 +0200 (So, 25. Jul 2010) | 1 line

linebreak
------------------------------------------------------------------------
r1 | jens | 2010-07-25 19:05:21 +0200 (So, 25. Jul 2010) | 1 line


------------------------------------------------------------------------

[-- Attachment #3: git-svn.log --]
[-- Type: text/plain, Size: 740 bytes --]

------------------------------------------------------------------------
r5 | jens | 2010-07-25 19:08:18 +0200 (So, 25. Jul 2010) | 2 lines

2 trailing newlines

------------------------------------------------------------------------
r4 | jens | 2010-07-25 19:07:42 +0200 (So, 25. Jul 2010) | 2 lines

trailing newline

------------------------------------------------------------------------
r3 | jens | 2010-07-25 19:06:58 +0200 (So, 25. Jul 2010) | 3 lines

one linebreak
line 2

------------------------------------------------------------------------
r2 | jens | 2010-07-25 19:06:05 +0200 (So, 25. Jul 2010) | 1 line

linebreak
Can't use an undefined value as an ARRAY reference at
/usr/lib/git-core/git-svn line 5717, <$fh> line 53.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: repos.dump --]
[-- Type: text/plain; charset=base64, Size: 1937 bytes --]

SVN-fs-dump-format-version: 2

UUID: 83f09cc9-4635-489b-9736-9fad07b44175

Revision-number: 0
Prop-content-length: 56
Content-length: 56

K 8
svn:date
V 27
2010-07-25T17:03:46.899612Z
PROPS-END

Revision-number: 1
Prop-content-length: 98
Content-length: 98

K 7
svn:log
V 0

K 10
svn:author
V 4
jens
K 8
svn:date
V 27
2010-07-25T17:05:21.869208Z
PROPS-END

Node-path: file
Node-kind: file
Node-action: add
Prop-content-length: 10
Text-content-length: 0
Text-content-md5: d41d8cd98f00b204e9800998ecf8427e
Content-length: 10

PROPS-END


Revision-number: 2
Prop-content-length: 107
Content-length: 107

K 7
svn:log
V 9
linebreak
K 10
svn:author
V 4
jens
K 8
svn:date
V 27
2010-07-25T17:06:05.643604Z
PROPS-END

Node-path: file
Node-kind: file
Node-action: change
Text-content-length: 13
Text-content-md5: dcf4e97bd1c01e863bf8989b23429f9c
Content-length: 13

no linebreak


Revision-number: 3
Prop-content-length: 120
Content-length: 120

K 7
svn:log
V 21
one linebreak
line 2

K 10
svn:author
V 4
jens
K 8
svn:date
V 27
2010-07-25T17:06:58.264468Z
PROPS-END

Node-path: file
Node-kind: file
Node-action: change
Text-content-length: 21
Text-content-md5: 4e70c8b04910e92f9cc7fbd6424352f0
Content-length: 21

one linebreak
line 2


Revision-number: 4
Prop-content-length: 117
Content-length: 117

K 7
svn:log
V 18
trailing newline


K 10
svn:author
V 4
jens
K 8
svn:date
V 27
2010-07-25T17:07:42.901800Z
PROPS-END

Node-path: file
Node-kind: file
Node-action: change
Text-content-length: 19
Text-content-md5: b20aeff36821448c3bc3c85942bc1fc5
Content-length: 19

trailing newline




Revision-number: 5
Prop-content-length: 121
Content-length: 121

K 7
svn:log
V 22
2 trailing newlines



K 10
svn:author
V 4
jens
K 8
svn:date
V 27
2010-07-25T17:08:18.343554Z
PROPS-END

Node-path: file
Node-kind: file
Node-action: change
Text-content-length: 23
Text-content-md5: e1314ce7619ef512653071d690dbd789
Content-length: 23

2 trailing newlines

Re: [PATCH 2/1] git svn: remove extraneous newline from log output

[Eric Wong]

Jens Seidel <jensseidel@users.sf.net> wrote:
> Hi Eric,
> 
> On Sun, Jul 25, 2010 at 08:13:44AM +0000, Eric Wong wrote:
> > This is to match the output of "svn log", which does not
> > add an extra newline before the next log entry.
> 
> thanks for the patch but it doesn't work in the general case.
> 
> I attached a sample Subversion repository dump where the output of
> git svn log and svn log still differs by empty lines.
> 
> I also noticed the following error:
> Can't use an undefined value as an ARRAY reference at                                                                                            
> /usr/lib/git-core/git-svn line 5717, <$fh> line 53.
> 
> I'm not sure whether I introduced this error once I patched
> git-svn ...
> The affected line is the second one in:
>            } elsif (/^${esc_color}    (git-svn-id:.+)$/o) {
>                         if (@{$c->{l}} && $c->{l}->[-1] eq "\n") {
>                                 pop @{$c->{l}};
>                         }
> (is -1 valid?)

Yes, -1 is valid but $c->{l} may not exist at that point..

But I had forgotten about a similar piece of code in the
"show_commit_normal" subroutine which does a similar job:

		while ($l->[$#$l] eq "\n" && $#$l > 0
		                          && $l->[($#$l - 1)] eq "\n") {
			pop @$l;
		}

This is partially because SVN doesn't enforce a trailing "\n" in
commit messages (unlike "git commit"), so we always append one
when we fetch and add "\ngit-svn-id: ...\n".  But then we also
need to take noMetadata users into account, too.

I'll let you ponder issues with this since you appear to care more than
I do about completely matching SVN output :)


For what it's worth, over the years, nobody ever "outed" me as a git-svn
user in SVN projects when I shared log output.  Some folks worked with
for years in the same offices and never made indication to knowing I
used git-svn (even after they started using it themselves).

-- 
Eric Wong

Re: [PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Jonathan Nieder]

Eric Wong wrote:

> @@ -5646,8 +5644,8 @@ sub show_commit_normal {
>  
>  	if ($can_localize) {
>  		my $d = Locale::gettext->domain("subversion");
> -		$sing_fmt = $d->nget(" | %d line", " | %d lines", 1);
> -		$plur_fmt = $d->nget(" | %d line", " | %d lines", 10);
> +		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
> +		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);

This change gives me pause: wouldn’t it be a problem in
non-English locales, where $sing_fmt changes before it is
used to set $plur_fmt?

Of course there is no problem because the code is equivalent to

		$sing_fmt = $d->get($sing_fmt);
		$plur_fmt = $d->get($plur_fmt);

(or at least Locale::gettext is advertised to work that way.)

diff --git i/git-svn.perl w/git-svn.perl
index 5252722..75e7165 100755
--- i/git-svn.perl
+++ w/git-svn.perl
@@ -5644,8 +5644,8 @@ sub show_commit_normal {
 
 	if ($can_localize) {
 		my $d = Locale::gettext->domain("subversion");
-		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
-		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
+		$sing_fmt = $d->get($sing_fmt);
+		$plur_fmt = $d->get($plur_fmt);
 	}
 	if (my $l = $c->{l}) {
 		while ($l->[$#$l] eq "\n" && $#$l > 0
-- 

Re: [PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Eric Wong]

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Eric Wong wrote:
> 
> > @@ -5646,8 +5644,8 @@ sub show_commit_normal {
> >  
> >  	if ($can_localize) {
> >  		my $d = Locale::gettext->domain("subversion");
> > -		$sing_fmt = $d->nget(" | %d line", " | %d lines", 1);
> > -		$plur_fmt = $d->nget(" | %d line", " | %d lines", 10);
> > +		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
> > +		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
> 
> This change gives me pause: wouldn’t it be a problem in
> non-English locales, where $sing_fmt changes before it is
> used to set $plur_fmt?
> 
> Of course there is no problem because the code is equivalent to
> 
> 		$sing_fmt = $d->get($sing_fmt);
> 		$plur_fmt = $d->get($plur_fmt);
> 
> (or at least Locale::gettext is advertised to work that way.)
> 
> diff --git i/git-svn.perl w/git-svn.perl
> index 5252722..75e7165 100755
> --- i/git-svn.perl
> +++ w/git-svn.perl
> @@ -5644,8 +5644,8 @@ sub show_commit_normal {
>  
>  	if ($can_localize) {
>  		my $d = Locale::gettext->domain("subversion");
> -		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
> -		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
> +		$sing_fmt = $d->get($sing_fmt);
> +		$plur_fmt = $d->get($plur_fmt);
>  	}
>  	if (my $l = $c->{l}) {
>  		while ($l->[$#$l] eq "\n" && $#$l > 0

Thanks Jonathan, that looks much better.  I'll squash that on top
Jens' first patch.

-- 
Eric Wong

Re: [PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Jens Seidel]

On Sun, Jul 25, 2010 at 08:40:57AM +0000, Eric Wong wrote:
> Jonathan Nieder <jrnieder@gmail.com> wrote:
> > diff --git i/git-svn.perl w/git-svn.perl
> > index 5252722..75e7165 100755
> > --- i/git-svn.perl
> > +++ w/git-svn.perl
> > @@ -5644,8 +5644,8 @@ sub show_commit_normal {
> >  
> >  	if ($can_localize) {
> >  		my $d = Locale::gettext->domain("subversion");
> > -		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
> > -		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
> > +		$sing_fmt = $d->get($sing_fmt);
> > +		$plur_fmt = $d->get($plur_fmt);
> >  	}
> >  	if (my $l = $c->{l}) {
> >  		while ($l->[$#$l] eq "\n" && $#$l > 0
> 
> Thanks Jonathan, that looks much better.  I'll squash that on top
> Jens' first patch.

No, this is still wrong, we have to use $d->nget as some languages can have
multiple plural forms. Russian e.g. has two: one if the last digit ends with
2-4 and a second one if it ends with 5-9. Exact rule:
"Plural-Forms:  nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 &&
n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"

That's why it's wrong to use $plur_fmt at all (and also "10" from above
makes no sense). Let's use:
$d->nget(" | %d line", " | %d lines", $nr_line), $nr_line)
where $nr_line is the variable.

Sorry for not providing a patch now but I need more time ... I suggest one
of you try it again and I check it :-)

Jens

Re: [PATCH/WIP] git-svn: tweak log format to match "svn log" output

[Jens Seidel]

[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]

On Sun, Jul 25, 2010 at 11:09:46AM +0200, Jens Seidel wrote:
> No, this is still wrong, we have to use $d->nget as some languages can have
> multiple plural forms. Russian e.g. has two: one if the last digit ends with
> 2-4 and a second one if it ends with 5-9. Exact rule:
> "Plural-Forms:  nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 &&
> n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
> 
> That's why it's wrong to use $plur_fmt at all (and also "10" from above
> makes no sense). Let's use:
> $d->nget(" | %d line", " | %d lines", $nr_line), $nr_line)
> where $nr_line is the variable.
> 
> Sorry for not providing a patch now but I need more time ... I suggest one
> of you try it again and I check it :-)

Here is a new patch:

(Please check the last stanza, Jonathan introduced $nr_line for the
output where 1 was used before. I reverted it.)

Since the partial patches get difficult to handle I attached also the
combined patch which includes everything from this thread.

And a further minor issue. The current locale for the test is extracted from
$LANG. It's possible that this variable isn't set but another one such as
LC_MESSAGES. So the test doesn't determine the locale but $LANG :-))

commit 239c97380df384162ebe8399754d072300d8b52f
Author: Jens Seidel <jensseidel@users.sf.net>
Date:   Sun Jul 25 18:35:28 2010 +0200

    Proper use of plural forms

diff --git a/git-svn.perl b/git-svn.perl
index f9b7711..71c2c9a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5639,14 +5639,22 @@ sub show_commit_normal {
 	print "$c->{c} | " if $show_commit;
 	print "$c->{a} | ", format_svn_date($c->{t_utc});
 	my $nr_line = 0;
-	my $sing_fmt = " | %d line";
-	my $plur_fmt = " | %d lines";
 
-	if ($can_localize) {
-		my $d = Locale::gettext->domain("subversion");
-		$sing_fmt = $d->nget($sing_fmt, $plur_fmt, 1);
-		$plur_fmt = $d->nget($sing_fmt, $plur_fmt, 10);
-	}
+	# returns " | $1 lines" properly translated
+	local *get_line_msg = sub {
+		my $n = shift;
+		if ($can_localize) {
+			my $d = Locale::gettext->domain("subversion");
+			return sprintf($d->nget(" | %d line", " | %d lines", $n), $n);
+		} else {
+			if ($n == 1) {
+				return " | 1 line";
+			} else {
+				return " | " . $n . " lines";
+			}
+		}
+	};
+
 	if (my $l = $c->{l}) {
 		while ($l->[$#$l] eq "\n" && $#$l > 0
 		                          && $l->[($#$l - 1)] eq "\n") {
@@ -5654,23 +5662,15 @@ sub show_commit_normal {
 		}
 		$nr_line = scalar @$l;
 		if (!$nr_line) {
-			print sprintf($sing_fmt, 1), "\n\n\n";
+			print get_line_msg(1), "\n\n\n"; 
 		} else {
-			if ($nr_line == 1) {
-				print sprintf($sing_fmt, $nr_line), "\n";
-			} else {
-				print sprintf($plur_fmt, $nr_line), "\n";
-			}
+			print get_line_msg($nr_line), "\n";
 			show_commit_changed_paths($c);
 			print "\n";
 			print $_ foreach @$l;
 		}
 	} else {
-		if ($nr_line == 1) {
-			print sprintf($sing_fmt, $nr_line), "\n";
-		} else {
-			print sprintf($plur_fmt, $nr_line), "\n";
-		}
+		print get_line_msg(1), "\n";
 		show_commit_changed_paths($c);
 		print "\n";
 

[-- Attachment #2: git-svn.diff --]
[-- Type: text/x-diff, Size: 4112 bytes --]

diff --git a/git-svn.perl b/git-svn.perl
index c416358..71c2c9a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -45,6 +45,7 @@ sub _req_svn {
 	}
 }
 my $can_compress = eval { require Compress::Zlib; 1};
+my $can_localize = eval { require Locale::gettext; 1};
 push @Git::SVN::Ra::ISA, 'SVN::Ra';
 push @SVN::Git::Editor::ISA, 'SVN::Delta::Editor';
 push @SVN::Git::Fetcher::ISA, 'SVN::Delta::Editor';
@@ -5535,7 +5536,12 @@ sub format_svn_date {
 	my $gm = timelocal(gmtime($t));
 	my $sign = qw( + + - )[ $t <=> $gm ];
 	my $gmoff = sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
-	return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
+	my $format = " (%a, %d %b %Y)";
+	if ($can_localize) {
+		my $d = Locale::gettext->domain("subversion");
+		$format = $d->get($format);
+	}
+	return strftime("%Y-%m-%d %H:%M:%S $gmoff$format", localtime($t));
 }
 
 sub parse_git_date {
@@ -5631,9 +5637,24 @@ sub show_commit_normal {
 	my ($c) = @_;
 	print commit_log_separator, "r$c->{r} | ";
 	print "$c->{c} | " if $show_commit;
-	print "$c->{a} | ", format_svn_date($c->{t_utc}), ' | ';
+	print "$c->{a} | ", format_svn_date($c->{t_utc});
 	my $nr_line = 0;
 
+	# returns " | $1 lines" properly translated
+	local *get_line_msg = sub {
+		my $n = shift;
+		if ($can_localize) {
+			my $d = Locale::gettext->domain("subversion");
+			return sprintf($d->nget(" | %d line", " | %d lines", $n), $n);
+		} else {
+			if ($n == 1) {
+				return " | 1 line";
+			} else {
+				return " | " . $n . " lines";
+			}
+		}
+	};
+
 	if (my $l = $c->{l}) {
 		while ($l->[$#$l] eq "\n" && $#$l > 0
 		                          && $l->[($#$l - 1)] eq "\n") {
@@ -5641,20 +5662,15 @@ sub show_commit_normal {
 		}
 		$nr_line = scalar @$l;
 		if (!$nr_line) {
-			print "1 line\n\n\n";
+			print get_line_msg(1), "\n\n\n"; 
 		} else {
-			if ($nr_line == 1) {
-				$nr_line = '1 line';
-			} else {
-				$nr_line .= ' lines';
-			}
-			print $nr_line, "\n";
+			print get_line_msg($nr_line), "\n";
 			show_commit_changed_paths($c);
 			print "\n";
 			print $_ foreach @$l;
 		}
 	} else {
-		print "1 line\n";
+		print get_line_msg(1), "\n";
 		show_commit_changed_paths($c);
 		print "\n";
 
@@ -5727,6 +5743,9 @@ sub cmd_show_log {
 			push @{$c->{stat}}, $_;
 			$stat = undef;
 		} elsif (/^${esc_color}    (git-svn-id:.+)$/o) {
+			if (@{$c->{l}} && $c->{l}->[-1] eq "\n") {
+				pop @{$c->{l}};
+			}
 			($c->{url}, $c->{r}, undef) = ::extract_metadata($1);
 		} elsif (s/^${esc_color}    //o) {
 			push @{$c->{l}}, $_;
diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
index 0374a74..586f64b 100755
--- a/t/t9116-git-svn-log.sh
+++ b/t/t9116-git-svn-log.sh
@@ -16,6 +16,7 @@ test_expect_success 'setup repository and import' '
 		done && \
 		svn_cmd import -m test . "$svnrepo"
 		cd .. &&
+	svn_cmd checkout "$svnrepo"/branches/a checkout &&
 	git svn init "$svnrepo" -T trunk -b branches -t tags &&
 	git svn fetch &&
 	git reset --hard trunk &&
@@ -36,7 +37,38 @@ test_expect_success 'setup repository and import' '
 	git commit -a -m spy &&
 	echo try >> README &&
 	git commit -a -m try &&
-	git svn dcommit
+	git svn dcommit &&
+	(
+		cd checkout &&
+		svn_cmd update
+	) &&
+
+	if test -n "$ORIGINAL_LANG" && test "$ORIGINAL_LANG" != C
+	then
+		test_set_prereq NONCLOCALE
+	fi
+	'
+
+test_expect_success 'log matches svn log' '
+	git reset --hard a &&
+	(
+		cd checkout &&
+		svn_cmd log >../expected
+	) &&
+	git svn log >actual &&
+	test_cmp expected actual
+	'
+
+test_expect_success NONCLOCALE 'log matches svn log, original locale' '
+	(
+		LC_ALL=$ORIGINAL_LANG &&
+		(
+			cd checkout &&
+			svn_cmd log >../expected
+		) &&
+		git svn log >actual
+	) &&
+	test_cmp expected actual
 	'
 
 test_expect_success 'run log' "
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e5523dd..62aa48c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -34,6 +34,9 @@ esac
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
 
+# Keep the original locale for tests
+ORIGINAL_LANG=$LANG
+
 # For repeatability, reset the environment to known value.
 LANG=C
 LC_ALL=C