Re: [PATCH/RFC 5/6] builtin-tag: add sort by date -D

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@xxxxxxxxx>
> ---
>  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux