From mboxrd@z Thu Jan 1 00:00:00 1970 From: whydoubt at gmail.com (Jeffrey Smith) Date: Sun, 24 Sep 2017 15:25:45 -0500 Subject: [RFCv2 PATCH 3/7] ui-blame: create links In-Reply-To: <20170923154731.GC2548@john.keeping.me.uk> References: <20170608021810.12964-1-whydoubt@gmail.com> <20170923033848.5922-1-whydoubt@gmail.com> <20170923033848.5922-4-whydoubt@gmail.com> <20170923154731.GC2548@john.keeping.me.uk> Message-ID: 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 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 >> --- >> 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 >> + * Copyright (C) 2006-2017 cgit Development Team >> * >> * 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("\n"); >> free(name); >> strbuf_release(&fullpath);