[PATCH] tag: reference with "h" instead of "id"

[Ferry Huberts]

Thanks for picking this up so fast John.
The solution is also what I preferred :-)

(didn't review the patch, been out of cgit dev for too long)

On 15/01/15 23:18, John Keeping wrote:
> When clicking on "log" from a tag we end up showing the log of whatever
> branch we used to reach the tag.  If the tag doesn't point onto a branch
> then the tagged commit won't appear in this output.
>
> By linking to tags with the head parameter instead of the "id" parameter
> the log link will show the log of the tag.  This is clearly desirable
> when the tag has been reached from the refs UI and changing the
> behaviour for tag decorations makes them match branch decorations where
> log -> decoration -> log shows the log of the decoration.
>
> Reported-by: Ferry Huberts <mailings at hupie.com>
> Signed-off-by: John Keeping <john at keeping.me.uk>
> ---
>> I think we actually want something like this:
>>
>> -- >8 --
>> diff --git a/ui-refs.c b/ui-refs.c
>> index bdd3b2c..d3017ec 100644
>> --- a/ui-refs.c
>> +++ b/ui-refs.c
>> @@ -140,7 +140,7 @@ static int print_tag(struct refinfo *ref)
>>   	}
>>
>>   	html("<tr><td>");
>> -	cgit_tag_link(name, NULL, NULL, ctx.qry.head, name);
>> +	cgit_tag_link(name, NULL, NULL, name, NULL);
>>   	html("</td><td>");
>>   	if (ctx.repo->snapshots && (obj->type == OBJ_COMMIT))
>>   		print_tag_downloads(ctx.repo, name);
>> -- 8< --
>>
>> The tag UI already does the right thing if we use "h" instead of "id".
>>
>> The only thing I'm not sure about is whether we should change all tag
>> links to behave like this.  That would make tag decorations consistent
>> with branch decorations which already change the head, so I think that
>> is the way to go.
>
> This is what that looks like.
>
>   ui-log.c    | 4 ++--
>   ui-refs.c   | 2 +-
>   ui-shared.c | 8 ++++----
>   ui-shared.h | 3 +--
>   4 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/ui-log.c b/ui-log.c
> index 657ff3c..1b60591 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -71,11 +71,11 @@ void show_commit_decorations(struct commit *commit)
>   		}
>   		else if (starts_with(deco->name, "tag: refs/tags/")) {
>   			strncpy(buf, deco->name + 15, sizeof(buf) - 1);
> -			cgit_tag_link(buf, NULL, "tag-deco", ctx.qry.head, buf);
> +			cgit_tag_link(buf, NULL, "tag-deco", buf);
>   		}
>   		else if (starts_with(deco->name, "refs/tags/")) {
>   			strncpy(buf, deco->name + 10, sizeof(buf) - 1);
> -			cgit_tag_link(buf, NULL, "tag-deco", ctx.qry.head, buf);
> +			cgit_tag_link(buf, NULL, "tag-deco", buf);
>   		}
>   		else if (starts_with(deco->name, "refs/remotes/")) {
>   			if (!ctx.repo->enable_remote_branches)
> diff --git a/ui-refs.c b/ui-refs.c
> index d2ba48d..ac8a6d4 100644
> --- a/ui-refs.c
> +++ b/ui-refs.c
> @@ -140,7 +140,7 @@ static int print_tag(struct refinfo *ref)
>   	}
>
>   	html("<tr><td>");
> -	cgit_tag_link(name, NULL, NULL, ctx.qry.head, name);
> +	cgit_tag_link(name, NULL, NULL, name);
>   	html("</td><td>");
>   	if (ctx.repo->snapshots && (obj->type == OBJ_COMMIT))
>   		print_tag_downloads(ctx.repo, name);
> diff --git a/ui-shared.c b/ui-shared.c
> index 32f23f9..d8cc4d7 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -273,9 +273,9 @@ void cgit_summary_link(const char *name, const char *title, const char *class,
>   }
>
>   void cgit_tag_link(const char *name, const char *title, const char *class,
> -		   const char *head, const char *rev)
> +		   const char *tag)
>   {
> -	reporevlink("tag", name, title, class, head, rev, NULL);
> +	reporevlink("tag", name, title, class, tag, NULL, NULL);
>   }
>
>   void cgit_tree_link(const char *name, const char *title, const char *class,
> @@ -443,8 +443,8 @@ static void cgit_self_link(char *name, const char *title, const char *class)
>   	else if (!strcmp(ctx.qry.page, "summary"))
>   		cgit_summary_link(name, title, class, ctx.qry.head);
>   	else if (!strcmp(ctx.qry.page, "tag"))
> -		cgit_tag_link(name, title, class, ctx.qry.head,
> -			      ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL);
> +		cgit_tag_link(name, title, class, ctx.qry.has_sha1 ?
> +			       ctx.qry.sha1 : ctx.qry.head);
>   	else if (!strcmp(ctx.qry.page, "tree"))
>   		cgit_tree_link(name, title, class, ctx.qry.head,
>   			       ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
> diff --git a/ui-shared.h b/ui-shared.h
> index f8cf220..021fe4e 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -18,8 +18,7 @@ extern void cgit_index_link(const char *name, const char *title,
>   extern void cgit_summary_link(const char *name, const char *title,
>   			      const char *class, const char *head);
>   extern void cgit_tag_link(const char *name, const char *title,
> -			  const char *class, const char *head,
> -			  const char *rev);
> +			  const char *class, const char *tag);
>   extern void cgit_tree_link(const char *name, const char *title,
>   			   const char *class, const char *head,
>   			   const char *rev, const char *path);
>

-- 
Ferry Huberts