git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC 5/6] builtin-tag: add sort by date -D
@ 2009-02-22 18:06 Marc-André Lureau
  2009-02-22 18:33 ` Junio C Hamano
  2009-02-22 18:38 ` Felipe Contreras
  0 siblings, 2 replies; 3+ messages in thread
From: Marc-André Lureau @ 2009-02-22 18:06 UTC (permalink / raw)
  To: git

Signed-off-by: Marc-Andre Lureau <marcandre.lureau@gmail.com>
---
 builtin-tag.c |  162 +++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 129 insertions(+), 33 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 01e7374..8ff9d03 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git tag -d <tagname>...",
-	"git tag -l [-n[<num>]] [<pattern>]",
+	"git tag -l [-n[<num>] -D] [<pattern>]",
 	"git tag -v <tagname>...",
 	NULL
 };
@@ -27,21 +27,108 @@ struct tag_filter {
 	const char *pattern;
 	int lines;
 	struct commit_list *with_commit;
+	int sort;
+	struct object_list *sorted_tags;
 };

+struct light_tag {
+	struct object object;
+	struct object *tagged;
+	char *refname;
+};
+
+#define OBJ_LIGHT_TAG (OBJ_MAX + 1)
 #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"

+static unsigned long object_date(struct object *object)
+{
+	if (object->type == OBJ_TAG)
+		return ((struct tag*)object)->date;
+	else if (object->type == OBJ_COMMIT)
+		return ((struct commit*)object)->date;
+	else if (object->type == OBJ_TREE)
+		return 0;
+	else if (object->type == OBJ_BLOB)
+		return 0;
+	else if (object->type == OBJ_LIGHT_TAG)
+		return object_date(((struct light_tag*)object)->tagged);
+
+	return 0;
+}
+
+static struct object_list *object_list_insert_by_date(struct object
*item, struct object_list **list)
+{
+	struct object_list **pp = list;
+	struct object_list *p;
+	unsigned long item_date;
+	unsigned long p_date;
+
+	if (!item->parsed)
+		return NULL;
+
+	while ((p = *pp) != NULL) {
+		p_date = object_date(p->item);
+		item_date = object_date(item);
+
+		if (p_date > item_date)
+			break;
+
+		pp = &p->next;
+	}
+	return object_list_insert(item, pp);
+}
+
+static void pretty_print_tag(const struct object *object, int lines)
+{
+	int i;
+	char *sp, *eol;
+	size_t len;
+
+	if (!lines) {
+		if (object->type == OBJ_TAG)
+			printf("%s\n", ((struct tag*)object)->tag);
+		else if (object->type == OBJ_LIGHT_TAG)
+			printf("%s\n", ((struct light_tag*)object)->refname);
+		/* other not implemented */
+		return;
+	}
+
+	if (object->type == OBJ_TAG) {
+		struct tag *tag;
+
+		tag = (struct tag*)object;
+		printf("%-15s ", tag->tag);
+
+		/* skip header */
+		sp = strstr(tag->buffer, "\n\n");
+		if (!sp)
+			return;
+
+		/* only take up to "lines" lines, and strip the signature */
+		for (i = 0, sp += 2;
+		     i < lines && sp < tag->buffer + tag->size &&
+			     prefixcmp(sp, PGP_SIGNATURE "\n");
+		     i++) {
+			if (i)
+				printf("\n    ");
+			eol = memchr(sp, '\n', tag->size - (sp - tag->buffer));
+			len = eol ? eol - sp : tag->size - (sp - tag->buffer);
+			fwrite(sp, len, 1, stdout);
+			if (!eol)
+				break;
+			sp = eol + 1;
+		}
+		putchar('\n');
+	}
+}
+
 static int show_reference(const char *refname, const unsigned char *sha1,
 			  int flag, void *cb_data)
 {
 	struct tag_filter *filter = cb_data;
+	struct object *object;

 	if (!fnmatch(filter->pattern, refname, 0)) {
-		int i;
-		unsigned long size;
-		enum object_type type;
-		char *buf, *sp, *eol;
-		size_t len;

 		if (filter->with_commit) {
 			struct commit *commit;
@@ -53,45 +140,43 @@ static int show_reference(const char *refname,
const unsigned char *sha1,
 				return 0;
 		}

-		if (!filter->lines) {
+		if (!filter->lines && !filter->sort) {
 			printf("%s\n", refname);
 			return 0;
 		}
-		printf("%-15s ", refname);

-		buf = read_sha1_file(sha1, &type, &size);
-		if (!buf || !size)
+		object = parse_object(sha1);
+		if (!object)
 			return 0;

-		/* skip header */
-		sp = strstr(buf, "\n\n");
-		if (!sp) {
-			free(buf);
-			return 0;
+		if (object->type != OBJ_TAG) {
+			struct light_tag *light_tag;
+			struct object *o;
+
+			o = xmalloc(sizeof(struct light_tag));
+			light_tag = (struct light_tag*)o;
+			o->parsed = 1;
+			o->used = 0;
+			o->type = OBJ_LIGHT_TAG;
+			o->flags = 0;
+			light_tag->tagged = object;
+			light_tag->refname = xstrdup(refname);
+			object = o;
 		}
-		/* only take up to "lines" lines, and strip the signature */
-		for (i = 0, sp += 2;
-				i < filter->lines && sp < buf + size &&
-				prefixcmp(sp, PGP_SIGNATURE "\n");
-				i++) {
-			if (i)
-				printf("\n    ");
-			eol = memchr(sp, '\n', size - (sp - buf));
-			len = eol ? eol - sp : size - (sp - buf);
-			fwrite(sp, len, 1, stdout);
-			if (!eol)
-				break;
-			sp = eol + 1;
+
+		if (filter->sort) {
+			object_list_insert_by_date(object, &filter->sorted_tags);
+			return 0;
 		}
-		putchar('\n');
-		free(buf);
+
+		pretty_print_tag(object, filter->lines);
 	}

 	return 0;
 }

 static int list_tags(const char *pattern, int lines,
-			struct commit_list *with_commit)
+			struct commit_list *with_commit, int sort)
 {
 	struct tag_filter filter;

@@ -101,9 +186,19 @@ static int list_tags(const char *pattern, int lines,
 	filter.pattern = pattern;
 	filter.lines = lines;
 	filter.with_commit = with_commit;
+	filter.sort = sort;
+	filter.sorted_tags = NULL;

 	for_each_tag_ref(show_reference, (void *) &filter);

+	if (filter.sort) {
+		struct object_list *l;
+		for (l = filter.sorted_tags; l; l = l->next) {
+			pretty_print_tag(l->item, lines);
+		}
+		/* free_object_list(filter.sorted_tags); */
+	}
+
 	return 0;
 }

@@ -370,12 +465,13 @@ int cmd_tag(int argc, const char **argv, const
char *prefix)
 	struct ref_lock *lock;

 	int annotate = 0, sign = 0, force = 0, lines = -1,
-		list = 0, delete = 0, verify = 0;
+		list = 0, delete = 0, verify = 0, sort = 0;
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct commit_list *with_commit = NULL;
 	struct option options[] = {
 		OPT_BOOLEAN('l', NULL, &list, "list tag names"),
+		OPT_BOOLEAN('D', NULL, &sort, "sort tag by date"),
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
 				"print n lines of each tag message",
 				PARSE_OPT_OPTARG, NULL, 1 },
@@ -425,7 +521,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_tag_usage, options);
 	if (list)
 		return list_tags(argv[0], lines == -1 ? 0 : lines,
-				 with_commit);
+				 with_commit, sort);
 	if (lines != -1)
 		die("-n option is only allowed with -l.");
 	if (with_commit)
-- 
1.6.2.rc1.28.g05ef4.dirty

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH/RFC 5/6] builtin-tag: add sort by date -D
  2009-02-22 18:06 [PATCH/RFC 5/6] builtin-tag: add sort by date -D Marc-André Lureau
@ 2009-02-22 18:33 ` Junio C Hamano
  2009-02-22 18:38 ` Felipe Contreras
  1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2009-02-22 18:33 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: git

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@gmail.com>
> ---
>  builtin-tag.c |  162 +++++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 129 insertions(+), 33 deletions(-)
>
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 01e7374..8ff9d03 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -16,7 +16,7 @@
>  static const char * const git_tag_usage[] = {
>  	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
>  	"git tag -d <tagname>...",
> -	"git tag -l [-n[<num>]] [<pattern>]",
> +	"git tag -l [-n[<num>] -D] [<pattern>]",

Please don't use a short-and-sweet "-D" for something whose usefulness is
not proven yet.  Especially this risks grief from typo-confusion with the
existing "-d" option that is destructive.

> +		if (object->type != OBJ_TAG) {
> +			struct light_tag *light_tag;
> +			struct object *o;
> +
> +			o = xmalloc(sizeof(struct light_tag));
> +			light_tag = (struct light_tag*)o;
> +			o->parsed = 1;
> +			o->used = 0;
> +			o->type = OBJ_LIGHT_TAG;
> +			o->flags = 0;
> +			light_tag->tagged = object;
> +			light_tag->refname = xstrdup(refname);
> +			object = o;

I really do not like this.  The only place you need a stand-in tag object
is inside this "sort tag objects and commits together", and you cannot
even handle lightweight tags that point at blobs or trees sanely with this
code anyway. It is not a good excuse to contaminate the object layer.

I think it might be a lot more sensible to introduce a structure like
this:

	struct tag_entry {
        	struct object *object;
                unsigned long date_for_sorting;
	};

and allocate and queue this structure in your for_each_ref callback
function, instead of the low-level objects.  If object *is* not a tag, you
can at that point find a suitable timestamp to stuff in date_for_sorting
(and if it is a tag, you can find a tagger field and parse the date into
date_for_sorting, which implies you do not necessarily need your patch 2/6
either and we can keep sizeof(struct tag) to the minimum as before).

Then your sort and output functions can sort and iterate over a list of
this structure.

You still need to think about what to do with lightweight tag that points
at a blob or a tree, though.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH/RFC 5/6] builtin-tag: add sort by date -D
  2009-02-22 18:06 [PATCH/RFC 5/6] builtin-tag: add sort by date -D Marc-André Lureau
  2009-02-22 18:33 ` Junio C Hamano
@ 2009-02-22 18:38 ` Felipe Contreras
  1 sibling, 0 replies; 3+ messages in thread
From: Felipe Contreras @ 2009-02-22 18:38 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: git

On Sun, Feb 22, 2009 at 8:06 PM, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@gmail.com>
> ---
>  builtin-tag.c |  162 +++++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 129 insertions(+), 33 deletions(-)
>
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 01e7374..8ff9d03 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -16,7 +16,7 @@
>  static const char * const git_tag_usage[] = {
>        "git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
>        "git tag -d <tagname>...",
> -       "git tag -l [-n[<num>]] [<pattern>]",
> +       "git tag -l [-n[<num>] -D] [<pattern>]",
>        "git tag -v <tagname>...",
>        NULL
>  };

Hmm, the -D option is independent of -n, so "[-D]". Also, it must be documented.

<snip/>

Wouldn't it make sense to split this patch so the first part adds the
option to filter, even if there are not filters functional, and the
second one adds the date filter?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-02-22 18:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-22 18:06 [PATCH/RFC 5/6] builtin-tag: add sort by date -D Marc-André Lureau
2009-02-22 18:33 ` Junio C Hamano
2009-02-22 18:38 ` Felipe Contreras

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).