* [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-04 12:30 ` Petr Baudis
@ 2008-09-04 13:03 ` Matthieu Moy
2008-09-04 19:12 ` Junio C Hamano
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2008-09-04 13:03 UTC (permalink / raw)
To: gitster, git; +Cc: 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-04 12:30 ` Petr Baudis
2008-09-04 13:03 ` Matthieu Moy
@ 2008-09-04 19:12 ` Junio C Hamano
2008-09-04 19:17 ` Junio C Hamano
2008-09-05 7:56 ` Junio C Hamano
3 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-09-04 19:12 UTC (permalink / raw)
To: Petr Baudis; +Cc: Matthieu Moy, git
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?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-04 12:30 ` Petr Baudis
2008-09-04 13:03 ` Matthieu Moy
2008-09-04 19:12 ` Junio C Hamano
@ 2008-09-04 19:17 ` Junio C Hamano
2008-09-05 6:31 ` Matthieu Moy
2008-09-05 7:56 ` Junio C Hamano
3 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2008-09-04 19:17 UTC (permalink / raw)
To: Petr Baudis; +Cc: Matthieu Moy, git
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-04 19:17 ` Junio C Hamano
@ 2008-09-05 6:31 ` Matthieu Moy
2008-09-05 7:29 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Matthieu Moy @ 2008-09-05 6:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, git
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-05 6:31 ` Matthieu Moy
@ 2008-09-05 7:29 ` Junio C Hamano
2008-09-05 8:07 ` Petr Baudis
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2008-09-05 7:29 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Petr Baudis, git
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[]
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-05 7:29 ` Junio C Hamano
@ 2008-09-05 8:07 ` Petr Baudis
2008-09-05 12:06 ` Matthieu Moy
0 siblings, 1 reply; 17+ messages in thread
From: Petr Baudis @ 2008-09-05 8:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matthieu Moy, git
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-05 8:07 ` Petr Baudis
@ 2008-09-05 12:06 ` Matthieu Moy
0 siblings, 0 replies; 17+ messages in thread
From: Matthieu Moy @ 2008-09-05 12:06 UTC (permalink / raw)
To: Petr Baudis; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Mention the fact that 'git annotate' is only for backward compatibility.
2008-09-04 12:30 ` Petr Baudis
` (2 preceding siblings ...)
2008-09-04 19:17 ` Junio C Hamano
@ 2008-09-05 7:56 ` Junio C Hamano
3 siblings, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2008-09-05 7:56 UTC (permalink / raw)
To: Petr Baudis; +Cc: Matthieu Moy, git
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
+'
+
^ permalink raw reply related [flat|nested] 17+ messages in thread