[PATCH 0/4] Make --decorate more useful

[PATCH 0/4] Make --decorate more useful

[Johannes Schindelin]

This series is meant to (partially) replace the patch I posted in

http://article.gmane.org/gmane.comp.version-control.git/24383

Often it is useful to find out what feature came in when, to quickly
find out if some version of the software is sufficient, or if the
other person has to upgrade.

With this series applied, you can say

	git log --decorate=tag -S<some keyword> <file>

to find out what that version was, indeed you even get a human readable
name for that commit, in addition to the commit name.

Johannes Schindelin (4):
      Move add_name_decoration() and add_ref_decoration() to commit.[ch]
      Move the --decorate option from builtin-log.c to revision.c.
      --decorate now decorates ancestors, too
      --decorate: prefer shorter names

 Documentation/git-log.txt        |    3 -
 Documentation/pretty-options.txt |   15 ++++++
 builtin-log.c                    |   39 +-------------
 commit.c                         |  104 ++++++++++++++++++++++++++++++++++++++
 commit.h                         |    5 ++
 log-tree.c                       |    4 ++
 revision.c                       |   47 +++++++++++++++++
 revision.h                       |    9 +++
 8 files changed, 186 insertions(+), 40 deletions(-)

[PATCH 1/4] Move add_name_decoration() and add_ref_decoration() to commit.[ch]

[Johannes Schindelin]

The functions add_{name,ref}_decoration() should really be in
commit.[ch], since the struct they are modifying is there, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-log.c |   25 -------------------------
 commit.c      |   25 +++++++++++++++++++++++++
 commit.h      |    3 +++
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 13bae31..c14eea5 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -21,31 +21,6 @@ static const char *fmt_patch_subject_prefix = "PATCH";
 /* this is in builtin-diff.c */
 void add_head(struct rev_info *revs);
 
-static void add_name_decoration(const char *prefix, const char *name, struct object *obj)
-{
-	int plen = strlen(prefix);
-	int nlen = strlen(name);
-	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
-	memcpy(res->name, prefix, plen);
-	memcpy(res->name + plen, name, nlen + 1);
-	res->next = add_decoration(&name_decoration, obj, res);
-}
-
-static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
-{
-	struct object *obj = parse_object(sha1);
-	if (!obj)
-		return 0;
-	add_name_decoration("", refname, obj);
-	while (obj->type == OBJ_TAG) {
-		obj = ((struct tag *)obj)->tagged;
-		if (!obj)
-			break;
-		add_name_decoration("tag: ", refname, obj);
-	}
-	return 0;
-}
-
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
diff --git a/commit.c b/commit.c
index 03436b1..c0748ed 100644
--- a/commit.c
+++ b/commit.c
@@ -1553,3 +1553,28 @@ int in_merge_bases(struct commit *commit, struct commit **reference, int num)
 	free_commit_list(bases);
 	return ret;
 }
+
+void add_name_decoration(const char *prefix, const char *name, struct object *obj)
+{
+	int plen = strlen(prefix);
+	int nlen = strlen(name);
+	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
+	memcpy(res->name, prefix, plen);
+	memcpy(res->name + plen, name, nlen + 1);
+	res->next = add_decoration(&name_decoration, obj, res);
+}
+
+int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
+{
+	struct object *obj = parse_object(sha1);
+	if (!obj)
+		return 0;
+	add_name_decoration("", refname, obj);
+	while (obj->type == OBJ_TAG) {
+		obj = ((struct tag *)obj)->tagged;
+		if (!obj)
+			break;
+		add_name_decoration("tag: ", refname, obj);
+	}
+	return 0;
+}
diff --git a/commit.h b/commit.h
index 467872e..787ae5e 100644
--- a/commit.h
+++ b/commit.h
@@ -29,6 +29,9 @@ struct name_decoration {
 	char name[1];
 };
 
+void add_name_decoration(const char *prefix, const char *name, struct object *obj);
+int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
+
 struct commit *lookup_commit(const unsigned char *sha1);
 struct commit *lookup_commit_reference(const unsigned char *sha1);
 struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
-- 
1.5.3.rc0.2783.gf3f7

[PATCH 2/4] Move the --decorate option from builtin-log.c to revision.c.

[Johannes Schindelin]

It is more logical to have the --decorate option in revision, like
many others affecting the way how commits are shown.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-log.txt        |    3 ---
 Documentation/pretty-options.txt |    4 ++++
 builtin-log.c                    |   14 ++------------
 revision.c                       |    4 ++++
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 7adcdef..b55a87e 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -51,9 +51,6 @@ include::pretty-options.txt[]
 	a record about how the tip of a reference was changed.
 	See also gitlink:git-reflog[1].
 
---decorate::
-	Print out the ref names of any commits that are shown.
-
 --full-diff::
 	Without this flag, "git log -p <paths>..." shows commits that
 	touch the specified paths, and diffs about the same specified
diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 746bc5b..3b21e0d 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -20,3 +20,7 @@ people using 80-column terminals.
 	command to re-code the commit log message in the encoding
 	preferred by the user.  For non plumbing commands this
 	defaults to UTF-8.
+
+--decorate::
+	When a commit is shown, and it matches a ref, print that ref name
+	in brackets after the commit name.
diff --git a/builtin-log.c b/builtin-log.c
index c14eea5..59372bb 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -24,9 +24,6 @@ void add_head(struct rev_info *revs);
 static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		      struct rev_info *rev)
 {
-	int i;
-	int decorate = 0;
-
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
@@ -40,15 +37,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		if (rev->diffopt.nr_paths != 1)
 			usage("git logs can only follow renames on one pathname at a time");
 	}
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--decorate")) {
-			if (!decorate)
-				for_each_ref(add_ref_decoration, NULL);
-			decorate = 1;
-		} else
-			die("unrecognized argument: %s", arg);
-	}
+	if (argc > 1)
+		die("unrecognized argument: %s", argv[1]);
 }
 
 static int cmd_log_walk(struct rev_info *rev)
diff --git a/revision.c b/revision.c
index 33ee9ee..7683d0a 100644
--- a/revision.c
+++ b/revision.c
@@ -1176,6 +1176,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->reverse ^= 1;
 				continue;
 			}
+			if (!strcmp(arg, "--decorate")) {
+				for_each_ref(add_ref_decoration, NULL);
+				continue;
+			}
 
 			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
 			if (opts > 0) {
-- 
1.5.3.rc0.2783.gf3f7

Re: [PATCH 2/4] Move the --decorate option from builtin-log.c to revision.c.

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> ---decorate::
> -	Print out the ref names of any commits that are shown.
> -
> ...
> +
> +--decorate::
> +	When a commit is shown, and it matches a ref, print that ref name
> +	in brackets after the commit name.

The character-pair ( and ) are usually called parentheses, not
brackets.

Other than that, these two code movements and refactorings feel
a sane thing to do.

Re: [PATCH 2/4] Move the --decorate option from builtin-log.c to revision.c.

[Johannes Schindelin]

Hi,

On Thu, 12 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > ---decorate::
> > -	Print out the ref names of any commits that are shown.
> > -
> > ...
> > +
> > +--decorate::
> > +	When a commit is shown, and it matches a ref, print that ref name
> > +	in brackets after the commit name.
> 
> The character-pair ( and ) are usually called parentheses, not
> brackets.

Okay.  You want me to resend, or will you fix it?

Ciao,
Dscho

[PATCH 3/4] --decorate now decorates ancestors, too

[Johannes Schindelin]

The option --decorate changed default behavior: Earlier, it decorated
commits pointed to by any ref.  The new behavior is this: decorate the
with the given refs and its ancestors, i.e.

	git log --decorate next master

will show "next", "next^", "next~2", ..., "master", "master^", ...
in parenthesis after the commit name.

However, you can still get the old behavior by giving --decorate=<mode>.
The available modes are:

- 'given'	the new default behavior,
- 'given-refs'	only decorate with the given refs, not their ancestors,
- 'tag'		decorate with all tag refs and their ancestors,
- 'tag-refs'	decorate with all tag refs, but not their ancestors,
- 'any'		decorate with all refs and their ancestors,
- 'any-ref'	decorate only with the refs, not their ancestors

The old behavior was 'any-ref'.

NOTE: Calling "git log --decorate=tag <commit>" is _not_ the same as
calling "git log <commit> | git -p name-rev --stdin":

For one, the latter will only try to name 40-character hex sequences
(which the former does not care about), but then it will also try
to name such sequences in the commit messages and the diffs.

More importantly, though, "--decorate=tag" will only build the names
while traversing commits, which makes it faster, but will fail to
correctly identify something like "git log --decorate=tag v0.99~2".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/pretty-options.txt |   17 ++++++++--
 commit.c                         |   68 +++++++++++++++++++++++++++++++++++--
 commit.h                         |    4 ++-
 log-tree.c                       |    4 ++
 revision.c                       |   43 +++++++++++++++++++++++-
 revision.h                       |    9 +++++
 6 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 3b21e0d..f82b1e9 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -21,6 +21,17 @@ people using 80-column terminals.
 	preferred by the user.  For non plumbing commands this
 	defaults to UTF-8.
 
---decorate::
-	When a commit is shown, and it matches a ref, print that ref name
-	in brackets after the commit name.
+--decorate[=<mode>]::
+	When a commit is shown, decorate its commit name with the refs
+	specified on the command line or their ancestors which point to
+	that commit.
++
+You can influence the behaviour by givin a mode:
+- 'given' is the default mode,
+- 'given-refs' skips the decoration for ancestors,
+- 'tag-refs' decorates only the tagged commits,
+- 'tag' decorates tagged commits and their ancestors,
+- 'any-ref' means that those commits are decorated that match any ref
+  exactly, but no ancestors, and
+- 'any' means that all commits that are pointed at by _any_ reachable
+  commit are decorated by the appropriate names.
diff --git a/commit.c b/commit.c
index c0748ed..3aa21cc 100644
--- a/commit.c
+++ b/commit.c
@@ -1554,13 +1554,24 @@ int in_merge_bases(struct commit *commit, struct commit **reference, int num)
 	return ret;
 }
 
-void add_name_decoration(const char *prefix, const char *name, struct object *obj)
+void add_name_decoration(const char *prefix, const char *name, int generation, struct object *obj)
 {
 	int plen = strlen(prefix);
 	int nlen = strlen(name);
-	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
+	struct name_decoration *res, *existing;
+
+	res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
 	memcpy(res->name, prefix, plen);
 	memcpy(res->name + plen, name, nlen + 1);
+
+	for (existing = lookup_decoration(&name_decoration, obj);
+			existing; existing = existing->next)
+		if (!strcmp(existing->name, res->name)) {
+			free(res);
+			return;
+		}
+
+	res->generation = generation;
 	res->next = add_decoration(&name_decoration, obj, res);
 }
 
@@ -1569,12 +1580,61 @@ int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags
 	struct object *obj = parse_object(sha1);
 	if (!obj)
 		return 0;
-	add_name_decoration("", refname, obj);
+	if (!prefixcmp(refname, "refs/heads/"))
+		refname += 11;
+	else if (!prefixcmp(refname, "refs/tags/"))
+		refname += 10;
+	else if (!prefixcmp(refname, "refs/remotes/"))
+		refname += 13;
+	else if (!prefixcmp(refname, "refs/"))
+		refname += 5;
+	add_name_decoration("", refname, 0, obj);
 	while (obj->type == OBJ_TAG) {
 		obj = ((struct tag *)obj)->tagged;
 		if (!obj)
 			break;
-		add_name_decoration("tag: ", refname, obj);
+		add_name_decoration("tag: ", refname, 0, obj);
 	}
 	return 0;
 }
+
+void add_name_decoration_to_parents(struct commit *commit)
+{
+	struct name_decoration *decoration, *decor;
+	struct commit_list *parents;
+	int parent_nr;
+
+	parents = commit->parents;
+	if (!parents)
+		return;
+
+	decoration = lookup_decoration(&name_decoration, &commit->object);
+	if (!decoration)
+		return;
+
+	for (decor = decoration; decor; decor = decor->next)
+		add_name_decoration("", decor->name, decor->generation + 1,
+				&parents->item->object);
+
+	parent_nr = 1;
+	while (parents->next) {
+		char buffer[PATH_MAX];
+		parents = parents->next;
+		parent_nr++;
+		for (decor = decoration; decor; decor = decor->next) {
+			if (decor->generation < 2)
+				snprintf(buffer, sizeof(buffer),
+						"%s%s^%d", decor->name,
+						decor->generation ? "^" : "",
+						parent_nr);
+			else
+				snprintf(buffer, sizeof(buffer),
+						"%s~%d^%d", decor->name,
+						decor->generation, parent_nr);
+			add_name_decoration("", buffer, 0,
+					&parents->item->object);
+		}
+	}
+}
+
+
diff --git a/commit.h b/commit.h
index 787ae5e..8dafc7b 100644
--- a/commit.h
+++ b/commit.h
@@ -26,11 +26,13 @@ extern const char *commit_type;
 extern struct decoration name_decoration;
 struct name_decoration {
 	struct name_decoration *next;
+	int generation;
 	char name[1];
 };
 
-void add_name_decoration(const char *prefix, const char *name, struct object *obj);
+void add_name_decoration(const char *prefix, const char *name, int generation, struct object *obj);
 int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
+void add_name_decoration_to_parents(struct commit *commit);
 
 struct commit *lookup_commit(const unsigned char *sha1);
 struct commit *lookup_commit_reference(const unsigned char *sha1);
diff --git a/log-tree.c b/log-tree.c
index 8624d5a..eb7641a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -26,6 +26,10 @@ static void show_decorations(struct commit *commit)
 	prefix = " (";
 	while (decoration) {
 		printf("%s%s", prefix, decoration->name);
+		if (decoration->generation == 1)
+			putchar('^');
+		else if (decoration->generation > 1)
+			printf("~%d", decoration->generation);
 		prefix = ", ";
 		decoration = decoration->next;
 	}
diff --git a/revision.c b/revision.c
index 7683d0a..79880b4 100644
--- a/revision.c
+++ b/revision.c
@@ -641,6 +641,10 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 	if (it->type != OBJ_COMMIT)
 		return 0;
 	commit = (struct commit *)it;
+	if (revs->decorate & DECORATE_GIVEN_REFS)
+		add_name_decoration("", arg, 0, it);
+	if (revs->decorate & DECORATE_RECURSIVE)
+		add_name_decoration_to_parents(commit);
 	for (parents = commit->parents; parents; parents = parents->next) {
 		it = &parents->item->object;
 		it->flags |= flags;
@@ -778,6 +782,11 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 				verify_non_filename(revs->prefix, arg);
 			}
 
+			if (revs->decorate & DECORATE_GIVEN_REFS) {
+				add_name_decoration("", this, 0, &a->object);
+				add_name_decoration("", next, 0, &b->object);
+			}
+
 			if (symmetric) {
 				exclude = get_merge_bases(a, b, 1);
 				add_pending_commit_list(revs, exclude,
@@ -817,6 +826,8 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	if (revs->decorate & DECORATE_GIVEN_REFS)
+		add_name_decoration("", arg, 0, object);
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
@@ -1177,7 +1188,29 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				continue;
 			}
 			if (!strcmp(arg, "--decorate")) {
-				for_each_ref(add_ref_decoration, NULL);
+				revs->decorate = DECORATE_GIVEN_REFS |
+					DECORATE_RECURSIVE;
+				continue;
+			}
+			if (!prefixcmp(arg, "--decorate=")) {
+				if (!strcmp(arg + 11, "any-ref"))
+					revs->decorate = DECORATE_ALL_REFS;
+				else if (!strcmp(arg + 11, "given-refs"))
+					revs->decorate = DECORATE_GIVEN_REFS;
+				else if (!strcmp(arg + 11, "given"))
+					revs->decorate = DECORATE_GIVEN_REFS |
+						DECORATE_RECURSIVE;
+				else if (!strcmp(arg + 11, "any"))
+					revs->decorate = DECORATE_GIVEN_REFS |
+						DECORATE_ALL_REFS |
+						DECORATE_RECURSIVE;
+				else if (!strcmp(arg + 11, "tag-refs"))
+					revs->decorate = DECORATE_TAG_REFS;
+				else if (!strcmp(arg + 11, "tag"))
+					revs->decorate = DECORATE_TAG_REFS |
+						DECORATE_RECURSIVE;
+				else
+					die("Unknown argument: %s", arg);
 				continue;
 			}
 
@@ -1213,6 +1246,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		}
 	}
 
+	if (revs->decorate & DECORATE_ALL_REFS)
+		for_each_ref(add_ref_decoration, NULL);
+	if (revs->decorate & DECORATE_TAG_REFS)
+		for_each_tag_ref(add_ref_decoration, NULL);
+
 	if (revs->grep_filter)
 		revs->grep_filter->regflags |= regflags;
 
@@ -1372,6 +1410,9 @@ static struct commit *get_revision_1(struct rev_info *revs)
 		revs->commits = entry->next;
 		free(entry);
 
+		if (revs->decorate & DECORATE_RECURSIVE)
+			add_name_decoration_to_parents(commit);
+
 		if (revs->reflog_info)
 			fake_reflog_parent(revs->reflog_info, commit);
 
diff --git a/revision.h b/revision.h
index f46b4d5..0c3dc67 100644
--- a/revision.h
+++ b/revision.h
@@ -16,6 +16,14 @@ struct log_info;
 
 typedef void (prune_fn_t)(struct rev_info *revs, struct commit *commit);
 
+enum decorate {
+	DECORATE_NONE = 0,
+	DECORATE_RECURSIVE = 1,
+	DECORATE_ALL_REFS = 2,
+	DECORATE_TAG_REFS = 4,
+	DECORATE_GIVEN_REFS = 8,
+};
+
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
@@ -65,6 +73,7 @@ struct rev_info {
 	unsigned int	shown_one:1,
 			abbrev_commit:1;
 	enum date_mode date_mode;
+	enum decorate	decorate;
 
 	const char **ignore_packed; /* pretend objects in these are unpacked */
 	int num_ignore_packed;
-- 
1.5.3.rc0.2783.gf3f7

Re: [PATCH 3/4] --decorate now decorates ancestors, too

[Theodore Tso]

On Wed, Jul 11, 2007 at 02:29:49AM +0100, Johannes Schindelin wrote:
> 
> The option --decorate changed default behavior: Earlier, it decorated
> commits pointed to by any ref.  The new behavior is this: decorate the
> with the given refs and its ancestors, i.e.
> 
> 	git log --decorate next master
> 
> will show "next", "next^", "next~2", ..., "master", "master^", ...
> in parenthesis after the commit name.

I'm wondering how useful the default is.  The arguments get used for
two things; both for git-log to decide what revisions to display, and
which refs to decorate, right?  I'm not sure that overloading is such
a great idea.

Also, I note that "git log --decorate" does nothing at all.  Maybe it
would be better to keep the default to be "any-ref" instead of "given"?

		      	 	   	    - Ted

Re: [PATCH 3/4] --decorate now decorates ancestors, too

[Junio C Hamano]

Theodore Tso <tytso@mit.edu> writes:

> On Wed, Jul 11, 2007 at 02:29:49AM +0100, Johannes Schindelin wrote:
>> 
>> The option --decorate changed default behavior: Earlier, it decorated
>> commits pointed to by any ref.  The new behavior is this: decorate the
>> with the given refs and its ancestors, i.e.
>> 
>> 	git log --decorate next master
>> 
>> will show "next", "next^", "next~2", ..., "master", "master^", ...
>> in parenthesis after the commit name.
>
> I'm wondering how useful the default is.  The arguments get used for
> two things; both for git-log to decide what revisions to display, and
> which refs to decorate, right?  I'm not sure that overloading is such
> a great idea.
>
> Also, I note that "git log --decorate" does nothing at all.  Maybe it
> would be better to keep the default to be "any-ref" instead of "given"?

I think defaulting to "given" is a regression.  It could be
argued that "tag-ref" or "tag" might be a better default
(judging from my experience with "name-rev"), but keeping
"any-ref" would probably be the safest.

But in general I do not see ("I haven't realized" might turn out
to be a better expression) much value in this series yet except
for the initial clean-up patches, while I think this option
would be quite expensive in terms of memory footprints on
projects with nontrivial size of history.  I dunno.

Re: [PATCH 3/4] --decorate now decorates ancestors, too

[Johannes Schindelin]

Hi,

On Thu, 12 Jul 2007, Junio C Hamano wrote:

> Theodore Tso <tytso@mit.edu> writes:
> 
> > On Wed, Jul 11, 2007 at 02:29:49AM +0100, Johannes Schindelin wrote:
> >> 
> >> The option --decorate changed default behavior: Earlier, it decorated
> >> commits pointed to by any ref.  The new behavior is this: decorate the
> >> with the given refs and its ancestors, i.e.
> >> 
> >> 	git log --decorate next master
> >> 
> >> will show "next", "next^", "next~2", ..., "master", "master^", ...
> >> in parenthesis after the commit name.
> >
> > I'm wondering how useful the default is.  The arguments get used for
> > two things; both for git-log to decide what revisions to display, and
> > which refs to decorate, right?  I'm not sure that overloading is such
> > a great idea.
> >
> > Also, I note that "git log --decorate" does nothing at all.  Maybe it
> > would be better to keep the default to be "any-ref" instead of "given"?
> 
> I think defaulting to "given" is a regression.  It could be
> argued that "tag-ref" or "tag" might be a better default
> (judging from my experience with "name-rev"), but keeping
> "any-ref" would probably be the safest.

Okay, fair enough.  I kind of expected people to disagree as of the 
default mode.  My preference would have been "tag", since I need that 
quite often, but I am willing to put my wishes aside there.

Probably I should follow up with a replacement patch, and a config 
variable "commit.decorateMode" patch, leaving the default at "any-ref"?

> But in general I do not see ("I haven't realized" might turn out to be a 
> better expression) much value in this series yet except for the initial 
> clean-up patches, while I think this option would be quite expensive in 
> terms of memory footprints on projects with nontrivial size of history.  
> I dunno.

There are a few points I want to make to convince you:

- I need this quite often to see which version introduced a certain 
  feature.  This is most visible on IRC, where people ask "how can I do X, 
  I have version Y", and me responding "There is a feature P, but 
  unfortunately, it is only available from revision Q~N" where Q > Y.

  I really want other people to do this easily, without having to know how 
  name-rev (which has a dash in its name, and thus is kind of 
  plumbing-ish) works.


- It is _not_ expensive.  It only ever allocates something in the case of 
  merges (at least that's how I designed it), and should free the used 
  memory when the commit name is not used at all (but maybe I forgot that 
  part...).  So you will have at most one allocation of a few bytes per 
  merge, and I really do not see how this could break down for 
  pathological, but real-world, cases.

- 40-character commit names seem to be really hard on people.

To drive the 3rd point home: I often see people confused as to what those 
long commit names are.  They do not even realise that they are _object_ 
names, actually, not only applicable to commit objects.

IMHO a great way to teach users that commit names are equivalent to 
shortcuts like "v1.5.0-rc1~20" would be to even enable decoration by tags 
by default.

Ciao,
Dscho

Re: [PATCH 3/4] --decorate now decorates ancestors, too

[Johannes Schindelin]

Hi,

On Thu, 12 Jul 2007, Junio C Hamano wrote:

> But in general I do not see ("I haven't realized" might turn out to be a 
> better expression) much value in this series yet except for the initial 
> clean-up patches, while I think this option would be quite expensive in 
> terms of memory footprints on projects with nontrivial size of history.  
> I dunno.

It would not, since you do not have to say "--decorate" at all.  However, 
if you do want the functionality this patch provides, you have to jump 
through hoops right now.  ATM an alias is my work around, since I run 
'master' as you requested:

git config --global alias.decoratelog '!sh -c
	"(case \"\$0\" in
		sh) git log --color;;
		*) git log --color \"\$0\" \"\$@\";;
	 esac) |
	 git -p name-rev --tags --stdin"'

Ugly, ain't it?

And I still have to look up at which release certain features were 
introduced every day.

Ciao,
Dscho

[PATCH 4/4] --decorate: prefer shorter names

[Johannes Schindelin]

When saying "--decorate=any", the decoration could get huge pretty
quickly.  Especially in a project like git.git, where there is a
lot of back and forth merging, older commits can be named in a
thousand ways.

However, usually you are only interested in short names.  For example,
most people are not interested in the fact that the initial commit
can be named by ways like v0.99~31^2~37^2^2~508^2^2~195^2~114, if you
can reference it by v0.99~954 instead.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 commit.c   |   39 +++++++++++++++++++++++++++++----------
 commit.h   |    4 ++--
 revision.c |   10 ++++++----
 3 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/commit.c b/commit.c
index 3aa21cc..4128ef4 100644
--- a/commit.c
+++ b/commit.c
@@ -1554,25 +1554,43 @@ int in_merge_bases(struct commit *commit, struct commit **reference, int num)
 	return ret;
 }
 
-void add_name_decoration(const char *prefix, const char *name, int generation, struct object *obj)
+void add_name_decoration(const char *prefix, const char *name, int generation, int distance, struct object *obj)
 {
 	int plen = strlen(prefix);
 	int nlen = strlen(name);
 	struct name_decoration *res, *existing;
+	int free_existing = 0;
+
+	existing = lookup_decoration(&name_decoration, obj);
+	if (existing && existing->distance < distance)
+		return;
+
+	if (existing && existing->distance > distance)
+		free_existing = 1;
 
 	res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
 	memcpy(res->name, prefix, plen);
 	memcpy(res->name + plen, name, nlen + 1);
 
-	for (existing = lookup_decoration(&name_decoration, obj);
-			existing; existing = existing->next)
-		if (!strcmp(existing->name, res->name)) {
-			free(res);
-			return;
-		}
+	if (!free_existing)
+		for (; existing; existing = existing->next)
+			if (!strcmp(existing->name, res->name)) {
+				free(res);
+				return;
+			}
 
 	res->generation = generation;
+	res->distance = distance;
 	res->next = add_decoration(&name_decoration, obj, res);
+
+	if (free_existing) {
+		for (existing = res->next; existing; ) {
+			struct name_decoration *next = existing->next;
+			free(existing);
+			existing = next;
+		}
+		res->next = NULL;
+	}
 }
 
 int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
@@ -1588,12 +1606,12 @@ int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags
 		refname += 13;
 	else if (!prefixcmp(refname, "refs/"))
 		refname += 5;
-	add_name_decoration("", refname, 0, obj);
+	add_name_decoration("", refname, 0, 0, obj);
 	while (obj->type == OBJ_TAG) {
 		obj = ((struct tag *)obj)->tagged;
 		if (!obj)
 			break;
-		add_name_decoration("tag: ", refname, 0, obj);
+		add_name_decoration("tag: ", refname, 0, 0, obj);
 	}
 	return 0;
 }
@@ -1614,7 +1632,7 @@ void add_name_decoration_to_parents(struct commit *commit)
 
 	for (decor = decoration; decor; decor = decor->next)
 		add_name_decoration("", decor->name, decor->generation + 1,
-				&parents->item->object);
+				decor->distance + 1, &parents->item->object);
 
 	parent_nr = 1;
 	while (parents->next) {
@@ -1632,6 +1650,7 @@ void add_name_decoration_to_parents(struct commit *commit)
 						"%s~%d^%d", decor->name,
 						decor->generation, parent_nr);
 			add_name_decoration("", buffer, 0,
+					decor->distance + 256,
 					&parents->item->object);
 		}
 	}
diff --git a/commit.h b/commit.h
index 8dafc7b..9b2bd55 100644
--- a/commit.h
+++ b/commit.h
@@ -26,11 +26,11 @@ extern const char *commit_type;
 extern struct decoration name_decoration;
 struct name_decoration {
 	struct name_decoration *next;
-	int generation;
+	int generation, distance;
 	char name[1];
 };
 
-void add_name_decoration(const char *prefix, const char *name, int generation, struct object *obj);
+void add_name_decoration(const char *prefix, const char *name, int generation, int distance, struct object *obj);
 int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
 void add_name_decoration_to_parents(struct commit *commit);
 
diff --git a/revision.c b/revision.c
index 79880b4..796275d 100644
--- a/revision.c
+++ b/revision.c
@@ -642,7 +642,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 		return 0;
 	commit = (struct commit *)it;
 	if (revs->decorate & DECORATE_GIVEN_REFS)
-		add_name_decoration("", arg, 0, it);
+		add_name_decoration("", arg, 0, 0, it);
 	if (revs->decorate & DECORATE_RECURSIVE)
 		add_name_decoration_to_parents(commit);
 	for (parents = commit->parents; parents; parents = parents->next) {
@@ -783,8 +783,10 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 			}
 
 			if (revs->decorate & DECORATE_GIVEN_REFS) {
-				add_name_decoration("", this, 0, &a->object);
-				add_name_decoration("", next, 0, &b->object);
+				add_name_decoration("", this, 0, 0,
+						&a->object);
+				add_name_decoration("", next, 0, 0,
+						&b->object);
 			}
 
 			if (symmetric) {
@@ -827,7 +829,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
 	if (revs->decorate & DECORATE_GIVEN_REFS)
-		add_name_decoration("", arg, 0, object);
+		add_name_decoration("", arg, 0, 0, object);
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
-- 
1.5.3.rc0.2783.gf3f7