[PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[Fredrik Kuivinen]

Signed-off-by: Fredrik Kuivinen <freku045@student.liu.se>


---

 t/annotate-tests.sh |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t8001-annotate.sh |   85 +-------------------------------------------------
 t/t8002-blame.sh    |    9 +++++
 3 files changed, 97 insertions(+), 83 deletions(-)
 create mode 100644 t/annotate-tests.sh
 create mode 100755 t/t8002-blame.sh

06b0e500a5202899dcfd037cf78ee4a982da46b4
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
new file mode 100644
index 0000000..54a4dfb
--- /dev/null
+++ b/t/annotate-tests.sh
@@ -0,0 +1,86 @@
+# This file isn't used as a test script directly, instead it is
+# sourced from t8001-annotate.sh and t8001-blame.sh.
+
+test_expect_success \
+    'prepare reference tree' \
+    'echo "1A quick brown fox jumps over the" >file &&
+     echo "lazy dog" >>file &&
+     git add file
+     GIT_AUTHOR_NAME="A" git commit -a -m "Initial."'
+
+test_expect_success \
+    'check all lines blamed on A' \
+    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
+
+test_expect_success \
+    'Setup new lines blamed on B' \
+    'echo "2A quick brown fox jumps over the" >>file &&
+     echo "lazy dog" >> file &&
+     GIT_AUTHOR_NAME="B" git commit -a -m "Second."'
+
+test_expect_success \
+    'Two lines blamed on A' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "A") == 2 ]'
+
+test_expect_success \
+    'Two lines blamed on B' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "B") == 2 ]'
+
+test_expect_success \
+    'merge-setup part 1' \
+    'git checkout -b branch1 master &&
+     echo "3A slow green fox jumps into the" >> file &&
+     echo "well." >> file &&
+     GIT_AUTHOR_NAME="B1" git commit -a -m "Branch1-1"'
+
+test_expect_success \
+    'Two lines blamed on A' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
+
+test_expect_success \
+    'Two lines blamed on B' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 2 ]'
+
+test_expect_success \
+    'Two lines blamed on B1' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
+
+test_expect_success \
+    'merge-setup part 2' \
+    'git checkout -b branch2 master &&
+     sed -e "s/2A quick brown/4A quick brown lazy dog/" < file > file.new &&
+     mv file.new file &&
+     GIT_AUTHOR_NAME="B2" git commit -a -m "Branch2-1"'
+
+test_expect_success \
+    'Two lines blamed on A' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
+
+test_expect_success \
+    'One line blamed on B' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
+
+test_expect_success \
+    'One line blamed on B2' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
+
+
+test_expect_success \
+    'merge-setup part 3' \
+    'git pull . branch1'
+
+test_expect_success \
+    'Two lines blamed on A' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
+
+test_expect_success \
+    'One line blamed on B' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
+
+test_expect_success \
+    'Two lines blamed on B1' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
+
+test_expect_success \
+    'One line blamed on B2' \
+    '[ $($PROG file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
diff --git a/t/t8001-annotate.sh b/t/t8001-annotate.sh
index 172908a..9e5a04b 100755
--- a/t/t8001-annotate.sh
+++ b/t/t8001-annotate.sh
@@ -3,88 +3,7 @@
 test_description='git-annotate'
 . ./test-lib.sh
 
-test_expect_success \
-    'prepare reference tree' \
-    'echo "1A quick brown fox jumps over the" >file &&
-     echo "lazy dog" >>file &&
-     git add file
-     GIT_AUTHOR_NAME="A" git commit -a -m "Initial."'
-
-test_expect_success \
-    'check all lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
-
-test_expect_success \
-    'Setup new lines blamed on B' \
-    'echo "2A quick brown fox jumps over the" >>file &&
-     echo "lazy dog" >> file &&
-     GIT_AUTHOR_NAME="B" git commit -a -m "Second."'
-
-test_expect_success \
-    'Two lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "B") == 2 ]'
-
-test_expect_success \
-    'merge-setup part 1' \
-    'git checkout -b branch1 master &&
-     echo "3A slow green fox jumps into the" >> file &&
-     echo "well." >> file &&
-     GIT_AUTHOR_NAME="B1" git commit -a -m "Branch1-1"'
-
-test_expect_success \
-    'Two lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B$") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B1' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
-
-test_expect_success \
-    'merge-setup part 2' \
-    'git checkout -b branch2 master &&
-     sed -e "s/2A quick brown/4A quick brown lazy dog/" < file > file.new &&
-     mv file.new file &&
-     GIT_AUTHOR_NAME="B2" git commit -a -m "Branch2-1"'
-
-test_expect_success \
-    'Two lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
-
-test_expect_success \
-    'One line blamed on B' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
-
-test_expect_success \
-    'One line blamed on B2' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
-
-
-test_expect_success \
-    'merge-setup part 3' \
-    'git pull . branch1'
-
-test_expect_success \
-    'Two lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
-
-test_expect_success \
-    'One line blamed on B' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
-
-test_expect_success \
-    'Two lines blamed on B1' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
-
-test_expect_success \
-    'One line blamed on B2' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
+PROG='git annotate'
+source ../annotate-tests.sh
 
 test_done
diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
new file mode 100755
index 0000000..1036c54
--- /dev/null
+++ b/t/t8002-blame.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+
+test_description='git-blame'
+. ./test-lib.sh
+
+PROG='git blame -c'
+source ../annotate-tests.sh
+
+test_done
-- 
1.2.4.g4644-dirty

Re: [PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[Ryan Anderson]

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


Along these lines, if anyone can pin down the complicated cases that
annotate and blame get differently, adding them as a test would be
*exceedingly* appreciated, even if it makes annotate (or blame) fail for
a bit, it gives us something to work against.

I've been trying to find some time this weekend to dig into why annotate
gets things wrong, but I've been distracted, unfortunately.


-- 

Ryan Anderson
  sometimes Pug Majere


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

> Along these lines, if anyone can pin down the complicated cases that
> annotate and blame get differently, adding them as a test would be
> *exceedingly* appreciated, even if it makes annotate (or blame) fail for
> a bit, it gives us something to work against.

In the t/trash repository, "git-annotate file master" and
"git-annotate file master^" behaves funkily.

OTOH "git-blame file master" assigns everything to that commit
while only the last two lines are attributable to it.  Needs a
bit more work on both sides ;-).

Re: [PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[Martin Langhoff]

On 3/6/06, Ryan Anderson <ryan@michonline.com> wrote:
> Along these lines, if anyone can pin down the complicated cases that
> annotate and blame get differently, adding them as a test would be
> *exceedingly* appreciated, even if it makes annotate (or blame) fail for
> a bit, it gives us something to work against.

It would be great if they both worked properly with paths that existed
in the past. Right now, I can't git-annotate or git-blame a file I
know was there in a past revision. I think I had taught Johannes'
annotate to deal with this, or at least intended to. As things stand,
git-annotate/git-blame need a checkout, which is really silly.

Tools like gitweb and cvsserver should be able to do:

  GIT_DIR=/somebarerepo/git.git git-(annotate|blame) -h headname Makefile

and not worry about creating a temporary index *and* checking out
Makefile so that things work. That's what cvsserver does now, anyway.
Ugly :-(

cheers,



m

[PATCH] annotate: Support annotation of files on other revisions.

[Ryan Anderson]

This is a bug fix, and cleans up one or two other things spotted during the
course of tracking down the main bug here.

Also, the test-suite is updated to reflect this case.

Signed-off-by: Ryan Anderson <ryan@michonline.com>

---

 git-annotate.perl   |    7 +++++--
 t/t8001-annotate.sh |    6 ++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

On Mon, Mar 06, 2006 at 01:29:32PM +1300, Martin Langhoff wrote:
> On 3/6/06, Ryan Anderson <ryan@michonline.com> wrote:
> > Along these lines, if anyone can pin down the complicated cases that
> > annotate and blame get differently, adding them as a test would be
> > *exceedingly* appreciated, even if it makes annotate (or blame) fail for
> > a bit, it gives us something to work against.
> 
> It would be great if they both worked properly with paths that existed
> in the past. Right now, I can't git-annotate or git-blame a file I
> know was there in a past revision. I think I had taught Johannes'
> annotate to deal with this, or at least intended to. As things stand,
> git-annotate/git-blame need a checkout, which is really silly.

annotate was *trying* to handle that cleanly, but failed due to a silly
bug.  This is the same bug that Junio pointed out:
	cd t
	./t8001-annotate.sh
	cd trash
	../../git-annotate file master

This is fixed by the patch at the end of this email.

> Tools like gitweb and cvsserver should be able to do:
> 
>   GIT_DIR=/somebarerepo/git.git git-(annotate|blame) -h headname Makefile
> 
> and not worry about creating a temporary index *and* checking out
> Makefile so that things work. That's what cvsserver does now, anyway.
> Ugly :-(

For annotate, the syntax I was using was:
	git annotate Makefile headname

I'm not married to it, so please, send a patch to change it if you want
(Please fix up the test case I'm sending in this patch, as well.)

cdd80fd28d300dd2400bf75ff64ae2bf1a8b92aa
diff --git a/git-annotate.perl b/git-annotate.perl
index d93ee19..5953ac6 100755
--- a/git-annotate.perl
+++ b/git-annotate.perl
@@ -10,6 +10,7 @@ use warnings;
 use strict;
 use Getopt::Long;
 use POSIX qw(strftime gmtime);
+use Data::Dumper;
 
 sub usage() {
 	print STDERR 'Usage: ${\basename $0} [-s] [-S revs-file] file [ revision ]
@@ -99,7 +100,7 @@ while (my $bound = pop @stack) {
 	}
 }
 push @revqueue, $head;
-init_claim( defined $starting_rev ? $starting_rev : 'dirty');
+init_claim( defined $starting_rev ? $head : 'dirty');
 unless (defined $starting_rev) {
 	my $diff = open_pipe("git","diff","-R", "HEAD", "--",$filename)
 		or die "Failed to call git diff to check for dirty state: $!";
@@ -345,6 +346,7 @@ sub git_cat_file {
 	return () unless defined $rev && defined $filename;
 
 	my $blob = git_ls_tree($rev, $filename);
+	die "Failed to find a blob for $filename in rev $rev\n" if !defined $blob;
 
 	my $catfile = open_pipe("git","cat-file", "blob", $blob)
 		or die "Failed to git-cat-file blob $blob (rev $rev, file $filename): " . $!;
@@ -367,12 +369,13 @@ sub git_ls_tree {
 
 	my ($mode, $type, $blob, $tfilename);
 	while(<$lstree>) {
+		chomp;
 		($mode, $type, $blob, $tfilename) = split(/\s+/, $_, 4);
 		last if ($tfilename eq $filename);
 	}
 	close($lstree);
 
-	return $blob if $filename eq $filename;
+	return $blob if ($tfilename eq $filename);
 	die "git-ls-tree failed to find blob for $filename";
 
 }
diff --git a/t/t8001-annotate.sh b/t/t8001-annotate.sh
index 172908a..761d0bc 100755
--- a/t/t8001-annotate.sh
+++ b/t/t8001-annotate.sh
@@ -87,4 +87,10 @@ test_expect_success \
     'One line blamed on B2' \
     '[ $(git annotate file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
 
+test_expect_success \
+    'Annotating an old revision works' \
+    '[ $(git annotate file master | awk "{print \$3}" | grep -c "^A$") == 2 ] && \
+     [ $(git annotate file master | awk "{print \$3}" | grep -c "^B$") == 2 ]'
+
+
 test_done
-- 
1.2.4.g9201-dirty



-- 

Ryan Anderson
  sometimes Pug Majere

Re: [PATCH] git-blame: Use the same tests for git-blame as for git-annotate

[Ryan Anderson]

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

Fredrik Kuivinen wrote:

>Signed-off-by: Fredrik Kuivinen <freku045@student.liu.se>
>
>
>---
>
> t/annotate-tests.sh |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++
> t/t8001-annotate.sh |   85 +-------------------------------------------------
> t/t8002-blame.sh    |    9 +++++
> 3 files changed, 97 insertions(+), 83 deletions(-)
> create mode 100644 t/annotate-tests.sh
> create mode 100755 t/t8002-blame.sh
>
>06b0e500a5202899dcfd037cf78ee4a982da46b4
>diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
>new file mode 100644
>index 0000000..54a4dfb
>--- /dev/null
>+++ b/t/annotate-tests.sh
>@@ -0,0 +1,86 @@
>+# This file isn't used as a test script directly, instead it is
>+# sourced from t8001-annotate.sh and t8001-blame.sh.
>+
>+test_expect_success \
>+    'prepare reference tree' \
>+    'echo "1A quick brown fox jumps over the" >file &&
>+     echo "lazy dog" >>file &&
>+     git add file
>+     GIT_AUTHOR_NAME="A" git commit -a -m "Initial."'
>+
>+test_expect_success \
>+    'check all lines blamed on A' \
>+    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
>  
>
This should be $PROG as well, I suspect.

Also, we need to agree on a syntax for working on non-HEAD revisions.

"git annotate $file $commitish" is what I had been using, but it's
really not something I feel strongly about.


-- 

Ryan Anderson
  sometimes Pug Majere


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Martin Langhoff]

On 3/6/06, Ryan Anderson <ryan@michonline.com> wrote:
> annotate was *trying* to handle that cleanly, but failed due to a silly

Great stuff, thanks! I'll let it hit master and then I'll drop the
messy part of req_annotate() in cvsserver.

> For annotate, the syntax I was using was:
>         git annotate Makefile headname
>
> I'm not married to it, so please, send a patch to change it if you want
> (Please fix up the test case I'm sending in this patch, as well.)

That's _perfect_. I was just making the syntax up.

cheers,


martin

Re: [PATCH] annotate: Support annotation of files on other revisions.

[A Large Angry SCM]

Ryan Anderson wrote:
...
> 
> For annotate, the syntax I was using was:
> 	git annotate Makefile headname
> 
> I'm not married to it, so please, send a patch to change it if you want
> (Please fix up the test case I'm sending in this patch, as well.)
> 

Wouldn't
	git annotate <headname> <filename(s?)>
be more consistent with other git commands?

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Ryan Anderson]

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

A Large Angry SCM wrote:

> Ryan Anderson wrote:
> ...
>
>>
>> For annotate, the syntax I was using was:
>>     git annotate Makefile headname
>>
>> I'm not married to it, so please, send a patch to change it if you want
>> (Please fix up the test case I'm sending in this patch, as well.)
>>
>
> Wouldn't
>     git annotate <headname> <filename(s?)>
> be more consistent with other git commands?
>
Yes, but <headname> (really, a commitish) needs to be optional.

I should probably switch to:
    git annotate [-c|--commit <committish>] <filename>
but that's partly why I'm asking for feedback.


-- 

Ryan Anderson
  sometimes Pug Majere


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Shawn Pearce]

Ryan Anderson <ryan@michonline.com> wrote:
> A Large Angry SCM wrote:
> 
> > Ryan Anderson wrote:
> > ...
> >
> >>
> >> For annotate, the syntax I was using was:
> >>     git annotate Makefile headname
> >>
> >> I'm not married to it, so please, send a patch to change it if you want
> >> (Please fix up the test case I'm sending in this patch, as well.)
> >>
> >
> > Wouldn't
> >     git annotate <headname> <filename(s?)>
> > be more consistent with other git commands?
> >
> Yes, but <headname> (really, a commitish) needs to be optional.
> 
> I should probably switch to:
>     git annotate [-c|--commit <committish>] <filename>
> but that's partly why I'm asking for feedback.

Yes but doesn't git-diff accept:

	git-diff Makefile
	git-diff HEAD Makefile

?  (Which is rather ugly as what if you have a tracked file actually
called HEAD and you want the first form when the commit-ish is
omitted.)  So accepting an optional commit-ish before the filename
would be in line with what git-diff accepts today.

But maybe breaking convention from git-diff now is a good thing,
with a future change to cleanup git-diff.

-- 
Shawn.

[PATCH] annotate/blame tests updates.

[Junio C Hamano]

Ryan Anderson <ryan@michonline.com> writes:

>>+test_expect_success \
>>+    'check all lines blamed on A' \
>>+    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
>>  
>>
> This should be $PROG as well, I suspect.

True.  Also I am suspecting this awk being a but unsafe.  The
default author is "A U Thor <author@example.com>", and without
FS specified awk would split at whitespaces.

I haven't pushed out the test-case sharing by Fredrik yet, but I
have already taken it.  This patch is on top of it.

-- >8 --
This rewrites the result check code a bit.  The earlier one
using awk was splitting columns at any whitespace, which
confused lines attributed incorrectly to the merge made by the
default author "A U Thor <author@example.com>" with lines
attributed to author "A".

The latest test by Ryan to add the "starting from older commit"
test is also included, with another older commit test.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 t/annotate-tests.sh |   81 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 44 insertions(+), 37 deletions(-)

92a903acfd0904e6dd6d18112428429938783d19
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 54a4dfb..d25a7a1 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -1,6 +1,37 @@
 # This file isn't used as a test script directly, instead it is
 # sourced from t8001-annotate.sh and t8001-blame.sh.
 
+check_count () {
+	head=
+	case "$1" in -h) head="$2"; shift; shift ;; esac
+	$PROG file $head | perl -e '
+		my %expect = (@ARGV);
+		my %count = ();
+		while (<STDIN>) {
+			if (/^[0-9a-f]+\t\(([^\t]+)\t/) {
+				my $author = $1;
+				for ($author) { s/^\s*//; s/\s*$//; }
+				if (exists $expect{$author}) {
+					$count{$author}++;
+				}
+			}
+		}
+		my $bad = 0;
+		while (my ($author, $count) = each %count) {
+			my $ok;
+			if ($expect{$author} != $count) {
+				$bad = 1;
+				$ok = "bad";
+			}
+			else {
+				$ok = "good";
+			}
+			print STDERR "Author $author (expected $expect{$author}, attributed $count) $ok\n";
+		}
+		exit($bad);
+	' "$@"
+}
+
 test_expect_success \
     'prepare reference tree' \
     'echo "1A quick brown fox jumps over the" >file &&
@@ -10,7 +41,7 @@ test_expect_success \
 
 test_expect_success \
     'check all lines blamed on A' \
-    '[ $(git annotate file | awk "{print \$3}" | grep -c "A") == 2 ]'
+    'check_count A 2'
 
 test_expect_success \
     'Setup new lines blamed on B' \
@@ -19,12 +50,8 @@ test_expect_success \
      GIT_AUTHOR_NAME="B" git commit -a -m "Second."'
 
 test_expect_success \
-    'Two lines blamed on A' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "A") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "B") == 2 ]'
+    'Two lines blamed on A, two on B' \
+    'check_count A 2 B 2'
 
 test_expect_success \
     'merge-setup part 1' \
@@ -34,16 +61,8 @@ test_expect_success \
      GIT_AUTHOR_NAME="B1" git commit -a -m "Branch1-1"'
 
 test_expect_success \
-    'Two lines blamed on A' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 2 ]'
-
-test_expect_success \
-    'Two lines blamed on B1' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
+    'Two lines blamed on A, two on B, two on B1' \
+    'check_count A 2 B 2 B1 2'
 
 test_expect_success \
     'merge-setup part 2' \
@@ -53,34 +72,22 @@ test_expect_success \
      GIT_AUTHOR_NAME="B2" git commit -a -m "Branch2-1"'
 
 test_expect_success \
-    'Two lines blamed on A' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
-
-test_expect_success \
-    'One line blamed on B' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
-
-test_expect_success \
-    'One line blamed on B2' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
-
+    'Two lines blamed on A, one on B, one on B2' \
+    'check_count A 2 B 1 B2 1'
 
 test_expect_success \
     'merge-setup part 3' \
     'git pull . branch1'
 
 test_expect_success \
-    'Two lines blamed on A' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^A$") == 2 ]'
+    'Two lines blamed on A, one on B, two on B1, one on B2' \
+    'check_count A 2 B 1 B1 2 B2 1'
 
 test_expect_success \
-    'One line blamed on B' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B$") == 1 ]'
+    'Annotating an old revision works' \
+    'check_count -h master A 2 B 2'
 
 test_expect_success \
-    'Two lines blamed on B1' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B1$") == 2 ]'
+    'Annotating an old revision works' \
+    'check_count -h master^ A 2'
 
-test_expect_success \
-    'One line blamed on B2' \
-    '[ $($PROG file | awk "{print \$3}" | grep -c "^B2$") == 1 ]'
-- 
1.2.4.g4668

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> 	git-diff Makefile
> 	git-diff HEAD Makefile
>
> ?  (Which is rather ugly as what if you have a tracked file actually
> called HEAD and you want the first form when the commit-ish is
> omitted.)  So accepting an optional commit-ish before the filename
> would be in line with what git-diff accepts today.

The use of "start of filelist" marker "--" is optional when
unambiguous, and that is why "git-diff Makefile" works.  To view
the change you still haven't update-index'ed to the path called
"HEAD", you would naturally say "git-diff -- HEAD"

Re: [PATCH] annotate: Support annotation of files on other revisions.

[A Large Angry SCM]

Shawn Pearce wrote:
> Ryan Anderson <ryan@michonline.com> wrote:
>>A Large Angry SCM wrote:
>>
>>>Ryan Anderson wrote:
>>>...
>>>
>>>>For annotate, the syntax I was using was:
>>>>    git annotate Makefile headname
>>>>
>>>>I'm not married to it, so please, send a patch to change it if you want
>>>>(Please fix up the test case I'm sending in this patch, as well.)
>>>>
>>>Wouldn't
>>>    git annotate <headname> <filename(s?)>
>>>be more consistent with other git commands?
>>>
>>Yes, but <headname> (really, a commitish) needs to be optional.
>>
>>I should probably switch to:
>>    git annotate [-c|--commit <committish>] <filename>
>>but that's partly why I'm asking for feedback.

That works.

Or maybe:
	git-annotate [<committish> --] <filename(s?)>
or:
	git-annotate [<committish>] -- <filename(s?)>

> 
> Yes but doesn't git-diff accept:
> 
> 	git-diff Makefile
> 	git-diff HEAD Makefile
> 
> ?  (Which is rather ugly as what if you have a tracked file actually
> called HEAD and you want the first form when the commit-ish is
> omitted.)  So accepting an optional commit-ish before the filename
> would be in line with what git-diff accepts today.
> 
> But maybe breaking convention from git-diff now is a good thing,
> with a future change to cleanup git-diff.
> 

Looking at the documentation, it looks like all of the commands that 
take paths do so as the last arguments. With any commit/tree arguments 
being, either, required or flagged.

Is there any reason that git-{annotate,blame} can't take more than one 
filename, ever?

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Junio C Hamano]

> Is there any reason that git-{annotate,blame} can't take more than one
> filename, ever?

I do not see it would be much useful -- the output does not
have a sign to show file boundary.

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > 	git-diff Makefile
> > 	git-diff HEAD Makefile
> >
> > ?  (Which is rather ugly as what if you have a tracked file actually
> > called HEAD and you want the first form when the commit-ish is
> > omitted.)  So accepting an optional commit-ish before the filename
> > would be in line with what git-diff accepts today.
> 
> The use of "start of filelist" marker "--" is optional when
> unambiguous, and that is why "git-diff Makefile" works.  To view
> the change you still haven't update-index'ed to the path called
> "HEAD", you would naturally say "git-diff -- HEAD"

Naturally.  :-)

After getting along fine with `git-diff foo` for months I would
natually not be surprised when `git-diff foo` didn't work because
I performed a `git-checkout -b foo pu` one day.

-- 
Shawn.

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Fredrik Kuivinen]

On Mon, Mar 06, 2006 at 05:18:35PM +1300, Martin Langhoff wrote:
> On 3/6/06, Ryan Anderson <ryan@michonline.com> wrote:
> > annotate was *trying* to handle that cleanly, but failed due to a silly
> 
> Great stuff, thanks! I'll let it hit master and then I'll drop the
> messy part of req_annotate() in cvsserver.
> 
> > For annotate, the syntax I was using was:
> >         git annotate Makefile headname
> >
> > I'm not married to it, so please, send a patch to change it if you want
> > (Please fix up the test case I'm sending in this patch, as well.)
> 
> That's _perfect_. I was just making the syntax up.
> 

That syntax should work with git-blame too. If it doesn't, it's a bug.

- Fredrik

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Johannes Schindelin]

Hi,

On Sun, 5 Mar 2006, Ryan Anderson wrote:

> +use Data::Dumper;

You really need this?

Ciao,
Dscho

P.S.: Not to be left behind, I also weigh in into the other discussion: 
I'd like "git-{annotate,blame} [HEAD [--]] [filenames...]".

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Johannes Schindelin]

Hi,

On Sun, 5 Mar 2006, Junio C Hamano wrote:

> > Is there any reason that git-{annotate,blame} can't take more than one
> > filename, ever?
> 
> I do not see it would be much useful -- the output does not
> have a sign to show file boundary.

CVS does it. Why shouldn't git, too?

Ciao,
Dscho

P.S.: The output for more than one file is separated by something like

	Annotations for <filename.txt>
	***************

Re: [PATCH] annotate: Support annotation of files on other revisions.

[Ryan Anderson]

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

Johannes Schindelin wrote:

>Hi,
>
>On Sun, 5 Mar 2006, Ryan Anderson wrote:
>
>  
>
>>+use Data::Dumper;
>>    
>>
>
>You really need this?
>  
>

I keep adding it back in when debugging things, and then promptly forget
to remove it when I cull the debugging back out.

It's a core module, so it shouldn't be a significant issue for any porting

-- 

Ryan Anderson
  sometimes Pug Majere


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]