blame vs annotate?

blame vs annotate?

[Jonathan del Strother]

What's the difference between 'git blame' & 'git annotate'?  The
output is practically identical - it's not obvious when you would use
one over the other

Re: blame vs annotate?

[Miklos Vajna]

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

On Wed, Sep 03, 2008 at 05:43:03PM +0100, Jonathan del Strother <maillist@steelskies.com> wrote:
> What's the difference between 'git blame' & 'git annotate'?  The
> output is practically identical - it's not obvious when you would use
> one over the other

'git annoate' is the same as 'git blame -c'.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: blame vs annotate?

[Petr Baudis]

On Wed, Sep 03, 2008 at 07:49:39PM +0200, Miklos Vajna wrote:
> On Wed, Sep 03, 2008 at 05:43:03PM +0100, Jonathan del Strother <maillist@steelskies.com> wrote:
> > What's the difference between 'git blame' & 'git annotate'?  The
> > output is practically identical - it's not obvious when you would use
> > one over the other
> 
> 'git annoate' is the same as 'git blame -c'.

But 'git blame -c' is defined as "the same as 'git annotate'". So this
answer does not really help. :-)

(And yes, I'd love to hear some as well. When pruning the completion
list, I've been on the verge of setting 'git annotate' as deprecated,
but decided to do that in a separate patch later and properly. Please
someone beat me to it. ;-)

				Petr "Pasky" Baudis

Re: blame vs annotate?

[Junio C Hamano]

"Jonathan del Strother" <maillist@steelskies.com> writes:

> What's the difference between 'git blame' & 'git annotate'?  The
> output is practically identical - it's not obvious when you would use
> one over the other

In short, don't use git-annotate unless you are an ancient script that was
written before git-blame was written.  They are functionally equivalent,
uses the same blame assigning engine, and the only difference is the
default output format, and blame knows how to mimick annotate output.

History.

The first implementation was called git-annotate that was implemented in
Perl.

    c65e898 (Add git-annotate, a tool for assigning blame., 2006-02-20)

A competing implementation in C was done and was called git-blame:

    cbfb73d (Add git-blame, a tool for assigning blame., 2006-02-21)

They competed for a while but it became more and more apparent that blame
was a better implementation.  After blame learned to mimick annotate
output, which some scripts depended on, Perl implementation was retired,
but the name git-annotate was left as a backward compatibility alias:

    f789e34 (Remove git-annotate.perl and create a builtin-alias for git-blame, 2006-10-09)

Then as an experiment, a reimplementation of blame in C, confusingly
codenamed git-pickaxe, was started, to detect code movements across and
within files:

    cee7f24 (git-pickaxe: blame rewritten., 2006-10-19)

This had the same output format as git-blame, also with compatibility
output format implemented for the older git-annotate.  After it matured,
it took over the original git-blame name.

    acca687 (git-pickaxe: retire pickaxe, 2006-11-08)

Re: blame vs annotate?

[Jakub Narebski]

"Jonathan del Strother" <maillist@steelskies.com> writes:

> What's the difference between 'git blame' & 'git annotate'?  The
> output is practically identical - it's not obvious when you would use
> one over the other

Long, long time ago (which in git terms mean almost 2 years ago:
git-annotate.perl was removed on Oct 9, 2006) there were two competing
implementations: git-annotate, which IIRC was first (starting at Feb
20, 2006) and was written in Perl, and git-blame (stated at Feb 21,
2006) which was written in C.  In long term git-blame won, if I
remember correctly because it avoided some strange corner case, and
assigned blame better.  Then git-annotate was made alias to "git-blame
-c", perhaps in additon to different synopsis (order of arguments and
possible options), for backward compatibility.

HTH.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: blame vs annotate?

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> "Jonathan del Strother" <maillist@steelskies.com> writes:
>
>> What's the difference between 'git blame' & 'git annotate'?  The
>> output is practically identical - it's not obvious when you would use
>> one over the other
>
> In short, don't use git-annotate unless you are an ancient script that was
> written before git-blame was written.  They are functionally equivalent,
> uses the same blame assigning engine, and the only difference is the
> default output format, and blame knows how to mimick annotate output.

Then, does it make sense to apply this?

--- a/Documentation/git-annotate.txt
+++ b/Documentation/git-annotate.txt
@@ -14,6 +14,10 @@ DESCRIPTION
 Annotates each line in the given file with information from the commit
 which introduced the line. Optionally annotate from a given revision.
 
+This command exists for backward compatibility. If you don't have
+existing scripts using 'git annotate', you should use
+linkgit:git-blame[1] instead.
+
 OPTIONS
 -------
 include::blame-options.txt[]

-- 
Matthieu

Re: blame vs annotate?

[Sverre Rabbelier]

On Thu, Sep 4, 2008 at 11:09, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> +This command exists for backward compatibility. If you don't have
> +existing scripts using 'git annotate', you should use
> +linkgit:git-blame[1] instead.

This implies that if you do have a script that uses git-annotate, you
should use it in your daily use too. Maybe something like "For regular
use you should use ...."?

-- 
Cheers,

Sverre Rabbelier

[PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-annotate.txt |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-annotate.txt b/Documentation/git-annotate.txt
index 8b6b56a..6e05825 100644
--- a/Documentation/git-annotate.txt
+++ b/Documentation/git-annotate.txt
@@ -14,6 +14,9 @@ DESCRIPTION
 Annotates each line in the given file with information from the commit
 which introduced the line. Optionally annotate from a given revision.
 
+This command exists for backward compatibility. For regular use, you
+should use linkgit:git-blame[1] instead.
+
 OPTIONS
 -------
 include::blame-options.txt[]
-- 
1.6.0.7.g161c

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Petr Baudis]

On Thu, Sep 04, 2008 at 02:00:52PM +0200, Matthieu Moy wrote:
> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  Documentation/git-annotate.txt |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/git-annotate.txt b/Documentation/git-annotate.txt
> index 8b6b56a..6e05825 100644
> --- a/Documentation/git-annotate.txt
> +++ b/Documentation/git-annotate.txt
> @@ -14,6 +14,9 @@ DESCRIPTION
>  Annotates each line in the given file with information from the commit
>  which introduced the line. Optionally annotate from a given revision.
>  
> +This command exists for backward compatibility. For regular use, you
> +should use linkgit:git-blame[1] instead.
> +
>  OPTIONS
>  -------
>  include::blame-options.txt[]

Can you please also mark it deprecated in the bash completion and
command-list.txt?

Will we still keep it around indefinitely or do we have any phase-out
plan? If the latter, we should also start printing warning when
executing it.

I'm also curious about

                if (suspect->commit->object.flags & UNINTERESTING) {
                        if (blank_boundary)
                                memset(hex, ' ', length);
                        else if (!cmd_is_annotate) {
                                length--;
                                putchar('^');
                        }
                }

in builtin-blame.c. Junio, you introduced this in e68989a739d - why
do you use a separate flag instead of OUTPUT_ANNOTATE_COMPAT? The fact
that git annotate == git blame -c does not hold true because of this
(admittedly obscure case).

-- 
				Petr "Pasky" Baudis
The next generation of interesting software will be done
on the Macintosh, not the IBM PC.  -- Bill Gates

[PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Matthieu Moy]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-annotate.txt         |    4 ++++
 command-list.txt                       |    2 +-
 contrib/completion/git-completion.bash |    1 +
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-annotate.txt b/Documentation/git-annotate.txt
index 8b6b56a..76f2b8c 100644
--- a/Documentation/git-annotate.txt
+++ b/Documentation/git-annotate.txt
@@ -14,6 +14,10 @@ DESCRIPTION
 Annotates each line in the given file with information from the commit
 which introduced the line. Optionally annotate from a given revision.
 
+This command exists for backward compatibility, as an alias for 'git
+blame -c'. For regular use, you should now use linkgit:git-blame[1]
+instead.
+
 OPTIONS
 -------
 include::blame-options.txt[]
diff --git a/command-list.txt b/command-list.txt
index 3583a33..03d1023 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -2,7 +2,7 @@
 # command name				category [deprecated] [common]
 git-add                                 mainporcelain common
 git-am                                  mainporcelain
-git-annotate                            ancillaryinterrogators
+git-annotate                            ancillaryinterrogators deprecated
 git-apply                               plumbingmanipulators
 git-archimport                          foreignscminterface
 git-archive                             mainporcelain
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4f64f8a..6630033 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -380,6 +380,7 @@ __git_porcelain_commands ()
 	do
 		case $i in
 		*--*)             : helper pattern;;
+		annotate)         : deprecated;;
 		applymbox)        : ask gittus;;
 		applypatch)       : ask gittus;;
 		archimport)       : import;;
-- 
1.6.0.7.g161c

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> I'm also curious about
>
>                 if (suspect->commit->object.flags & UNINTERESTING) {
>                         if (blank_boundary)
>                                 memset(hex, ' ', length);
>                         else if (!cmd_is_annotate) {
>                                 length--;
>                                 putchar('^');
>                         }
>                 }
>
> in builtin-blame.c. Junio, you introduced this in e68989a739d - why
> do you use a separate flag instead of OUTPUT_ANNOTATE_COMPAT? The fact
> that git annotate == git blame -c does not hold true because of this
> (admittedly obscure case).

I do not recall the context of this change, but I do not think this is a
deliberate omission.  The breakage the quoted commit fixed was about
cvsserver that does run "git-annotate" not "git-blame -c" and that must be
the reason this has been unnoticed for a long time.

Oh, by the way, somebody should update cvsserver to run "git annotate" and
send in a tested patch, please?

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Can you please also mark it deprecated in the bash completion and
> command-list.txt?

I am hesitant to use the word "deprecated" in git(7) manual page, as the
word has much stronger connotation than merely "because a more often used
equivalent exists, there is no reason to encourage this for new people,
but on the other hand there is no reason to remove or phase it out to hurt
existing users", which is what "git annotate" is.

"Deprecated" is more like "it is still supported but its use is actively
discouraged".  I do not think we actively discourage it, nor need to phase
it out.

People coming from different background may look for annotate and get
frustrated until they find out blame is the equivalent (or vice versa).

While I do not mind marking annotate as ": infrequently used ;;" in the
completion script, I am skeptical about the value of such a change.
Dropping annotate from the completion is a _slight_ improvement in that
when people type "git a<TAB>" they will see one less candidates, but it is
not a great deal of improvement: four down from five.

Mentioning that we support both names for findability in both annotate and
blame manual pages (not just annotate page) might be a better thing to do.

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Petr Baudis <pasky@suse.cz> writes:
>
>> Can you please also mark it deprecated in the bash completion and
>> command-list.txt?

[...]

> While I do not mind marking annotate as ": infrequently used ;;" in the
> completion script, I am skeptical about the value of such a change.
> Dropping annotate from the completion is a _slight_ improvement in that
> when people type "git a<TAB>" they will see one less candidates, but it is
> not a great deal of improvement: four down from five.

Hm, right. Someone used to "$scm annotate" will probably try
"git an<TAB>" and not finding anything there would be misleading
(probably why svn has "annotate" as an alias for "blame").

> Mentioning that we support both names for findability in both annotate and
> blame manual pages (not just annotate page) might be a better thing to do.

git-blame.txt already somehow has it in the documentation for "-c". I
can't think of a good wording to improve it.

-- 
Matthieu

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
> ...
> Hm, right. Someone used to "$scm annotate" will probably try
> "git an<TAB>" and not finding anything there would be misleading
> (probably why svn has "annotate" as an alias for "blame").
>
>> Mentioning that we support both names for findability in both annotate and
>> blame manual pages (not just annotate page) might be a better thing to do.
>
> git-blame.txt already somehow has it in the documentation for "-c". I
> can't think of a good wording to improve it.

When somebody is reading git-blame.txt (or git-annotate.txt) for the first
time, I think the message we would like to send is:

 (1) Here is why you would want to use this command, what it can do
     (perhaps more than what you would have expected from "$scm blame"),
     and how you tell it to do what it does.

     This is obvious.

 (2) You might have heard of the command with the other name.  There is no
     difference between the two, except they differ in their default
     output formats.  For historical reasons, we will continue supporting
     both.

     This is essential to squelch noises about "git has both?  how are
     they different?"

 (3) We tend to encourage blame over annotate for new scripts and new
     people, but there is no reason to choose one over the other.

     This is not as important as (2), but would be useful to squelch
     noises about "when will we start deprecating this?"

The mention of "annotate" in the description of "-c" hints almost nothing
in (2) except that the output format might be different (otherwise we
would not have "-c" command to begin with), but I guess that's Ok.  As
long as we describe (2) on git-annotate page clearly enough, people who
are really curious could refer to git-annotate page.

So based on your wording, how about doing it like this?  I chose to only
hint (3) subtly in this version instead of being overly explicit.

diff --git c/Documentation/git-annotate.txt w/Documentation/git-annotate.txt
index 8b6b56a..78dc5e2 100644
--- c/Documentation/git-annotate.txt
+++ w/Documentation/git-annotate.txt
@@ -14,6 +14,11 @@ DESCRIPTION
 Annotates each line in the given file with information from the commit
 which introduced the line. Optionally annotate from a given revision.
 
+The only difference from this command and linkgit:git-blame[1] is that
+they use slightly different output formats, and this command exists only
+for backward compatibility to support existing scripts, and provide more
+familiar command name for people coming from other SCM systems.
+
 OPTIONS
 -------
 include::blame-options.txt[]

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> I'm also curious about
> ...
> in builtin-blame.c. Junio, you introduced this in e68989a739d - why
> do you use a separate flag instead of OUTPUT_ANNOTATE_COMPAT?

Let's do this.  Still passes t8001 and t9400.

-- >8 --
"blame -c" should be compatible with "annotate"

There is no reason to have a separate variable cmd_is_annotate;
OUTPUT_ANNOTATE_COMPAT option is supposed to produce the compatibility
output, and we should produce the same output even when the command was
not invoked as "annotate" but as "blame -c".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-blame.c                 |    9 +++++----
 t/t9400-git-cvsserver-server.sh |   13 +++++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git i/builtin-blame.c w/builtin-blame.c
index d2fc68c..9bc901c 100644
--- i/builtin-blame.c
+++ w/builtin-blame.c
@@ -38,7 +38,6 @@ static int show_root;
 static int reverse;
 static int blank_boundary;
 static int incremental;
-static int cmd_is_annotate;
 static int xdl_opts = XDF_NEED_MINIMAL;
 static struct string_list mailmap;
 
@@ -1686,7 +1685,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 		if (suspect->commit->object.flags & UNINTERESTING) {
 			if (blank_boundary)
 				memset(hex, ' ', length);
-			else if (!cmd_is_annotate) {
+			else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
 				length--;
 				putchar('^');
 			}
@@ -2317,8 +2316,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	};
 
 	struct parse_opt_ctx_t ctx;
-
-	cmd_is_annotate = !strcmp(argv[0], "annotate");
+	int cmd_is_annotate = !strcmp(argv[0], "annotate");
 
 	git_config(git_blame_config, NULL);
 	init_revisions(&revs, NULL);
@@ -2346,6 +2344,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	argc = parse_options_end(&ctx);
 
+	if (cmd_is_annotate)
+		output_option |= OUTPUT_ANNOTATE_COMPAT;
+
 	if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);
diff --git i/t/t9400-git-cvsserver-server.sh w/t/t9400-git-cvsserver-server.sh
index 4b91f8d..c1850d2 100755
--- i/t/t9400-git-cvsserver-server.sh
+++ w/t/t9400-git-cvsserver-server.sh
@@ -488,4 +488,17 @@ test_expect_success 'cvs co -c (shows module database)' '
     ! grep -v "^master[	 ]\+master$" < out
 '
 
+#------------
+# CVS ANNOTATE
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs annotate' '
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs annotate merge >../out &&
+    sed -e "s/ .*//" ../out >../actual &&
+    for i in 3 1 1 1 1 1 1 1 2 4; do echo 1.$i; done >../expect &&
+    test_cmp ../expect ../actual
+'
+

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Petr Baudis]

On Fri, Sep 05, 2008 at 12:29:51AM -0700, Junio C Hamano wrote:
> diff --git c/Documentation/git-annotate.txt w/Documentation/git-annotate.txt
> index 8b6b56a..78dc5e2 100644
> --- c/Documentation/git-annotate.txt
> +++ w/Documentation/git-annotate.txt
> @@ -14,6 +14,11 @@ DESCRIPTION
>  Annotates each line in the given file with information from the commit
>  which introduced the line. Optionally annotate from a given revision.
>  
> +The only difference from this command and linkgit:git-blame[1] is that
                       ^^^^ between?
> +they use slightly different output formats, and this command exists only
> +for backward compatibility to support existing scripts, and provide more
> +familiar command name for people coming from other SCM systems.
> +
>  OPTIONS
>  -------
>  include::blame-options.txt[]

I like this one. I'm still not too happy about leaving the command in
the command list and tab completion since I believe reducing the number
of commands (which is still quite intimidating for the user), even just
by a little, is more important than encouraging people to use
compatibility aliases. I don't feel very strongly about it, though.

Acked-by: Petr Baudis <pasky@suse.cz>

Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.

[Matthieu Moy]

Petr Baudis <pasky@suse.cz> writes:

> On Fri, Sep 05, 2008 at 12:29:51AM -0700, Junio C Hamano wrote:
>> diff --git c/Documentation/git-annotate.txt w/Documentation/git-annotate.txt
>> index 8b6b56a..78dc5e2 100644
>> --- c/Documentation/git-annotate.txt
>> +++ w/Documentation/git-annotate.txt
>> @@ -14,6 +14,11 @@ DESCRIPTION
>>  Annotates each line in the given file with information from the commit
>>  which introduced the line. Optionally annotate from a given revision.
>>  
>> +The only difference from this command and linkgit:git-blame[1] is that
>                        ^^^^ between?
>> +they use slightly different output formats, and this command exists only

I'm not sure it's clear enough that "this command" is "annotate" since
you talked about git blame right before, but I'm being picky.

>> +for backward compatibility to support existing scripts, and provide more
>> +familiar command name for people coming from other SCM systems.
>> +
>>  OPTIONS
>>  -------
>>  include::blame-options.txt[]
>
> I like this one.

Meetoo.

-- 
Matthieu