List for cgit developers and users
 help / color / mirror / Atom feed
* log of tags
@ 2015-01-15 16:18 mailings
  2015-01-15 17:27 ` cgit
  0 siblings, 1 reply; 6+ messages in thread
From: mailings @ 2015-01-15 16:18 UTC (permalink / raw)


Hi

Just noticed this:
You can't show the log of a tag when that tag is not on a branch, which 
happen to me a lot for expired maintenance branches.

Sound familiar?

How to fix?
Include a 'log' line, like the 'Tagged object' line?
Or just make clicking on the log 'tab' do the right thing? (it currently 
just shows the log of the default branch)

-- 
Ferry Huberts



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

* log of tags
  2015-01-15 16:18 log of tags mailings
@ 2015-01-15 17:27 ` cgit
  2015-01-15 21:55   ` john
  0 siblings, 1 reply; 6+ messages in thread
From: cgit @ 2015-01-15 17:27 UTC (permalink / raw)


On Thu, 15 Jan 2015 at 17:18:02, Ferry Huberts wrote:
> Hi
> 
> Just noticed this:
> You can't show the log of a tag when that tag is not on a branch, which 
> happen to me a lot for expired maintenance branches.
> 
> Sound familiar?
> 
> How to fix?
> Include a 'log' line, like the 'Tagged object' line?
> Or just make clicking on the log 'tab' do the right thing? (it currently 
> just shows the log of the default branch)
> [...]

Making the "log" link point to the log of the currently selected commit
is easy but I do not know whether that is what we want to do. It is a
convenient way to navigate back to the "full" log and I think this is
what most users would expect. Note that you can always manually append
"?id=$tag" to the log URI which is what I sometimes do when posting
links to the log of a release somewhere.

Anyway, if you still want the "make clicking on the log 'tab' do the
right thing" behavior, try this (untested):

-- >8 --
diff --git a/ui-shared.c b/ui-shared.c
index 32f23f9..2eb0d4e 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -928,7 +928,7 @@ void cgit_print_pageheader(void)
 		cgit_refs_link("refs", NULL, hc("refs"), ctx.qry.head,
 			       ctx.qry.sha1, NULL);
 		cgit_log_link("log", NULL, hc("log"), ctx.qry.head,
-			      NULL, ctx.qry.vpath, 0, NULL, NULL,
+			      ctx.qry.sha1, ctx.qry.vpath, 0, NULL, NULL,
 			      ctx.qry.showmsg);
 		cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
 			       ctx.qry.sha1, ctx.qry.vpath);


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

* log of tags
  2015-01-15 17:27 ` cgit
@ 2015-01-15 21:55   ` john
  2015-01-15 22:18     ` [PATCH] tag: reference with "h" instead of "id" john
  0 siblings, 1 reply; 6+ messages in thread
From: john @ 2015-01-15 21:55 UTC (permalink / raw)


On Thu, Jan 15, 2015 at 06:27:50PM +0100, Lukas Fleischer wrote:
> On Thu, 15 Jan 2015 at 17:18:02, Ferry Huberts wrote:
> > Hi
> > 
> > Just noticed this:
> > You can't show the log of a tag when that tag is not on a branch, which 
> > happen to me a lot for expired maintenance branches.
> > 
> > Sound familiar?
> > 
> > How to fix?
> > Include a 'log' line, like the 'Tagged object' line?
> > Or just make clicking on the log 'tab' do the right thing? (it currently 
> > just shows the log of the default branch)
> > [...]
> 
> Making the "log" link point to the log of the currently selected commit
> is easy but I do not know whether that is what we want to do. It is a
> convenient way to navigate back to the "full" log and I think this is
> what most users would expect. Note that you can always manually append
> "?id=$tag" to the log URI which is what I sometimes do when posting
> links to the log of a release somewhere.
> 
> Anyway, if you still want the "make clicking on the log 'tab' do the
> right thing" behavior, try this (untested):

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.


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

* [PATCH] tag: reference with "h" instead of "id"
  2015-01-15 21:55   ` john
@ 2015-01-15 22:18     ` john
  2015-01-15 22:23       ` mailings
  2015-01-19 16:52       ` Jason
  0 siblings, 2 replies; 6+ messages in thread
From: john @ 2015-01-15 22:18 UTC (permalink / raw)


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);
-- 
2.2.1.409.g417db82



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

* [PATCH] tag: reference with "h" instead of "id"
  2015-01-15 22:18     ` [PATCH] tag: reference with "h" instead of "id" john
@ 2015-01-15 22:23       ` mailings
  2015-01-19 16:52       ` Jason
  1 sibling, 0 replies; 6+ messages in thread
From: mailings @ 2015-01-15 22:23 UTC (permalink / raw)


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


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

* [PATCH] tag: reference with "h" instead of "id"
  2015-01-15 22:18     ` [PATCH] tag: reference with "h" instead of "id" john
  2015-01-15 22:23       ` mailings
@ 2015-01-19 16:52       ` Jason
  1 sibling, 0 replies; 6+ messages in thread
From: Jason @ 2015-01-19 16:52 UTC (permalink / raw)


Works well, thanks John. Applied and pushed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20150119/70c05827/attachment.html>


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

end of thread, other threads:[~2015-01-19 16:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-15 16:18 log of tags mailings
2015-01-15 17:27 ` cgit
2015-01-15 21:55   ` john
2015-01-15 22:18     ` [PATCH] tag: reference with "h" instead of "id" john
2015-01-15 22:23       ` mailings
2015-01-19 16:52       ` Jason

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).