[RFCv2 PATCH 3/7] ui-blame: create links

[Jeffrey Smith]

That pretty well sums up the dilemma I had in coming up with this solution.
It's not great, but it was the best compromise that I've found.
I would be as glad as anyone for a cleaner design.

On Sat, Sep 23, 2017 at 10:47 AM, John Keeping <john at keeping.me.uk> wrote:
> On Fri, Sep 22, 2017 at 10:38:44PM -0500, Jeff Smith wrote:
>> Create links to the blame page.
>>
>> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
>> ---
>>  ui-shared.c | 20 +++++++++++++++++---
>>  ui-shared.h |  3 +++
>>  ui-tree.c   | 10 +++++++++-
>>  3 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/ui-shared.c b/ui-shared.c
>> index e5c9a02..faa0d6a 100644
>> --- a/ui-shared.c
>> +++ b/ui-shared.c
>> @@ -986,8 +996,12 @@ void cgit_print_pageheader(void)
>>               cgit_log_link("log", NULL, hc("log"), ctx.qry.head,
>>                             NULL, ctx.qry.vpath, 0, NULL, NULL,
>>                             ctx.qry.showmsg, ctx.qry.follow);
>> -             cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
>> -                            ctx.qry.sha1, ctx.qry.vpath);
>> +             if (ctx.qry.page && !strcmp(ctx.qry.page, "blame"))
>> +                     cgit_blame_link("blame", NULL, hc("blame"), ctx.qry.head,
>> +                                     ctx.qry.sha1, ctx.qry.vpath);
>> +             else
>> +                     cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
>> +                                    ctx.qry.sha1, ctx.qry.vpath);
>
> I'm curious what other people think of this link, I'm not sure it's
> right but I can't think of a better behaviour.  In some sense it feels
> like blame is a subtype of tree so we should still show the tree link
> here, but that makes the "is it active?" check more complicated, and we
> can't add blame unconditionally because it has no meaning if the path is
> a tree rather than a blob.
>
>>               cgit_commit_link("commit", NULL, hc("commit"),
>>                                ctx.qry.head, ctx.qry.sha1, ctx.qry.vpath);
>>               cgit_diff_link("diff", NULL, hc("diff"), ctx.qry.head,
>> diff --git a/ui-shared.h b/ui-shared.h
>> index 87799f1..e5992bc 100644
>> --- a/ui-shared.h
>> +++ b/ui-shared.h
>> @@ -26,6 +26,9 @@ extern void cgit_tree_link(const char *name, const char *title,
>>  extern void cgit_plain_link(const char *name, const char *title,
>>                           const char *class, const char *head,
>>                           const char *rev, const char *path);
>> +extern void cgit_blame_link(const char *name, const char *title,
>> +                         const char *class, const char *head,
>> +                         const char *rev, const char *path);
>>  extern void cgit_log_link(const char *name, const char *title,
>>                         const char *class, const char *head, const char *rev,
>>                         const char *path, int ofs, const char *grep,
>> diff --git a/ui-tree.c b/ui-tree.c
>> index ca24a03..3513d5e 100644
>> --- a/ui-tree.c
>> +++ b/ui-tree.c
>> @@ -1,6 +1,6 @@
>>  /* ui-tree.c: functions for tree output
>>   *
>> - * Copyright (C) 2006-2014 cgit Development Team <cgit at lists.zx2c4.com>
>> + * Copyright (C) 2006-2017 cgit Development Team <cgit at lists.zx2c4.com>
>>   *
>>   * Licensed under GNU General Public License v2
>>   *   (see COPYING for full license text)
>> @@ -141,6 +141,11 @@ static void print_object(const unsigned char *sha1, char *path, const char *base
>>       htmlf("blob: %s (", sha1_to_hex(sha1));
>>       cgit_plain_link("plain", NULL, NULL, ctx.qry.head,
>>                       rev, path);
>> +     if (ctx.cfg.enable_blame) {
>> +             html(") (");
>> +             cgit_blame_link("blame", NULL, NULL, ctx.qry.head,
>> +                             rev, path);
>> +     }
>>       html(")\n");
>>
>>       if (ctx.cfg.max_blob_size && size / 1024 > ctx.cfg.max_blob_size) {
>> @@ -275,6 +280,9 @@ static int ls_item(const unsigned char *sha1, struct strbuf *base,
>>       if (!S_ISGITLINK(mode))
>>               cgit_plain_link("plain", NULL, "button", ctx.qry.head,
>>                               walk_tree_ctx->curr_rev, fullpath.buf);
>> +     if (!S_ISDIR(mode) && ctx.cfg.enable_blame)
>> +             cgit_blame_link("blame", NULL, "button", ctx.qry.head,
>> +                             walk_tree_ctx->curr_rev, fullpath.buf);
>>       html("</td></tr>\n");
>>       free(name);
>>       strbuf_release(&fullpath);