* log --follow
@ 2013-04-15 11:51
2013-04-15 12:38 ` john
0 siblings, 1 reply; 18+ messages in thread
From: @ 2013-04-15 11:51 UTC (permalink / raw)
Hi all,
I just noticed that cgit does not show history over renames. Is it
somehow reasonable to add "--follow" by default, or make it an option?
Or are there reasons against it?
Thanks,
Ren?
^ permalink raw reply [flat|nested] 18+ messages in thread
* log --follow
2013-04-15 11:51 log --follow
@ 2013-04-15 12:38 ` john
2013-04-15 17:30 `
0 siblings, 1 reply; 18+ messages in thread
From: john @ 2013-04-15 12:38 UTC (permalink / raw)
On Mon, Apr 15, 2013 at 01:51:43PM +0200, Ren? Neumann wrote:
> I just noticed that cgit does not show history over renames. Is it
> somehow reasonable to add "--follow" by default, or make it an option?
> Or are there reasons against it?
I don't think it should be turned on unconditionally, since it does
require some more work to generate the log, but I see no reason why we
couldn't add an option for it.
The patch below should cause the log to be generated with "--follow",
but obviously it needs more work to be turned into a proper patch. If
this is useful I'll try to find some time to do this properly.
-- >8 --
diff --git a/ui-log.c b/ui-log.c
index 2aa12c3..01bddd1 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -342,7 +342,9 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
if (path) {
+ static const char *follow_arg = "--follow";
static const char *double_dash_arg = "--";
+ vector_push(&vec, &follow_arg, 0);
vector_push(&vec, &double_dash_arg, 0);
vector_push(&vec, &path, 0);
}
^ permalink raw reply [flat|nested] 18+ messages in thread
* log --follow
2013-04-15 12:38 ` john
@ 2013-04-15 17:30 `
2013-04-16 8:48 ` john
0 siblings, 1 reply; 18+ messages in thread
From: @ 2013-04-15 17:30 UTC (permalink / raw)
Am 15.04.2013 14:38, schrieb John Keeping:
> On Mon, Apr 15, 2013 at 01:51:43PM +0200, Ren? Neumann wrote:
>> I just noticed that cgit does not show history over renames. Is it
>> somehow reasonable to add "--follow" by default, or make it an option?
>> Or are there reasons against it?
>
> I don't think it should be turned on unconditionally, since it does
> require some more work to generate the log, but I see no reason why we
> couldn't add an option for it.
>
> The patch below should cause the log to be generated with "--follow",
> but obviously it needs more work to be turned into a proper patch. If
> this is useful I'll try to find some time to do this properly.
Unfortunately, this does not work. Now, when showing the log of one
file, it shows the complete history of the repository (where all
non-corresponding patches are empty).
I can't explain, why it behaves like that...
> -- >8 --
> diff --git a/ui-log.c b/ui-log.c
> index 2aa12c3..01bddd1 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -342,7 +342,9 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> }
>
> if (path) {
> + static const char *follow_arg = "--follow";
> static const char *double_dash_arg = "--";
> + vector_push(&vec, &follow_arg, 0);
> vector_push(&vec, &double_dash_arg, 0);
> vector_push(&vec, &path, 0);
> }
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* log --follow
2013-04-15 17:30 `
@ 2013-04-16 8:48 ` john
2013-04-17 19:48 ` [PATCH] log: allow users to follow a file john
0 siblings, 1 reply; 18+ messages in thread
From: john @ 2013-04-16 8:48 UTC (permalink / raw)
On Mon, Apr 15, 2013 at 07:30:11PM +0200, Ren? Neumann wrote:
> Am 15.04.2013 14:38, schrieb John Keeping:
> > On Mon, Apr 15, 2013 at 01:51:43PM +0200, Ren? Neumann wrote:
> >> I just noticed that cgit does not show history over renames. Is it
> >> somehow reasonable to add "--follow" by default, or make it an option?
> >> Or are there reasons against it?
> >
> > I don't think it should be turned on unconditionally, since it does
> > require some more work to generate the log, but I see no reason why we
> > couldn't add an option for it.
> >
> > The patch below should cause the log to be generated with "--follow",
> > but obviously it needs more work to be turned into a proper patch. If
> > this is useful I'll try to find some time to do this properly.
>
> Unfortunately, this does not work. Now, when showing the log of one
> file, it shows the complete history of the repository (where all
> non-corresponding patches are empty).
>
> I can't explain, why it behaves like that...
Interesting - I didn't test it earlier but I do get the same behaviour
you do.
It seems that passing "--follow" to the rev-parse machinery means that
when we walk the revisions we see every commit, since we must now
examine every commit so that the diff machinery can detect renames in
which we are interested. This means that we need to do more work when
examining a commit before deciding whether we should print it.
I started looking at this yesterday but need some more time to get my
head around the interactions of the different diff flags. I should have
some time to dig into this further this evening.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] log: allow users to follow a file
2013-04-16 8:48 ` john
@ 2013-04-17 19:48 ` john
2013-04-18 17:31 `
0 siblings, 1 reply; 18+ messages in thread
From: john @ 2013-04-17 19:48 UTC (permalink / raw)
Teach the "log" UI to behave in the same way as "git log --follow", when
given a suitable instruction by the user. The default behaviour remains
to show the log without following renames, but the follow behaviour can
be activated by following a link in the page header.
Follow is not the default because outputting merges in follow mode is
tricky ("git log --follow" will not show merges). We also disable the
graph in follow mode because the commit graph is not simplified so we
end up with frequent gaps in the graph and many lines that do not
connect with any commits we're actually showing.
Since following renames requires running diff on every commit we
consider, I've added a knob to the configuration file to globally
enable/disable this feature. Note that we may consider a large number
of commits the revision walking machinery no longer performs any path
limitation so we have to examine every commit until we find a page full
of commits that affect the target path or something related to it.
Suggested-by: Ren? Neumann <lists at necoro.eu>
Signed-off-by: John Keeping <john at keeping.me.uk>
---
cgit.c | 4 +++
cgit.h | 2 ++
cgitrc.5.txt | 4 +++
ui-log.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
ui-refs.c | 2 +-
ui-repolist.c | 2 +-
ui-shared.c | 20 +++++++++++---
ui-shared.h | 2 +-
ui-tree.c | 2 +-
9 files changed, 108 insertions(+), 17 deletions(-)
diff --git a/cgit.c b/cgit.c
index 6f44ef2..81312bb 100644
--- a/cgit.c
+++ b/cgit.c
@@ -171,6 +171,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
else if (!strcmp(name, "enable-filter-overrides"))
ctx.cfg.enable_filter_overrides = atoi(value);
+ else if (!strcmp(name, "enable-follow-links"))
+ ctx.cfg.enable_follow_links = atoi(value);
else if (!strcmp(name, "enable-http-clone"))
ctx.cfg.enable_http_clone = atoi(value);
else if (!strcmp(name, "enable-index-links"))
@@ -338,6 +340,8 @@ static void querystring_cb(const char *name, const char *value)
ctx.qry.context = atoi(value);
} else if (!strcmp(name, "ignorews")) {
ctx.qry.ignorews = atoi(value);
+ } else if (!strcmp(name, "follow")) {
+ ctx.qry.follow = atoi(value);
}
}
diff --git a/cgit.h b/cgit.h
index 850b792..6c0c429 100644
--- a/cgit.h
+++ b/cgit.h
@@ -163,6 +163,7 @@ struct cgit_query {
int show_all;
int context;
int ignorews;
+ int follow;
char *vpath;
};
@@ -203,6 +204,7 @@ struct cgit_config {
int case_sensitive_sort;
int embedded;
int enable_filter_overrides;
+ int enable_follow_links;
int enable_http_clone;
int enable_index_links;
int enable_index_owner;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 39b031e..48ef249 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -121,6 +121,10 @@ enable-filter-overrides::
Flag which, when set to "1", allows all filter settings to be
overridden in repository-specific cgitrc files. Default value: none.
+enable-follow-links::
+ Flag which, when set to "1", allows users to follow a file in the log
+ view. Default value: "0".
+
enable-http-clone::
If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
If you use an alternate way of serving git repositories, you may wish
diff --git a/ui-log.c b/ui-log.c
index 2aa12c3..d008387 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -66,7 +66,7 @@ void show_commit_decorations(struct commit *commit)
strncpy(buf, deco->name + 11, sizeof(buf) - 1);
cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else if (!prefixcmp(deco->name, "tag: refs/tags/")) {
strncpy(buf, deco->name + 15, sizeof(buf) - 1);
@@ -83,7 +83,7 @@ void show_commit_decorations(struct commit *commit)
cgit_log_link(buf, NULL, "remote-deco", NULL,
sha1_to_hex(commit->object.sha1),
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else {
strncpy(buf, deco->name, sizeof(buf) - 1);
@@ -96,6 +96,50 @@ next:
}
}
+static int show_commit(struct commit *commit, struct rev_info *revs)
+{
+ struct commit_list *parents = commit->parents;
+ struct commit *parent;
+ int found = 0, saved_fmt;
+ unsigned saved_flags = revs->diffopt.flags;
+
+
+ /* Always show if we're not in "follow" mode with a single file. */
+ if (!ctx.qry.follow)
+ return 1;
+
+ /*
+ * In "follow" mode, we don't show merges. This is consistent with
+ * "git log --follow -- <file>".
+ */
+ if (parents && parents->next)
+ return 0;
+
+ /*
+ * If this is the root commit, do what rev_info tells us.
+ */
+ if (!parents)
+ return revs->show_root_diff;
+
+ /* When we get here we have precisely one parent. */
+ parent = parents->item;
+ parse_commit(parent);
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ diff_tree_sha1(parent->tree->object.sha1,
+ commit->tree->object.sha1,
+ "", &revs->diffopt);
+ diffcore_std(&revs->diffopt);
+
+ found = !diff_queue_is_empty();
+ saved_fmt = revs->diffopt.output_format;
+ revs->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_flush(&revs->diffopt);
+ revs->diffopt.output_format = saved_fmt;
+ revs->diffopt.flags = saved_flags;
+
+ return found;
+}
+
static void print_commit(struct commit *commit, struct rev_info *revs)
{
struct commitinfo *info;
@@ -324,7 +368,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
}
}
- if (commit_graph) {
+
+ if (!path || !ctx.cfg.enable_follow_links) {
+ /*
+ * If we don't have a path, "follow" is a no-op so make sure
+ * the variable is set to false to avoid needing to check
+ * both this and whether we have a path everywhere.
+ */
+ ctx.qry.follow = 0;
+ }
+
+ if (commit_graph && !ctx.qry.follow) {
static const char *graph_arg = "--graph";
static const char *color_arg = "--color";
vector_push(&vec, &graph_arg, 0);
@@ -342,7 +396,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
if (path) {
+ static const char *follow_arg = "--follow";
static const char *double_dash_arg = "--";
+ if (ctx.qry.follow)
+ vector_push(&vec, &follow_arg, 0);
vector_push(&vec, &double_dash_arg, 0);
vector_push(&vec, &path, 0);
}
@@ -356,6 +413,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
rev.commit_format = CMIT_FMT_DEFAULT;
rev.verbose_header = 1;
rev.show_root_diff = 0;
+ rev.simplify_history = 1;
setup_revisions(vec.count, vec.data, &rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
@@ -377,7 +435,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
NULL, ctx.qry.head, ctx.qry.sha1,
ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
+ ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
+ ctx.qry.follow);
html(")");
}
html("</th><th class='left'>Author</th>");
@@ -396,15 +455,20 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
if (ofs<0)
ofs = 0;
- for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
+ for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL;) {
+ if (show_commit(commit, &rev))
+ i++;
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
commit->parents = NULL;
}
- for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
- print_commit(commit, &rev);
+ for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL;) {
+ if (show_commit(commit, &rev)) {
+ i++;
+ print_commit(commit, &rev);
+ }
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
@@ -417,7 +481,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs - cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
if ((commit = get_revision(&rev)) != NULL) {
@@ -425,14 +490,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs + cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
html("</ul>");
} else if ((commit = get_revision(&rev)) != NULL) {
htmlf("<tr class='nohover'><td colspan='%d'>", columns);
cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
- ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
+ ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</td></tr>\n");
}
diff --git a/ui-refs.c b/ui-refs.c
index 0ae0612..f4eefd1 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -71,7 +71,7 @@ static int print_branch(struct refinfo *ref)
return 1;
html("<tr><td>");
cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
html("</td><td>");
if (ref->object->type == OBJ_COMMIT) {
diff --git a/ui-repolist.c b/ui-repolist.c
index 47ca997..d96f252 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -314,7 +314,7 @@ void cgit_print_repolist()
html("<td>");
cgit_summary_link("summary", NULL, "button", NULL);
cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
- 0, NULL, NULL, ctx.qry.showmsg);
+ 0, NULL, NULL, ctx.qry.showmsg, 0);
cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
html("</td>");
}
diff --git a/ui-shared.c b/ui-shared.c
index 519eef7..24560ba 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -284,7 +284,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
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, const char *pattern, int showmsg)
+ int ofs, const char *grep, const char *pattern, int showmsg,
+ int follow)
{
char *delim;
@@ -313,6 +314,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
if (showmsg) {
html(delim);
html("showmsg=1");
+ delim = "&";
+ }
+ if (follow) {
+ html(delim);
+ html("follow=1");
}
html("'>");
html_txt(name);
@@ -452,7 +458,7 @@ static void cgit_self_link(char *name, const char *title, const char *class,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
ctx->qry.path, ctx->qry.ofs,
ctx->qry.grep, ctx->qry.search,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
else if (!strcmp(ctx->qry.page, "commit"))
cgit_commit_link(name, title, class, ctx->qry.head,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
@@ -854,7 +860,7 @@ void cgit_print_pageheader(struct cgit_context *ctx)
ctx->qry.sha1, NULL);
cgit_log_link("log", NULL, hc(ctx, "log"), ctx->qry.head,
NULL, ctx->qry.vpath, 0, NULL, NULL,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
cgit_tree_link("tree", NULL, hc(ctx, "tree"), ctx->qry.head,
ctx->qry.sha1, ctx->qry.vpath);
cgit_commit_link("commit", NULL, hc(ctx, "commit"),
@@ -906,6 +912,14 @@ void cgit_print_pageheader(struct cgit_context *ctx)
html("<div class='path'>");
html("path: ");
cgit_print_path_crumbs(ctx, ctx->qry.vpath);
+ if (ctx->cfg.enable_follow_links && !strcmp(ctx->qry.page, "log")) {
+ html(" (");
+ ctx->qry.follow = !ctx->qry.follow;
+ cgit_self_link(ctx->qry.follow ? "follow" : "unfollow",
+ NULL, NULL, ctx);
+ ctx->qry.follow = !ctx->qry.follow;
+ html(")");
+ }
html("</div>");
}
html("<div class='content'>");
diff --git a/ui-shared.h b/ui-shared.h
index 5987e77..3156846 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -26,7 +26,7 @@ extern void cgit_plain_link(const char *name, const char *title,
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,
- const char *pattern, int showmsg);
+ const char *pattern, int showmsg, int follow);
extern void cgit_commit_link(char *name, const char *title,
const char *class, const char *head,
const char *rev, const char *path,
diff --git a/ui-tree.c b/ui-tree.c
index aa5dee9..ebb3e9b 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -169,7 +169,7 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen,
html("<td>");
cgit_log_link("log", NULL, "button", ctx.qry.head,
walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
if (ctx.repo->max_stats)
cgit_stats_link("stats", NULL, "button", ctx.qry.head,
fullpath.buf);
--
1.8.2.694.ga76e9c3.dirty
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] log: allow users to follow a file
2013-04-17 19:48 ` [PATCH] log: allow users to follow a file john
@ 2013-04-18 17:31 `
2013-04-19 7:54 ` john
0 siblings, 1 reply; 18+ messages in thread
From: @ 2013-04-18 17:31 UTC (permalink / raw)
Thanks a lot for your patch, it is a huge step in the right direction
(it is way more involved than I could imagine). One issue remains: It
still displays empty diffstats (example: see [1]) for the commits
_before_ the rename, as they are limited to the _new_ name. This might
even lead to wrong commits if you have a commit "A -> B, C -> A" and are
looking at the history of the new file A.
> Suggested-by: Ren? Neumann <lists at necoro.eu>
Could you change the mail address to 'necoro at necoro.eu'?
- Ren?
[1] https://git.necoro.eu/web/kosten.git/log/templates/layout.jinja?follow=1
> Signed-off-by: John Keeping <john at keeping.me.uk>
> ---
> cgit.c | 4 +++
> cgit.h | 2 ++
> cgitrc.5.txt | 4 +++
> ui-log.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> ui-refs.c | 2 +-
> ui-repolist.c | 2 +-
> ui-shared.c | 20 +++++++++++---
> ui-shared.h | 2 +-
> ui-tree.c | 2 +-
> 9 files changed, 108 insertions(+), 17 deletions(-)
>
> diff --git a/cgit.c b/cgit.c
> index 6f44ef2..81312bb 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -171,6 +171,8 @@ static void config_cb(const char *name, const char *value)
> ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
> else if (!strcmp(name, "enable-filter-overrides"))
> ctx.cfg.enable_filter_overrides = atoi(value);
> + else if (!strcmp(name, "enable-follow-links"))
> + ctx.cfg.enable_follow_links = atoi(value);
> else if (!strcmp(name, "enable-http-clone"))
> ctx.cfg.enable_http_clone = atoi(value);
> else if (!strcmp(name, "enable-index-links"))
> @@ -338,6 +340,8 @@ static void querystring_cb(const char *name, const char *value)
> ctx.qry.context = atoi(value);
> } else if (!strcmp(name, "ignorews")) {
> ctx.qry.ignorews = atoi(value);
> + } else if (!strcmp(name, "follow")) {
> + ctx.qry.follow = atoi(value);
> }
> }
>
> diff --git a/cgit.h b/cgit.h
> index 850b792..6c0c429 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -163,6 +163,7 @@ struct cgit_query {
> int show_all;
> int context;
> int ignorews;
> + int follow;
> char *vpath;
> };
>
> @@ -203,6 +204,7 @@ struct cgit_config {
> int case_sensitive_sort;
> int embedded;
> int enable_filter_overrides;
> + int enable_follow_links;
> int enable_http_clone;
> int enable_index_links;
> int enable_index_owner;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 39b031e..48ef249 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -121,6 +121,10 @@ enable-filter-overrides::
> Flag which, when set to "1", allows all filter settings to be
> overridden in repository-specific cgitrc files. Default value: none.
>
> +enable-follow-links::
> + Flag which, when set to "1", allows users to follow a file in the log
> + view. Default value: "0".
> +
> enable-http-clone::
> If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
> If you use an alternate way of serving git repositories, you may wish
> diff --git a/ui-log.c b/ui-log.c
> index 2aa12c3..d008387 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -66,7 +66,7 @@ void show_commit_decorations(struct commit *commit)
> strncpy(buf, deco->name + 11, sizeof(buf) - 1);
> cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
> ctx.qry.vpath, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> }
> else if (!prefixcmp(deco->name, "tag: refs/tags/")) {
> strncpy(buf, deco->name + 15, sizeof(buf) - 1);
> @@ -83,7 +83,7 @@ void show_commit_decorations(struct commit *commit)
> cgit_log_link(buf, NULL, "remote-deco", NULL,
> sha1_to_hex(commit->object.sha1),
> ctx.qry.vpath, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> }
> else {
> strncpy(buf, deco->name, sizeof(buf) - 1);
> @@ -96,6 +96,50 @@ next:
> }
> }
>
> +static int show_commit(struct commit *commit, struct rev_info *revs)
> +{
> + struct commit_list *parents = commit->parents;
> + struct commit *parent;
> + int found = 0, saved_fmt;
> + unsigned saved_flags = revs->diffopt.flags;
> +
> +
> + /* Always show if we're not in "follow" mode with a single file. */
> + if (!ctx.qry.follow)
> + return 1;
> +
> + /*
> + * In "follow" mode, we don't show merges. This is consistent with
> + * "git log --follow -- <file>".
> + */
> + if (parents && parents->next)
> + return 0;
> +
> + /*
> + * If this is the root commit, do what rev_info tells us.
> + */
> + if (!parents)
> + return revs->show_root_diff;
> +
> + /* When we get here we have precisely one parent. */
> + parent = parents->item;
> + parse_commit(parent);
> + DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
> + diff_tree_sha1(parent->tree->object.sha1,
> + commit->tree->object.sha1,
> + "", &revs->diffopt);
> + diffcore_std(&revs->diffopt);
> +
> + found = !diff_queue_is_empty();
> + saved_fmt = revs->diffopt.output_format;
> + revs->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
> + diff_flush(&revs->diffopt);
> + revs->diffopt.output_format = saved_fmt;
> + revs->diffopt.flags = saved_flags;
> +
> + return found;
> +}
> +
> static void print_commit(struct commit *commit, struct rev_info *revs)
> {
> struct commitinfo *info;
> @@ -324,7 +368,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> }
> }
> }
> - if (commit_graph) {
> +
> + if (!path || !ctx.cfg.enable_follow_links) {
> + /*
> + * If we don't have a path, "follow" is a no-op so make sure
> + * the variable is set to false to avoid needing to check
> + * both this and whether we have a path everywhere.
> + */
> + ctx.qry.follow = 0;
> + }
> +
> + if (commit_graph && !ctx.qry.follow) {
> static const char *graph_arg = "--graph";
> static const char *color_arg = "--color";
> vector_push(&vec, &graph_arg, 0);
> @@ -342,7 +396,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> }
>
> if (path) {
> + static const char *follow_arg = "--follow";
> static const char *double_dash_arg = "--";
> + if (ctx.qry.follow)
> + vector_push(&vec, &follow_arg, 0);
> vector_push(&vec, &double_dash_arg, 0);
> vector_push(&vec, &path, 0);
> }
> @@ -356,6 +413,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> rev.commit_format = CMIT_FMT_DEFAULT;
> rev.verbose_header = 1;
> rev.show_root_diff = 0;
> + rev.simplify_history = 1;
> setup_revisions(vec.count, vec.data, &rev, NULL);
> load_ref_decorations(DECORATE_FULL_REFS);
> rev.show_decorations = 1;
> @@ -377,7 +435,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
> NULL, ctx.qry.head, ctx.qry.sha1,
> ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
> + ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
> + ctx.qry.follow);
> html(")");
> }
> html("</th><th class='left'>Author</th>");
> @@ -396,15 +455,20 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> if (ofs<0)
> ofs = 0;
>
> - for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
> + for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL;) {
> + if (show_commit(commit, &rev))
> + i++;
> free(commit->buffer);
> commit->buffer = NULL;
> free_commit_list(commit->parents);
> commit->parents = NULL;
> }
>
> - for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
> - print_commit(commit, &rev);
> + for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL;) {
> + if (show_commit(commit, &rev)) {
> + i++;
> + print_commit(commit, &rev);
> + }
> free(commit->buffer);
> commit->buffer = NULL;
> free_commit_list(commit->parents);
> @@ -417,7 +481,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
> ctx.qry.sha1, ctx.qry.vpath,
> ofs - cnt, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg);
> + ctx.qry.search, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</li>");
> }
> if ((commit = get_revision(&rev)) != NULL) {
> @@ -425,14 +490,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
> ctx.qry.sha1, ctx.qry.vpath,
> ofs + cnt, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg);
> + ctx.qry.search, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</li>");
> }
> html("</ul>");
> } else if ((commit = get_revision(&rev)) != NULL) {
> htmlf("<tr class='nohover'><td colspan='%d'>", columns);
> cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
> - ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
> + ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</td></tr>\n");
> }
>
> diff --git a/ui-refs.c b/ui-refs.c
> index 0ae0612..f4eefd1 100644
> --- a/ui-refs.c
> +++ b/ui-refs.c
> @@ -71,7 +71,7 @@ static int print_branch(struct refinfo *ref)
> return 1;
> html("<tr><td>");
> cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> html("</td><td>");
>
> if (ref->object->type == OBJ_COMMIT) {
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 47ca997..d96f252 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -314,7 +314,7 @@ void cgit_print_repolist()
> html("<td>");
> cgit_summary_link("summary", NULL, "button", NULL);
> cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
> - 0, NULL, NULL, ctx.qry.showmsg);
> + 0, NULL, NULL, ctx.qry.showmsg, 0);
> cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
> html("</td>");
> }
> diff --git a/ui-shared.c b/ui-shared.c
> index 519eef7..24560ba 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -284,7 +284,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
>
> 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, const char *pattern, int showmsg)
> + int ofs, const char *grep, const char *pattern, int showmsg,
> + int follow)
> {
> char *delim;
>
> @@ -313,6 +314,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
> if (showmsg) {
> html(delim);
> html("showmsg=1");
> + delim = "&";
> + }
> + if (follow) {
> + html(delim);
> + html("follow=1");
> }
> html("'>");
> html_txt(name);
> @@ -452,7 +458,7 @@ static void cgit_self_link(char *name, const char *title, const char *class,
> ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
> ctx->qry.path, ctx->qry.ofs,
> ctx->qry.grep, ctx->qry.search,
> - ctx->qry.showmsg);
> + ctx->qry.showmsg, ctx->qry.follow);
> else if (!strcmp(ctx->qry.page, "commit"))
> cgit_commit_link(name, title, class, ctx->qry.head,
> ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
> @@ -854,7 +860,7 @@ void cgit_print_pageheader(struct cgit_context *ctx)
> ctx->qry.sha1, NULL);
> cgit_log_link("log", NULL, hc(ctx, "log"), ctx->qry.head,
> NULL, ctx->qry.vpath, 0, NULL, NULL,
> - ctx->qry.showmsg);
> + ctx->qry.showmsg, ctx->qry.follow);
> cgit_tree_link("tree", NULL, hc(ctx, "tree"), ctx->qry.head,
> ctx->qry.sha1, ctx->qry.vpath);
> cgit_commit_link("commit", NULL, hc(ctx, "commit"),
> @@ -906,6 +912,14 @@ void cgit_print_pageheader(struct cgit_context *ctx)
> html("<div class='path'>");
> html("path: ");
> cgit_print_path_crumbs(ctx, ctx->qry.vpath);
> + if (ctx->cfg.enable_follow_links && !strcmp(ctx->qry.page, "log")) {
> + html(" (");
> + ctx->qry.follow = !ctx->qry.follow;
> + cgit_self_link(ctx->qry.follow ? "follow" : "unfollow",
> + NULL, NULL, ctx);
> + ctx->qry.follow = !ctx->qry.follow;
> + html(")");
> + }
> html("</div>");
> }
> html("<div class='content'>");
> diff --git a/ui-shared.h b/ui-shared.h
> index 5987e77..3156846 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -26,7 +26,7 @@ extern void cgit_plain_link(const char *name, const char *title,
> 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,
> - const char *pattern, int showmsg);
> + const char *pattern, int showmsg, int follow);
> extern void cgit_commit_link(char *name, const char *title,
> const char *class, const char *head,
> const char *rev, const char *path,
> diff --git a/ui-tree.c b/ui-tree.c
> index aa5dee9..ebb3e9b 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -169,7 +169,7 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen,
> html("<td>");
> cgit_log_link("log", NULL, "button", ctx.qry.head,
> walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> if (ctx.repo->max_stats)
> cgit_stats_link("stats", NULL, "button", ctx.qry.head,
> fullpath.buf);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] log: allow users to follow a file
2013-04-18 17:31 `
@ 2013-04-19 7:54 ` john
2013-04-20 10:14 ` [PATCH v2 0/2] Allow users to follow file renames john
0 siblings, 1 reply; 18+ messages in thread
From: john @ 2013-04-19 7:54 UTC (permalink / raw)
On Thu, Apr 18, 2013 at 07:31:55PM +0200, Ren? Neumann wrote:
> Thanks a lot for your patch, it is a huge step in the right direction
> (it is way more involved than I could imagine). One issue remains: It
> still displays empty diffstats (example: see [1]) for the commits
> _before_ the rename, as they are limited to the _new_ name. This might
> even lead to wrong commits if you have a commit "A -> B, C -> A" and are
> looking at the history of the new file A.
Thanks for trying this out. I need to think about how to do this for a
bit because we'll need to change the links in the log to point to the
correct (pre-rename) file path, but then the "Log" link at the top of
the page will link to the log with that file name and we will only show
the pre-rename commits.
The answer to that should help with the corner case of the rename commit
itself, where we want to show both file paths, but I think it becomes
complicated when there are multiple renames in the history.
I don't really want to introduce two new query parameters, one of them
just so that clicking "Log" does the same as navigating back in the
browser. Perhaps we should accept that once you click on a pre-rename
commit you are now examining that file path, so clicking "Log" doesn't
show any changes after it has been renamed.
That way we only need a bit of extra state to show the rename commit
itself, and the "follow" flag I'm already introducing should be
sufficient for that.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] Allow users to follow file renames
2013-04-19 7:54 ` john
@ 2013-04-20 10:14 ` john
2013-04-20 10:14 ` [PATCH v2 1/2] shared: make cgit_diff_tree_cb public john
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: john @ 2013-04-20 10:14 UTC (permalink / raw)
I've split this into two patches now because getting the file and line
counts right in the log view is made easier if we can reuse the
cgit_diff_tree_cb function from shared.c.
Most of the changes from v1 involve getting things right in the commits
where we actually perform a rename. Getting the commit/diff links right
just involves updating the context's vpath when we run the diff to see
if the file has changed so that the commit & diff links specify the file
as it was known in that commit.
John Keeping (2):
shared: make cgit_diff_tree_cb public
log: allow users to follow a file
cgit.c | 4 ++
cgit.h | 5 +++
cgitrc.5.txt | 4 ++
shared.c | 4 +-
ui-diff.c | 35 ++++++++++++++++
ui-log.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
ui-refs.c | 2 +-
ui-repolist.c | 2 +-
ui-shared.c | 28 +++++++++++--
ui-shared.h | 2 +-
ui-tree.c | 2 +-
11 files changed, 199 insertions(+), 22 deletions(-)
--
1.8.2.1.715.gb260f47
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] shared: make cgit_diff_tree_cb public
2013-04-20 10:14 ` [PATCH v2 0/2] Allow users to follow file renames john
@ 2013-04-20 10:14 ` john
2013-04-20 10:14 ` [PATCH v2 2/2] log: allow users to follow a file john
2013-04-26 13:52 ` [PATCH v2 0/2] Allow users to follow file renames Jason
2 siblings, 0 replies; 18+ messages in thread
From: john @ 2013-04-20 10:14 UTC (permalink / raw)
This will allow us to use this nice wrapper function elsewhere, avoiding
dealing with the diff queue when we only need to inspect a filepair.
Signed-off-by: John Keeping <john at keeping.me.uk>
---
cgit.h | 3 +++
shared.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/cgit.h b/cgit.h
index 850b792..b56e756 100644
--- a/cgit.h
+++ b/cgit.h
@@ -315,6 +315,9 @@ extern int cgit_refs_cb(const char *refname, const unsigned char *sha1,
extern void *cgit_free_commitinfo(struct commitinfo *info);
+void cgit_diff_tree_cb(struct diff_queue_struct *q,
+ struct diff_options *options, void *data);
+
extern int cgit_diff_files(const unsigned char *old_sha1,
const unsigned char *new_sha1,
unsigned long *old_size, unsigned long *new_size,
diff --git a/shared.c b/shared.c
index 4369378..4a73ee6 100644
--- a/shared.c
+++ b/shared.c
@@ -245,8 +245,8 @@ int cgit_refs_cb(const char *refname, const unsigned char *sha1, int flags,
return 0;
}
-static void cgit_diff_tree_cb(struct diff_queue_struct *q,
- struct diff_options *options, void *data)
+void cgit_diff_tree_cb(struct diff_queue_struct *q,
+ struct diff_options *options, void *data)
{
int i;
--
1.8.2.1.715.gb260f47
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] log: allow users to follow a file
2013-04-20 10:14 ` [PATCH v2 0/2] Allow users to follow file renames john
2013-04-20 10:14 ` [PATCH v2 1/2] shared: make cgit_diff_tree_cb public john
@ 2013-04-20 10:14 ` john
2013-04-20 11:21 `
2013-04-20 12:21 ` [PATCH 2/2 v3] " john
2013-04-26 13:52 ` [PATCH v2 0/2] Allow users to follow file renames Jason
2 siblings, 2 replies; 18+ messages in thread
From: john @ 2013-04-20 10:14 UTC (permalink / raw)
Teach the "log" UI to behave in the same way as "git log --follow", when
given a suitable instruction by the user. The default behaviour remains
to show the log without following renames, but the follow behaviour can
be activated by following a link in the page header.
Follow is not the default because outputting merges in follow mode is
tricky ("git log --follow" will not show merges). We also disable the
graph in follow mode because the commit graph is not simplified so we
end up with frequent gaps in the graph and many lines that do not
connect with any commits we're actually showing.
We also teach the "diff" and "commit" UIs to respect the follow flag on
URLs, causing the single-file version of these UIs to detect renames.
This feature is needed only for commits that rename the path we're
interested in.
For commits before the file has been renamed (i.e. that appear later in
the log list) we change the file path in the links from the log to point
to the old name; this means that links to commits always limit by the
path known to that commit. If we didn't do this we would need to walk
down the log diff'ing every commit whenever we want to show a commit.
The drawback is that the "Log" link in the top bar of such a page links
to the log limited by the old name, so it will only show pre-rename
commits. I consider this a reasonable trade-off since the "Back" button
still works and the log matches the path displayed in the top bar.
Since following renames requires running diff on every commit we
consider, I've added a knob to the configuration file to globally
enable/disable this feature. Note that we may consider a large number
of commits the revision walking machinery no longer performs any path
limitation so we have to examine every commit until we find a page full
of commits that affect the target path or something related to it.
Suggested-by: Ren? Neumann <necoro at necoro.eu>
Signed-off-by: John Keeping <john at keeping.me.uk>
---
cgit.c | 4 ++
cgit.h | 2 +
cgitrc.5.txt | 4 ++
ui-diff.c | 35 +++++++++++++++
ui-log.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
ui-refs.c | 2 +-
ui-repolist.c | 2 +-
ui-shared.c | 28 ++++++++++--
ui-shared.h | 2 +-
ui-tree.c | 2 +-
10 files changed, 195 insertions(+), 20 deletions(-)
diff --git a/cgit.c b/cgit.c
index 6f44ef2..81312bb 100644
--- a/cgit.c
+++ b/cgit.c
@@ -171,6 +171,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
else if (!strcmp(name, "enable-filter-overrides"))
ctx.cfg.enable_filter_overrides = atoi(value);
+ else if (!strcmp(name, "enable-follow-links"))
+ ctx.cfg.enable_follow_links = atoi(value);
else if (!strcmp(name, "enable-http-clone"))
ctx.cfg.enable_http_clone = atoi(value);
else if (!strcmp(name, "enable-index-links"))
@@ -338,6 +340,8 @@ static void querystring_cb(const char *name, const char *value)
ctx.qry.context = atoi(value);
} else if (!strcmp(name, "ignorews")) {
ctx.qry.ignorews = atoi(value);
+ } else if (!strcmp(name, "follow")) {
+ ctx.qry.follow = atoi(value);
}
}
diff --git a/cgit.h b/cgit.h
index b56e756..c0758b0 100644
--- a/cgit.h
+++ b/cgit.h
@@ -163,6 +163,7 @@ struct cgit_query {
int show_all;
int context;
int ignorews;
+ int follow;
char *vpath;
};
@@ -203,6 +204,7 @@ struct cgit_config {
int case_sensitive_sort;
int embedded;
int enable_filter_overrides;
+ int enable_follow_links;
int enable_http_clone;
int enable_index_links;
int enable_index_owner;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 39b031e..48ef249 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -121,6 +121,10 @@ enable-filter-overrides::
Flag which, when set to "1", allows all filter settings to be
overridden in repository-specific cgitrc files. Default value: none.
+enable-follow-links::
+ Flag which, when set to "1", allows users to follow a file in the log
+ view. Default value: "0".
+
enable-http-clone::
If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
If you use an alternate way of serving git repositories, you may wish
diff --git a/ui-diff.c b/ui-diff.c
index 8b38209..2d75f34 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -36,6 +36,7 @@ static struct fileinfo {
static int use_ssdiff = 0;
static struct diff_filepair *current_filepair;
+static const char *current_prefix;
struct diff_filespec *cgit_get_current_old_file(void)
{
@@ -132,11 +133,30 @@ static void count_diff_lines(char *line, int len)
}
}
+static int show_filepair(struct diff_filepair *pair)
+{
+ /* Always show if we have no limiting prefix. */
+ if (!current_prefix)
+ return 1;
+
+ /* Show if either path in the pair begins with the prefix. */
+ if (!prefixcmp(pair->one->path, current_prefix) ||
+ !prefixcmp(pair->two->path, current_prefix))
+ return 1;
+
+ /* Otherwise we don't want to show this filepair. */
+ return 0;
+}
+
static void inspect_filepair(struct diff_filepair *pair)
{
int binary = 0;
unsigned long old_size = 0;
unsigned long new_size = 0;
+
+ if (!show_filepair(pair))
+ return;
+
files++;
lines_added = 0;
lines_removed = 0;
@@ -279,6 +299,9 @@ static void filepair_cb(struct diff_filepair *pair)
int binary = 0;
linediff_fn print_line_fn = print_line;
+ if (!show_filepair(pair))
+ return;
+
current_filepair = pair;
if (use_ssdiff) {
cgit_ssdiff_header_begin();
@@ -364,6 +387,18 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
unsigned long size;
struct commit *commit, *commit2;
+ /*
+ * If "follow" is set then the diff machinery needs to examine the
+ * entire commit to detect renames so we must limit the paths in our
+ * own callbacks and not pass the prefix to the diff machinery.
+ */
+ if (ctx.qry.follow && ctx.cfg.enable_follow_links) {
+ current_prefix = prefix;
+ prefix = "";
+ } else {
+ current_prefix = NULL;
+ }
+
if (!new_rev)
new_rev = ctx.qry.head;
get_sha1(new_rev, new_rev_sha1);
diff --git a/ui-log.c b/ui-log.c
index 2aa12c3..f46e8f2 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -12,7 +12,7 @@
#include "ui-shared.h"
#include "vector.h"
-int files, add_lines, rem_lines;
+int files, add_lines, rem_lines, lines_counted;
/*
* The list of available column colors in the commit graph.
@@ -66,7 +66,7 @@ void show_commit_decorations(struct commit *commit)
strncpy(buf, deco->name + 11, sizeof(buf) - 1);
cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else if (!prefixcmp(deco->name, "tag: refs/tags/")) {
strncpy(buf, deco->name + 15, sizeof(buf) - 1);
@@ -83,7 +83,7 @@ void show_commit_decorations(struct commit *commit)
cgit_log_link(buf, NULL, "remote-deco", NULL,
sha1_to_hex(commit->object.sha1),
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else {
strncpy(buf, deco->name, sizeof(buf) - 1);
@@ -96,6 +96,72 @@ next:
}
}
+static void handle_rename(struct diff_filepair *pair)
+{
+ /*
+ * After we have seen a rename, we generate links to the previous
+ * name of the file so that commit & diff views get fed the path
+ * that is correct for the commit they are showing, avoiding the
+ * need to walk the entire history leading back to every commit we
+ * show in order detect renames.
+ */
+ free(ctx.qry.vpath);
+ ctx.qry.vpath = xstrdup(pair->two->path);
+ inspect_files(pair);
+}
+
+static int show_commit(struct commit *commit, struct rev_info *revs)
+{
+ struct commit_list *parents = commit->parents;
+ struct commit *parent;
+ int found = 0, saved_fmt;
+ unsigned saved_flags = revs->diffopt.flags;
+
+
+ /* Always show if we're not in "follow" mode with a single file. */
+ if (!ctx.qry.follow)
+ return 1;
+
+ /*
+ * In "follow" mode, we don't show merges. This is consistent with
+ * "git log --follow -- <file>".
+ */
+ if (parents && parents->next)
+ return 0;
+
+ /*
+ * If this is the root commit, do what rev_info tells us.
+ */
+ if (!parents)
+ return revs->show_root_diff;
+
+ /* When we get here we have precisely one parent. */
+ parent = parents->item;
+ parse_commit(parent);
+
+ files = 0;
+ add_lines = 0;
+ rem_lines = 0;
+
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ diff_tree_sha1(parent->tree->object.sha1,
+ commit->tree->object.sha1,
+ "", &revs->diffopt);
+ diffcore_std(&revs->diffopt);
+
+ found = !diff_queue_is_empty();
+ saved_fmt = revs->diffopt.output_format;
+ revs->diffopt.output_format = DIFF_FORMAT_CALLBACK;
+ revs->diffopt.format_callback = cgit_diff_tree_cb;
+ revs->diffopt.format_callback_data = handle_rename;
+ diff_flush(&revs->diffopt);
+ revs->diffopt.output_format = saved_fmt;
+ revs->diffopt.flags = saved_flags;
+
+ lines_counted = 1;
+ return found;
+}
+
static void print_commit(struct commit *commit, struct rev_info *revs)
{
struct commitinfo *info;
@@ -173,7 +239,8 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
cgit_print_age(commit->date, TM_WEEK * 2, FMT_SHORTDATE);
}
- if (ctx.repo->enable_log_filecount || ctx.repo->enable_log_linecount) {
+ if (!lines_counted && (ctx.repo->enable_log_filecount ||
+ ctx.repo->enable_log_linecount)) {
files = 0;
add_lines = 0;
rem_lines = 0;
@@ -324,7 +391,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
}
}
- if (commit_graph) {
+
+ if (!path || !ctx.cfg.enable_follow_links) {
+ /*
+ * If we don't have a path, "follow" is a no-op so make sure
+ * the variable is set to false to avoid needing to check
+ * both this and whether we have a path everywhere.
+ */
+ ctx.qry.follow = 0;
+ }
+
+ if (commit_graph && !ctx.qry.follow) {
static const char *graph_arg = "--graph";
static const char *color_arg = "--color";
vector_push(&vec, &graph_arg, 0);
@@ -342,7 +419,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
if (path) {
+ static const char *follow_arg = "--follow";
static const char *double_dash_arg = "--";
+ if (ctx.qry.follow)
+ vector_push(&vec, &follow_arg, 0);
vector_push(&vec, &double_dash_arg, 0);
vector_push(&vec, &path, 0);
}
@@ -356,10 +436,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
rev.commit_format = CMIT_FMT_DEFAULT;
rev.verbose_header = 1;
rev.show_root_diff = 0;
+ rev.simplify_history = 1;
setup_revisions(vec.count, vec.data, &rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
rev.grep_filter.regflags |= REG_ICASE;
+
+ rev.diffopt.detect_rename = 1;
+ rev.diffopt.rename_limit = ctx.cfg.renamelimit;
+ if (ctx.qry.ignorews)
+ DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
+
compile_grep_patterns(&rev.grep_filter);
prepare_revision_walk(&rev);
@@ -377,11 +464,12 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
NULL, ctx.qry.head, ctx.qry.sha1,
ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
+ ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
+ ctx.qry.follow);
html(")");
}
html("</th><th class='left'>Author</th>");
- if (commit_graph)
+ if (rev.graph)
html("<th class='left'>Age</th>");
if (ctx.repo->enable_log_filecount) {
html("<th class='left'>Files</th>");
@@ -396,15 +484,32 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
if (ofs<0)
ofs = 0;
- for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
+ for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL;) {
+ if (show_commit(commit, &rev))
+ i++;
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
commit->parents = NULL;
}
- for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
- print_commit(commit, &rev);
+ for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL;) {
+ /*
+ * In "follow" mode, we must count the files and lines the
+ * first time we invoke diff on a given commit, and we need
+ * to do that to see if the commit touches the path we care
+ * about, so we do it in show_commit. Hence we must clear
+ * lines_counted here.
+ *
+ * This has the side effect of avoiding running diff twice
+ * when we are both following renames and showing file
+ * and/or line counts.
+ */
+ lines_counted = 0;
+ if (show_commit(commit, &rev)) {
+ i++;
+ print_commit(commit, &rev);
+ }
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
@@ -417,7 +522,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs - cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
if ((commit = get_revision(&rev)) != NULL) {
@@ -425,14 +531,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs + cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
html("</ul>");
} else if ((commit = get_revision(&rev)) != NULL) {
htmlf("<tr class='nohover'><td colspan='%d'>", columns);
cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
- ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
+ ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</td></tr>\n");
}
diff --git a/ui-refs.c b/ui-refs.c
index 0ae0612..f4eefd1 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -71,7 +71,7 @@ static int print_branch(struct refinfo *ref)
return 1;
html("<tr><td>");
cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
html("</td><td>");
if (ref->object->type == OBJ_COMMIT) {
diff --git a/ui-repolist.c b/ui-repolist.c
index 47ca997..d96f252 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -314,7 +314,7 @@ void cgit_print_repolist()
html("<td>");
cgit_summary_link("summary", NULL, "button", NULL);
cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
- 0, NULL, NULL, ctx.qry.showmsg);
+ 0, NULL, NULL, ctx.qry.showmsg, 0);
cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
html("</td>");
}
diff --git a/ui-shared.c b/ui-shared.c
index 519eef7..04d0368 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -284,7 +284,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
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, const char *pattern, int showmsg)
+ int ofs, const char *grep, const char *pattern, int showmsg,
+ int follow)
{
char *delim;
@@ -313,6 +314,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
if (showmsg) {
html(delim);
html("showmsg=1");
+ delim = "&";
+ }
+ if (follow) {
+ html(delim);
+ html("follow=1");
}
html("'>");
html_txt(name);
@@ -355,6 +361,10 @@ void cgit_commit_link(char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
if (name[0] != '\0')
html_txt(name);
@@ -411,6 +421,10 @@ void cgit_diff_link(const char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
html_txt(name);
html("</a>");
@@ -452,7 +466,7 @@ static void cgit_self_link(char *name, const char *title, const char *class,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
ctx->qry.path, ctx->qry.ofs,
ctx->qry.grep, ctx->qry.search,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
else if (!strcmp(ctx->qry.page, "commit"))
cgit_commit_link(name, title, class, ctx->qry.head,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
@@ -854,7 +868,7 @@ void cgit_print_pageheader(struct cgit_context *ctx)
ctx->qry.sha1, NULL);
cgit_log_link("log", NULL, hc(ctx, "log"), ctx->qry.head,
NULL, ctx->qry.vpath, 0, NULL, NULL,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
cgit_tree_link("tree", NULL, hc(ctx, "tree"), ctx->qry.head,
ctx->qry.sha1, ctx->qry.vpath);
cgit_commit_link("commit", NULL, hc(ctx, "commit"),
@@ -906,6 +920,14 @@ void cgit_print_pageheader(struct cgit_context *ctx)
html("<div class='path'>");
html("path: ");
cgit_print_path_crumbs(ctx, ctx->qry.vpath);
+ if (ctx->cfg.enable_follow_links && !strcmp(ctx->qry.page, "log")) {
+ html(" (");
+ ctx->qry.follow = !ctx->qry.follow;
+ cgit_self_link(ctx->qry.follow ? "follow" : "unfollow",
+ NULL, NULL, ctx);
+ ctx->qry.follow = !ctx->qry.follow;
+ html(")");
+ }
html("</div>");
}
html("<div class='content'>");
diff --git a/ui-shared.h b/ui-shared.h
index 5987e77..3156846 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -26,7 +26,7 @@ extern void cgit_plain_link(const char *name, const char *title,
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,
- const char *pattern, int showmsg);
+ const char *pattern, int showmsg, int follow);
extern void cgit_commit_link(char *name, const char *title,
const char *class, const char *head,
const char *rev, const char *path,
diff --git a/ui-tree.c b/ui-tree.c
index aa5dee9..ebb3e9b 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -169,7 +169,7 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen,
html("<td>");
cgit_log_link("log", NULL, "button", ctx.qry.head,
walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
if (ctx.repo->max_stats)
cgit_stats_link("stats", NULL, "button", ctx.qry.head,
fullpath.buf);
--
1.8.2.1.715.gb260f47
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] log: allow users to follow a file
2013-04-20 10:14 ` [PATCH v2 2/2] log: allow users to follow a file john
@ 2013-04-20 11:21 `
2013-04-20 12:21 ` [PATCH 2/2 v3] " john
1 sibling, 0 replies; 18+ messages in thread
From: @ 2013-04-20 11:21 UTC (permalink / raw)
Awesome! Your patches work great over here. I'm so glad, I had not tried
to implement this myself :)
> For commits before the file has been renamed (i.e. that appear later in
> the log list) we change the file path in the links from the log to point
> to the old name; this means that links to commits always limit by the
> path known to that commit. If we didn't do this we would need to walk
> down the log diff'ing every commit whenever we want to show a commit.
> The drawback is that the "Log" link in the top bar of such a page links
> to the log limited by the old name, so it will only show pre-rename
> commits. I consider this a reasonable trade-off since the "Back" button
> still works and the log matches the path displayed in the top bar.
I also think this is reasonable. When running into this issue, you
usually already know what you are doing, so one might be surprised but
not lost.
- Ren?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2 v3] log: allow users to follow a file
2013-04-20 10:14 ` [PATCH v2 2/2] log: allow users to follow a file john
2013-04-20 11:21 `
@ 2013-04-20 12:21 ` john
2015-03-24 6:02 `
2015-08-12 14:34 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public john
1 sibling, 2 replies; 18+ messages in thread
From: john @ 2013-04-20 12:21 UTC (permalink / raw)
Teach the "log" UI to behave in the same way as "git log --follow", when
given a suitable instruction by the user. The default behaviour remains
to show the log without following renames, but the follow behaviour can
be activated by following a link in the page header.
Follow is not the default because outputting merges in follow mode is
tricky ("git log --follow" will not show merges). We also disable the
graph in follow mode because the commit graph is not simplified so we
end up with frequent gaps in the graph and many lines that do not
connect with any commits we're actually showing.
We also teach the "diff" and "commit" UIs to respect the follow flag on
URLs, causing the single-file version of these UIs to detect renames.
This feature is needed only for commits that rename the path we're
interested in.
For commits before the file has been renamed (i.e. that appear later in
the log list) we change the file path in the links from the log to point
to the old name; this means that links to commits always limit by the
path known to that commit. If we didn't do this we would need to walk
down the log diff'ing every commit whenever we want to show a commit.
The drawback is that the "Log" link in the top bar of such a page links
to the log limited by the old name, so it will only show pre-rename
commits. I consider this a reasonable trade-off since the "Back" button
still works and the log matches the path displayed in the top bar.
Since following renames requires running diff on every commit we
consider, I've added a knob to the configuration file to globally
enable/disable this feature. Note that we may consider a large number
of commits the revision walking machinery no longer performs any path
limitation so we have to examine every commit until we find a page full
of commits that affect the target path or something related to it.
Suggested-by: Ren? Neumann <necoro at necoro.eu>
Signed-off-by: John Keeping <john at keeping.me.uk>
---
I've just realised I didn't commit a final fixup to this before sending
it out. The difference is in handle_rename() which doesn't leak memory
in this version.
cgit.c | 4 ++
cgit.h | 2 +
cgitrc.5.txt | 4 ++
ui-diff.c | 35 +++++++++++++++
ui-log.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
ui-refs.c | 2 +-
ui-repolist.c | 2 +-
ui-shared.c | 28 ++++++++++--
ui-shared.h | 2 +-
ui-tree.c | 2 +-
10 files changed, 197 insertions(+), 20 deletions(-)
diff --git a/cgit.c b/cgit.c
index 6f44ef2..81312bb 100644
--- a/cgit.c
+++ b/cgit.c
@@ -171,6 +171,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
else if (!strcmp(name, "enable-filter-overrides"))
ctx.cfg.enable_filter_overrides = atoi(value);
+ else if (!strcmp(name, "enable-follow-links"))
+ ctx.cfg.enable_follow_links = atoi(value);
else if (!strcmp(name, "enable-http-clone"))
ctx.cfg.enable_http_clone = atoi(value);
else if (!strcmp(name, "enable-index-links"))
@@ -338,6 +340,8 @@ static void querystring_cb(const char *name, const char *value)
ctx.qry.context = atoi(value);
} else if (!strcmp(name, "ignorews")) {
ctx.qry.ignorews = atoi(value);
+ } else if (!strcmp(name, "follow")) {
+ ctx.qry.follow = atoi(value);
}
}
diff --git a/cgit.h b/cgit.h
index b56e756..c0758b0 100644
--- a/cgit.h
+++ b/cgit.h
@@ -163,6 +163,7 @@ struct cgit_query {
int show_all;
int context;
int ignorews;
+ int follow;
char *vpath;
};
@@ -203,6 +204,7 @@ struct cgit_config {
int case_sensitive_sort;
int embedded;
int enable_filter_overrides;
+ int enable_follow_links;
int enable_http_clone;
int enable_index_links;
int enable_index_owner;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 39b031e..48ef249 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -121,6 +121,10 @@ enable-filter-overrides::
Flag which, when set to "1", allows all filter settings to be
overridden in repository-specific cgitrc files. Default value: none.
+enable-follow-links::
+ Flag which, when set to "1", allows users to follow a file in the log
+ view. Default value: "0".
+
enable-http-clone::
If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
If you use an alternate way of serving git repositories, you may wish
diff --git a/ui-diff.c b/ui-diff.c
index 8b38209..2d75f34 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -36,6 +36,7 @@ static struct fileinfo {
static int use_ssdiff = 0;
static struct diff_filepair *current_filepair;
+static const char *current_prefix;
struct diff_filespec *cgit_get_current_old_file(void)
{
@@ -132,11 +133,30 @@ static void count_diff_lines(char *line, int len)
}
}
+static int show_filepair(struct diff_filepair *pair)
+{
+ /* Always show if we have no limiting prefix. */
+ if (!current_prefix)
+ return 1;
+
+ /* Show if either path in the pair begins with the prefix. */
+ if (!prefixcmp(pair->one->path, current_prefix) ||
+ !prefixcmp(pair->two->path, current_prefix))
+ return 1;
+
+ /* Otherwise we don't want to show this filepair. */
+ return 0;
+}
+
static void inspect_filepair(struct diff_filepair *pair)
{
int binary = 0;
unsigned long old_size = 0;
unsigned long new_size = 0;
+
+ if (!show_filepair(pair))
+ return;
+
files++;
lines_added = 0;
lines_removed = 0;
@@ -279,6 +299,9 @@ static void filepair_cb(struct diff_filepair *pair)
int binary = 0;
linediff_fn print_line_fn = print_line;
+ if (!show_filepair(pair))
+ return;
+
current_filepair = pair;
if (use_ssdiff) {
cgit_ssdiff_header_begin();
@@ -364,6 +387,18 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
unsigned long size;
struct commit *commit, *commit2;
+ /*
+ * If "follow" is set then the diff machinery needs to examine the
+ * entire commit to detect renames so we must limit the paths in our
+ * own callbacks and not pass the prefix to the diff machinery.
+ */
+ if (ctx.qry.follow && ctx.cfg.enable_follow_links) {
+ current_prefix = prefix;
+ prefix = "";
+ } else {
+ current_prefix = NULL;
+ }
+
if (!new_rev)
new_rev = ctx.qry.head;
get_sha1(new_rev, new_rev_sha1);
diff --git a/ui-log.c b/ui-log.c
index 2aa12c3..6bab166 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -12,7 +12,7 @@
#include "ui-shared.h"
#include "vector.h"
-int files, add_lines, rem_lines;
+int files, add_lines, rem_lines, lines_counted;
/*
* The list of available column colors in the commit graph.
@@ -66,7 +66,7 @@ void show_commit_decorations(struct commit *commit)
strncpy(buf, deco->name + 11, sizeof(buf) - 1);
cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else if (!prefixcmp(deco->name, "tag: refs/tags/")) {
strncpy(buf, deco->name + 15, sizeof(buf) - 1);
@@ -83,7 +83,7 @@ void show_commit_decorations(struct commit *commit)
cgit_log_link(buf, NULL, "remote-deco", NULL,
sha1_to_hex(commit->object.sha1),
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else {
strncpy(buf, deco->name, sizeof(buf) - 1);
@@ -96,6 +96,74 @@ next:
}
}
+static void handle_rename(struct diff_filepair *pair)
+{
+ /*
+ * After we have seen a rename, we generate links to the previous
+ * name of the file so that commit & diff views get fed the path
+ * that is correct for the commit they are showing, avoiding the
+ * need to walk the entire history leading back to every commit we
+ * show in order detect renames.
+ */
+ if (0 != strcmp(ctx.qry.vpath, pair->two->path)) {
+ free(ctx.qry.vpath);
+ ctx.qry.vpath = xstrdup(pair->two->path);
+ }
+ inspect_files(pair);
+}
+
+static int show_commit(struct commit *commit, struct rev_info *revs)
+{
+ struct commit_list *parents = commit->parents;
+ struct commit *parent;
+ int found = 0, saved_fmt;
+ unsigned saved_flags = revs->diffopt.flags;
+
+
+ /* Always show if we're not in "follow" mode with a single file. */
+ if (!ctx.qry.follow)
+ return 1;
+
+ /*
+ * In "follow" mode, we don't show merges. This is consistent with
+ * "git log --follow -- <file>".
+ */
+ if (parents && parents->next)
+ return 0;
+
+ /*
+ * If this is the root commit, do what rev_info tells us.
+ */
+ if (!parents)
+ return revs->show_root_diff;
+
+ /* When we get here we have precisely one parent. */
+ parent = parents->item;
+ parse_commit(parent);
+
+ files = 0;
+ add_lines = 0;
+ rem_lines = 0;
+
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ diff_tree_sha1(parent->tree->object.sha1,
+ commit->tree->object.sha1,
+ "", &revs->diffopt);
+ diffcore_std(&revs->diffopt);
+
+ found = !diff_queue_is_empty();
+ saved_fmt = revs->diffopt.output_format;
+ revs->diffopt.output_format = DIFF_FORMAT_CALLBACK;
+ revs->diffopt.format_callback = cgit_diff_tree_cb;
+ revs->diffopt.format_callback_data = handle_rename;
+ diff_flush(&revs->diffopt);
+ revs->diffopt.output_format = saved_fmt;
+ revs->diffopt.flags = saved_flags;
+
+ lines_counted = 1;
+ return found;
+}
+
static void print_commit(struct commit *commit, struct rev_info *revs)
{
struct commitinfo *info;
@@ -173,7 +241,8 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
cgit_print_age(commit->date, TM_WEEK * 2, FMT_SHORTDATE);
}
- if (ctx.repo->enable_log_filecount || ctx.repo->enable_log_linecount) {
+ if (!lines_counted && (ctx.repo->enable_log_filecount ||
+ ctx.repo->enable_log_linecount)) {
files = 0;
add_lines = 0;
rem_lines = 0;
@@ -324,7 +393,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
}
}
- if (commit_graph) {
+
+ if (!path || !ctx.cfg.enable_follow_links) {
+ /*
+ * If we don't have a path, "follow" is a no-op so make sure
+ * the variable is set to false to avoid needing to check
+ * both this and whether we have a path everywhere.
+ */
+ ctx.qry.follow = 0;
+ }
+
+ if (commit_graph && !ctx.qry.follow) {
static const char *graph_arg = "--graph";
static const char *color_arg = "--color";
vector_push(&vec, &graph_arg, 0);
@@ -342,7 +421,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
if (path) {
+ static const char *follow_arg = "--follow";
static const char *double_dash_arg = "--";
+ if (ctx.qry.follow)
+ vector_push(&vec, &follow_arg, 0);
vector_push(&vec, &double_dash_arg, 0);
vector_push(&vec, &path, 0);
}
@@ -356,10 +438,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
rev.commit_format = CMIT_FMT_DEFAULT;
rev.verbose_header = 1;
rev.show_root_diff = 0;
+ rev.simplify_history = 1;
setup_revisions(vec.count, vec.data, &rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
rev.grep_filter.regflags |= REG_ICASE;
+
+ rev.diffopt.detect_rename = 1;
+ rev.diffopt.rename_limit = ctx.cfg.renamelimit;
+ if (ctx.qry.ignorews)
+ DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
+
compile_grep_patterns(&rev.grep_filter);
prepare_revision_walk(&rev);
@@ -377,11 +466,12 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
NULL, ctx.qry.head, ctx.qry.sha1,
ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
+ ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
+ ctx.qry.follow);
html(")");
}
html("</th><th class='left'>Author</th>");
- if (commit_graph)
+ if (rev.graph)
html("<th class='left'>Age</th>");
if (ctx.repo->enable_log_filecount) {
html("<th class='left'>Files</th>");
@@ -396,15 +486,32 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
if (ofs<0)
ofs = 0;
- for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
+ for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL;) {
+ if (show_commit(commit, &rev))
+ i++;
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
commit->parents = NULL;
}
- for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
- print_commit(commit, &rev);
+ for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL;) {
+ /*
+ * In "follow" mode, we must count the files and lines the
+ * first time we invoke diff on a given commit, and we need
+ * to do that to see if the commit touches the path we care
+ * about, so we do it in show_commit. Hence we must clear
+ * lines_counted here.
+ *
+ * This has the side effect of avoiding running diff twice
+ * when we are both following renames and showing file
+ * and/or line counts.
+ */
+ lines_counted = 0;
+ if (show_commit(commit, &rev)) {
+ i++;
+ print_commit(commit, &rev);
+ }
free(commit->buffer);
commit->buffer = NULL;
free_commit_list(commit->parents);
@@ -417,7 +524,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs - cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
if ((commit = get_revision(&rev)) != NULL) {
@@ -425,14 +533,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs + cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
html("</ul>");
} else if ((commit = get_revision(&rev)) != NULL) {
htmlf("<tr class='nohover'><td colspan='%d'>", columns);
cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
- ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
+ ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</td></tr>\n");
}
diff --git a/ui-refs.c b/ui-refs.c
index 0ae0612..f4eefd1 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -71,7 +71,7 @@ static int print_branch(struct refinfo *ref)
return 1;
html("<tr><td>");
cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
html("</td><td>");
if (ref->object->type == OBJ_COMMIT) {
diff --git a/ui-repolist.c b/ui-repolist.c
index 47ca997..d96f252 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -314,7 +314,7 @@ void cgit_print_repolist()
html("<td>");
cgit_summary_link("summary", NULL, "button", NULL);
cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
- 0, NULL, NULL, ctx.qry.showmsg);
+ 0, NULL, NULL, ctx.qry.showmsg, 0);
cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
html("</td>");
}
diff --git a/ui-shared.c b/ui-shared.c
index 519eef7..04d0368 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -284,7 +284,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
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, const char *pattern, int showmsg)
+ int ofs, const char *grep, const char *pattern, int showmsg,
+ int follow)
{
char *delim;
@@ -313,6 +314,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
if (showmsg) {
html(delim);
html("showmsg=1");
+ delim = "&";
+ }
+ if (follow) {
+ html(delim);
+ html("follow=1");
}
html("'>");
html_txt(name);
@@ -355,6 +361,10 @@ void cgit_commit_link(char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
if (name[0] != '\0')
html_txt(name);
@@ -411,6 +421,10 @@ void cgit_diff_link(const char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
html_txt(name);
html("</a>");
@@ -452,7 +466,7 @@ static void cgit_self_link(char *name, const char *title, const char *class,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
ctx->qry.path, ctx->qry.ofs,
ctx->qry.grep, ctx->qry.search,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
else if (!strcmp(ctx->qry.page, "commit"))
cgit_commit_link(name, title, class, ctx->qry.head,
ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
@@ -854,7 +868,7 @@ void cgit_print_pageheader(struct cgit_context *ctx)
ctx->qry.sha1, NULL);
cgit_log_link("log", NULL, hc(ctx, "log"), ctx->qry.head,
NULL, ctx->qry.vpath, 0, NULL, NULL,
- ctx->qry.showmsg);
+ ctx->qry.showmsg, ctx->qry.follow);
cgit_tree_link("tree", NULL, hc(ctx, "tree"), ctx->qry.head,
ctx->qry.sha1, ctx->qry.vpath);
cgit_commit_link("commit", NULL, hc(ctx, "commit"),
@@ -906,6 +920,14 @@ void cgit_print_pageheader(struct cgit_context *ctx)
html("<div class='path'>");
html("path: ");
cgit_print_path_crumbs(ctx, ctx->qry.vpath);
+ if (ctx->cfg.enable_follow_links && !strcmp(ctx->qry.page, "log")) {
+ html(" (");
+ ctx->qry.follow = !ctx->qry.follow;
+ cgit_self_link(ctx->qry.follow ? "follow" : "unfollow",
+ NULL, NULL, ctx);
+ ctx->qry.follow = !ctx->qry.follow;
+ html(")");
+ }
html("</div>");
}
html("<div class='content'>");
diff --git a/ui-shared.h b/ui-shared.h
index 5987e77..3156846 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -26,7 +26,7 @@ extern void cgit_plain_link(const char *name, const char *title,
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,
- const char *pattern, int showmsg);
+ const char *pattern, int showmsg, int follow);
extern void cgit_commit_link(char *name, const char *title,
const char *class, const char *head,
const char *rev, const char *path,
diff --git a/ui-tree.c b/ui-tree.c
index aa5dee9..ebb3e9b 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -169,7 +169,7 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen,
html("<td>");
cgit_log_link("log", NULL, "button", ctx.qry.head,
walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
if (ctx.repo->max_stats)
cgit_stats_link("stats", NULL, "button", ctx.qry.head,
fullpath.buf);
--
1.8.2.1.715.gb260f47
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] Allow users to follow file renames
2013-04-20 10:14 ` [PATCH v2 0/2] Allow users to follow file renames john
2013-04-20 10:14 ` [PATCH v2 1/2] shared: make cgit_diff_tree_cb public john
2013-04-20 10:14 ` [PATCH v2 2/2] log: allow users to follow a file john
@ 2013-04-26 13:52 ` Jason
2 siblings, 0 replies; 18+ messages in thread
From: Jason @ 2013-04-26 13:52 UTC (permalink / raw)
Thanks John. Merged to jk/follow-renames, and I'll be doing a review of it soon.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2 v3] log: allow users to follow a file
2013-04-20 12:21 ` [PATCH 2/2 v3] " john
@ 2015-03-24 6:02 `
2015-08-12 14:34 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public john
1 sibling, 0 replies; 18+ messages in thread
From: @ 2015-03-24 6:02 UTC (permalink / raw)
Hi all,
when cleaning up my cgit installation (and upgrading to a recent
version), I noted that I had the following patch applied. A quick search
revealed, that it has not found its way into cgit. Was this an
oversight, or was it on purpose?
- Ren?
Am 20.04.2013 um 14:21 schrieb John Keeping:
> Teach the "log" UI to behave in the same way as "git log --follow", when
> given a suitable instruction by the user. The default behaviour remains
> to show the log without following renames, but the follow behaviour can
> be activated by following a link in the page header.
>
> Follow is not the default because outputting merges in follow mode is
> tricky ("git log --follow" will not show merges). We also disable the
> graph in follow mode because the commit graph is not simplified so we
> end up with frequent gaps in the graph and many lines that do not
> connect with any commits we're actually showing.
>
> We also teach the "diff" and "commit" UIs to respect the follow flag on
> URLs, causing the single-file version of these UIs to detect renames.
> This feature is needed only for commits that rename the path we're
> interested in.
>
> For commits before the file has been renamed (i.e. that appear later in
> the log list) we change the file path in the links from the log to point
> to the old name; this means that links to commits always limit by the
> path known to that commit. If we didn't do this we would need to walk
> down the log diff'ing every commit whenever we want to show a commit.
> The drawback is that the "Log" link in the top bar of such a page links
> to the log limited by the old name, so it will only show pre-rename
> commits. I consider this a reasonable trade-off since the "Back" button
> still works and the log matches the path displayed in the top bar.
>
> Since following renames requires running diff on every commit we
> consider, I've added a knob to the configuration file to globally
> enable/disable this feature. Note that we may consider a large number
> of commits the revision walking machinery no longer performs any path
> limitation so we have to examine every commit until we find a page full
> of commits that affect the target path or something related to it.
>
> Suggested-by: Ren? Neumann <necoro at necoro.eu>
> Signed-off-by: John Keeping <john at keeping.me.uk>
> ---
> I've just realised I didn't commit a final fixup to this before sending
> it out. The difference is in handle_rename() which doesn't leak memory
> in this version.
>
> cgit.c | 4 ++
> cgit.h | 2 +
> cgitrc.5.txt | 4 ++
> ui-diff.c | 35 +++++++++++++++
> ui-log.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> ui-refs.c | 2 +-
> ui-repolist.c | 2 +-
> ui-shared.c | 28 ++++++++++--
> ui-shared.h | 2 +-
> ui-tree.c | 2 +-
> 10 files changed, 197 insertions(+), 20 deletions(-)
>
> diff --git a/cgit.c b/cgit.c
> index 6f44ef2..81312bb 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -171,6 +171,8 @@ static void config_cb(const char *name, const char *value)
> ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
> else if (!strcmp(name, "enable-filter-overrides"))
> ctx.cfg.enable_filter_overrides = atoi(value);
> + else if (!strcmp(name, "enable-follow-links"))
> + ctx.cfg.enable_follow_links = atoi(value);
> else if (!strcmp(name, "enable-http-clone"))
> ctx.cfg.enable_http_clone = atoi(value);
> else if (!strcmp(name, "enable-index-links"))
> @@ -338,6 +340,8 @@ static void querystring_cb(const char *name, const char *value)
> ctx.qry.context = atoi(value);
> } else if (!strcmp(name, "ignorews")) {
> ctx.qry.ignorews = atoi(value);
> + } else if (!strcmp(name, "follow")) {
> + ctx.qry.follow = atoi(value);
> }
> }
>
> diff --git a/cgit.h b/cgit.h
> index b56e756..c0758b0 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -163,6 +163,7 @@ struct cgit_query {
> int show_all;
> int context;
> int ignorews;
> + int follow;
> char *vpath;
> };
>
> @@ -203,6 +204,7 @@ struct cgit_config {
> int case_sensitive_sort;
> int embedded;
> int enable_filter_overrides;
> + int enable_follow_links;
> int enable_http_clone;
> int enable_index_links;
> int enable_index_owner;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 39b031e..48ef249 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -121,6 +121,10 @@ enable-filter-overrides::
> Flag which, when set to "1", allows all filter settings to be
> overridden in repository-specific cgitrc files. Default value: none.
>
> +enable-follow-links::
> + Flag which, when set to "1", allows users to follow a file in the log
> + view. Default value: "0".
> +
> enable-http-clone::
> If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
> If you use an alternate way of serving git repositories, you may wish
> diff --git a/ui-diff.c b/ui-diff.c
> index 8b38209..2d75f34 100644
> --- a/ui-diff.c
> +++ b/ui-diff.c
> @@ -36,6 +36,7 @@ static struct fileinfo {
>
> static int use_ssdiff = 0;
> static struct diff_filepair *current_filepair;
> +static const char *current_prefix;
>
> struct diff_filespec *cgit_get_current_old_file(void)
> {
> @@ -132,11 +133,30 @@ static void count_diff_lines(char *line, int len)
> }
> }
>
> +static int show_filepair(struct diff_filepair *pair)
> +{
> + /* Always show if we have no limiting prefix. */
> + if (!current_prefix)
> + return 1;
> +
> + /* Show if either path in the pair begins with the prefix. */
> + if (!prefixcmp(pair->one->path, current_prefix) ||
> + !prefixcmp(pair->two->path, current_prefix))
> + return 1;
> +
> + /* Otherwise we don't want to show this filepair. */
> + return 0;
> +}
> +
> static void inspect_filepair(struct diff_filepair *pair)
> {
> int binary = 0;
> unsigned long old_size = 0;
> unsigned long new_size = 0;
> +
> + if (!show_filepair(pair))
> + return;
> +
> files++;
> lines_added = 0;
> lines_removed = 0;
> @@ -279,6 +299,9 @@ static void filepair_cb(struct diff_filepair *pair)
> int binary = 0;
> linediff_fn print_line_fn = print_line;
>
> + if (!show_filepair(pair))
> + return;
> +
> current_filepair = pair;
> if (use_ssdiff) {
> cgit_ssdiff_header_begin();
> @@ -364,6 +387,18 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
> unsigned long size;
> struct commit *commit, *commit2;
>
> + /*
> + * If "follow" is set then the diff machinery needs to examine the
> + * entire commit to detect renames so we must limit the paths in our
> + * own callbacks and not pass the prefix to the diff machinery.
> + */
> + if (ctx.qry.follow && ctx.cfg.enable_follow_links) {
> + current_prefix = prefix;
> + prefix = "";
> + } else {
> + current_prefix = NULL;
> + }
> +
> if (!new_rev)
> new_rev = ctx.qry.head;
> get_sha1(new_rev, new_rev_sha1);
> diff --git a/ui-log.c b/ui-log.c
> index 2aa12c3..6bab166 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -12,7 +12,7 @@
> #include "ui-shared.h"
> #include "vector.h"
>
> -int files, add_lines, rem_lines;
> +int files, add_lines, rem_lines, lines_counted;
>
> /*
> * The list of available column colors in the commit graph.
> @@ -66,7 +66,7 @@ void show_commit_decorations(struct commit *commit)
> strncpy(buf, deco->name + 11, sizeof(buf) - 1);
> cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
> ctx.qry.vpath, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> }
> else if (!prefixcmp(deco->name, "tag: refs/tags/")) {
> strncpy(buf, deco->name + 15, sizeof(buf) - 1);
> @@ -83,7 +83,7 @@ void show_commit_decorations(struct commit *commit)
> cgit_log_link(buf, NULL, "remote-deco", NULL,
> sha1_to_hex(commit->object.sha1),
> ctx.qry.vpath, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> }
> else {
> strncpy(buf, deco->name, sizeof(buf) - 1);
> @@ -96,6 +96,74 @@ next:
> }
> }
>
> +static void handle_rename(struct diff_filepair *pair)
> +{
> + /*
> + * After we have seen a rename, we generate links to the previous
> + * name of the file so that commit & diff views get fed the path
> + * that is correct for the commit they are showing, avoiding the
> + * need to walk the entire history leading back to every commit we
> + * show in order detect renames.
> + */
> + if (0 != strcmp(ctx.qry.vpath, pair->two->path)) {
> + free(ctx.qry.vpath);
> + ctx.qry.vpath = xstrdup(pair->two->path);
> + }
> + inspect_files(pair);
> +}
> +
> +static int show_commit(struct commit *commit, struct rev_info *revs)
> +{
> + struct commit_list *parents = commit->parents;
> + struct commit *parent;
> + int found = 0, saved_fmt;
> + unsigned saved_flags = revs->diffopt.flags;
> +
> +
> + /* Always show if we're not in "follow" mode with a single file. */
> + if (!ctx.qry.follow)
> + return 1;
> +
> + /*
> + * In "follow" mode, we don't show merges. This is consistent with
> + * "git log --follow -- <file>".
> + */
> + if (parents && parents->next)
> + return 0;
> +
> + /*
> + * If this is the root commit, do what rev_info tells us.
> + */
> + if (!parents)
> + return revs->show_root_diff;
> +
> + /* When we get here we have precisely one parent. */
> + parent = parents->item;
> + parse_commit(parent);
> +
> + files = 0;
> + add_lines = 0;
> + rem_lines = 0;
> +
> + DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
> + diff_tree_sha1(parent->tree->object.sha1,
> + commit->tree->object.sha1,
> + "", &revs->diffopt);
> + diffcore_std(&revs->diffopt);
> +
> + found = !diff_queue_is_empty();
> + saved_fmt = revs->diffopt.output_format;
> + revs->diffopt.output_format = DIFF_FORMAT_CALLBACK;
> + revs->diffopt.format_callback = cgit_diff_tree_cb;
> + revs->diffopt.format_callback_data = handle_rename;
> + diff_flush(&revs->diffopt);
> + revs->diffopt.output_format = saved_fmt;
> + revs->diffopt.flags = saved_flags;
> +
> + lines_counted = 1;
> + return found;
> +}
> +
> static void print_commit(struct commit *commit, struct rev_info *revs)
> {
> struct commitinfo *info;
> @@ -173,7 +241,8 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
> cgit_print_age(commit->date, TM_WEEK * 2, FMT_SHORTDATE);
> }
>
> - if (ctx.repo->enable_log_filecount || ctx.repo->enable_log_linecount) {
> + if (!lines_counted && (ctx.repo->enable_log_filecount ||
> + ctx.repo->enable_log_linecount)) {
> files = 0;
> add_lines = 0;
> rem_lines = 0;
> @@ -324,7 +393,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> }
> }
> }
> - if (commit_graph) {
> +
> + if (!path || !ctx.cfg.enable_follow_links) {
> + /*
> + * If we don't have a path, "follow" is a no-op so make sure
> + * the variable is set to false to avoid needing to check
> + * both this and whether we have a path everywhere.
> + */
> + ctx.qry.follow = 0;
> + }
> +
> + if (commit_graph && !ctx.qry.follow) {
> static const char *graph_arg = "--graph";
> static const char *color_arg = "--color";
> vector_push(&vec, &graph_arg, 0);
> @@ -342,7 +421,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> }
>
> if (path) {
> + static const char *follow_arg = "--follow";
> static const char *double_dash_arg = "--";
> + if (ctx.qry.follow)
> + vector_push(&vec, &follow_arg, 0);
> vector_push(&vec, &double_dash_arg, 0);
> vector_push(&vec, &path, 0);
> }
> @@ -356,10 +438,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> rev.commit_format = CMIT_FMT_DEFAULT;
> rev.verbose_header = 1;
> rev.show_root_diff = 0;
> + rev.simplify_history = 1;
> setup_revisions(vec.count, vec.data, &rev, NULL);
> load_ref_decorations(DECORATE_FULL_REFS);
> rev.show_decorations = 1;
> rev.grep_filter.regflags |= REG_ICASE;
> +
> + rev.diffopt.detect_rename = 1;
> + rev.diffopt.rename_limit = ctx.cfg.renamelimit;
> + if (ctx.qry.ignorews)
> + DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
> +
> compile_grep_patterns(&rev.grep_filter);
> prepare_revision_walk(&rev);
>
> @@ -377,11 +466,12 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
> NULL, ctx.qry.head, ctx.qry.sha1,
> ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
> + ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
> + ctx.qry.follow);
> html(")");
> }
> html("</th><th class='left'>Author</th>");
> - if (commit_graph)
> + if (rev.graph)
> html("<th class='left'>Age</th>");
> if (ctx.repo->enable_log_filecount) {
> html("<th class='left'>Files</th>");
> @@ -396,15 +486,32 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> if (ofs<0)
> ofs = 0;
>
> - for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
> + for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL;) {
> + if (show_commit(commit, &rev))
> + i++;
> free(commit->buffer);
> commit->buffer = NULL;
> free_commit_list(commit->parents);
> commit->parents = NULL;
> }
>
> - for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
> - print_commit(commit, &rev);
> + for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL;) {
> + /*
> + * In "follow" mode, we must count the files and lines the
> + * first time we invoke diff on a given commit, and we need
> + * to do that to see if the commit touches the path we care
> + * about, so we do it in show_commit. Hence we must clear
> + * lines_counted here.
> + *
> + * This has the side effect of avoiding running diff twice
> + * when we are both following renames and showing file
> + * and/or line counts.
> + */
> + lines_counted = 0;
> + if (show_commit(commit, &rev)) {
> + i++;
> + print_commit(commit, &rev);
> + }
> free(commit->buffer);
> commit->buffer = NULL;
> free_commit_list(commit->parents);
> @@ -417,7 +524,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
> ctx.qry.sha1, ctx.qry.vpath,
> ofs - cnt, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg);
> + ctx.qry.search, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</li>");
> }
> if ((commit = get_revision(&rev)) != NULL) {
> @@ -425,14 +533,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
> cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
> ctx.qry.sha1, ctx.qry.vpath,
> ofs + cnt, ctx.qry.grep,
> - ctx.qry.search, ctx.qry.showmsg);
> + ctx.qry.search, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</li>");
> }
> html("</ul>");
> } else if ((commit = get_revision(&rev)) != NULL) {
> htmlf("<tr class='nohover'><td colspan='%d'>", columns);
> cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
> - ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
> + ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
> + ctx.qry.follow);
> html("</td></tr>\n");
> }
>
> diff --git a/ui-refs.c b/ui-refs.c
> index 0ae0612..f4eefd1 100644
> --- a/ui-refs.c
> +++ b/ui-refs.c
> @@ -71,7 +71,7 @@ static int print_branch(struct refinfo *ref)
> return 1;
> html("<tr><td>");
> cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> html("</td><td>");
>
> if (ref->object->type == OBJ_COMMIT) {
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 47ca997..d96f252 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -314,7 +314,7 @@ void cgit_print_repolist()
> html("<td>");
> cgit_summary_link("summary", NULL, "button", NULL);
> cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
> - 0, NULL, NULL, ctx.qry.showmsg);
> + 0, NULL, NULL, ctx.qry.showmsg, 0);
> cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
> html("</td>");
> }
> diff --git a/ui-shared.c b/ui-shared.c
> index 519eef7..04d0368 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -284,7 +284,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
>
> 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, const char *pattern, int showmsg)
> + int ofs, const char *grep, const char *pattern, int showmsg,
> + int follow)
> {
> char *delim;
>
> @@ -313,6 +314,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
> if (showmsg) {
> html(delim);
> html("showmsg=1");
> + delim = "&";
> + }
> + if (follow) {
> + html(delim);
> + html("follow=1");
> }
> html("'>");
> html_txt(name);
> @@ -355,6 +361,10 @@ void cgit_commit_link(char *name, const char *title, const char *class,
> html("ignorews=1");
> delim = "&";
> }
> + if (ctx.qry.follow) {
> + html(delim);
> + html("follow=1");
> + }
> html("'>");
> if (name[0] != '\0')
> html_txt(name);
> @@ -411,6 +421,10 @@ void cgit_diff_link(const char *name, const char *title, const char *class,
> html("ignorews=1");
> delim = "&";
> }
> + if (ctx.qry.follow) {
> + html(delim);
> + html("follow=1");
> + }
> html("'>");
> html_txt(name);
> html("</a>");
> @@ -452,7 +466,7 @@ static void cgit_self_link(char *name, const char *title, const char *class,
> ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
> ctx->qry.path, ctx->qry.ofs,
> ctx->qry.grep, ctx->qry.search,
> - ctx->qry.showmsg);
> + ctx->qry.showmsg, ctx->qry.follow);
> else if (!strcmp(ctx->qry.page, "commit"))
> cgit_commit_link(name, title, class, ctx->qry.head,
> ctx->qry.has_sha1 ? ctx->qry.sha1 : NULL,
> @@ -854,7 +868,7 @@ void cgit_print_pageheader(struct cgit_context *ctx)
> ctx->qry.sha1, NULL);
> cgit_log_link("log", NULL, hc(ctx, "log"), ctx->qry.head,
> NULL, ctx->qry.vpath, 0, NULL, NULL,
> - ctx->qry.showmsg);
> + ctx->qry.showmsg, ctx->qry.follow);
> cgit_tree_link("tree", NULL, hc(ctx, "tree"), ctx->qry.head,
> ctx->qry.sha1, ctx->qry.vpath);
> cgit_commit_link("commit", NULL, hc(ctx, "commit"),
> @@ -906,6 +920,14 @@ void cgit_print_pageheader(struct cgit_context *ctx)
> html("<div class='path'>");
> html("path: ");
> cgit_print_path_crumbs(ctx, ctx->qry.vpath);
> + if (ctx->cfg.enable_follow_links && !strcmp(ctx->qry.page, "log")) {
> + html(" (");
> + ctx->qry.follow = !ctx->qry.follow;
> + cgit_self_link(ctx->qry.follow ? "follow" : "unfollow",
> + NULL, NULL, ctx);
> + ctx->qry.follow = !ctx->qry.follow;
> + html(")");
> + }
> html("</div>");
> }
> html("<div class='content'>");
> diff --git a/ui-shared.h b/ui-shared.h
> index 5987e77..3156846 100644
> --- a/ui-shared.h
> +++ b/ui-shared.h
> @@ -26,7 +26,7 @@ extern void cgit_plain_link(const char *name, const char *title,
> 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,
> - const char *pattern, int showmsg);
> + const char *pattern, int showmsg, int follow);
> extern void cgit_commit_link(char *name, const char *title,
> const char *class, const char *head,
> const char *rev, const char *path,
> diff --git a/ui-tree.c b/ui-tree.c
> index aa5dee9..ebb3e9b 100644
> --- a/ui-tree.c
> +++ b/ui-tree.c
> @@ -169,7 +169,7 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen,
> html("<td>");
> cgit_log_link("log", NULL, "button", ctx.qry.head,
> walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
> - ctx.qry.showmsg);
> + ctx.qry.showmsg, 0);
> if (ctx.repo->max_stats)
> cgit_stats_link("stats", NULL, "button", ctx.qry.head,
> fullpath.buf);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 1/2] shared: make cgit_diff_tree_cb public
2013-04-20 12:21 ` [PATCH 2/2 v3] " john
2015-03-24 6:02 `
@ 2015-08-12 14:34 ` john
2015-08-12 14:55 ` [PATCH v4 2/2] log: allow users to follow a file john
2015-08-12 15:41 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public Jason
1 sibling, 2 replies; 18+ messages in thread
From: john @ 2015-08-12 14:34 UTC (permalink / raw)
This will allow us to use this nice wrapper function elsewhere, avoiding
dealing with the diff queue when we only need to inspect a filepair.
Signed-off-by: John Keeping <john at keeping.me.uk>
---
Unchanged since v2/3.
cgit.h | 3 +++
shared.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/cgit.h b/cgit.h
index db9a8eb..b2253d2 100644
--- a/cgit.h
+++ b/cgit.h
@@ -340,6 +340,9 @@ extern int cgit_refs_cb(const char *refname, const struct object_id *oid,
extern void *cgit_free_commitinfo(struct commitinfo *info);
+void cgit_diff_tree_cb(struct diff_queue_struct *q,
+ struct diff_options *options, void *data);
+
extern int cgit_diff_files(const unsigned char *old_sha1,
const unsigned char *new_sha1,
unsigned long *old_size, unsigned long *new_size,
diff --git a/shared.c b/shared.c
index a83afcb..0431b59 100644
--- a/shared.c
+++ b/shared.c
@@ -250,8 +250,8 @@ int cgit_refs_cb(const char *refname, const struct object_id *oid, int flags,
return 0;
}
-static void cgit_diff_tree_cb(struct diff_queue_struct *q,
- struct diff_options *options, void *data)
+void cgit_diff_tree_cb(struct diff_queue_struct *q,
+ struct diff_options *options, void *data)
{
int i;
--
2.5.0.466.g9af26fa
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 2/2] log: allow users to follow a file
2015-08-12 14:34 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public john
@ 2015-08-12 14:55 ` john
2015-08-12 15:01 ` Jason
2015-08-12 15:41 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public Jason
1 sibling, 1 reply; 18+ messages in thread
From: john @ 2015-08-12 14:55 UTC (permalink / raw)
Teach the "log" UI to behave in the same way as "git log --follow", when
given a suitable instruction by the user. The default behaviour remains
to show the log without following renames, but the follow behaviour can
be activated by following a link in the page header.
Follow is not the default because outputting merges in follow mode is
tricky ("git log --follow" will not show merges). We also disable the
graph in follow mode because the commit graph is not simplified so we
end up with frequent gaps in the graph and many lines that do not
connect with any commits we're actually showing.
We also teach the "diff" and "commit" UIs to respect the follow flag on
URLs, causing the single-file version of these UIs to detect renames.
This feature is needed only for commits that rename the path we're
interested in.
For commits before the file has been renamed (i.e. that appear later in
the log list) we change the file path in the links from the log to point
to the old name; this means that links to commits always limit by the
path known to that commit. If we didn't do this we would need to walk
down the log diff'ing every commit whenever we want to show a commit.
The drawback is that the "Log" link in the top bar of such a page links
to the log limited by the old name, so it will only show pre-rename
commits. I consider this a reasonable trade-off since the "Back" button
still works and the log matches the path displayed in the top bar.
Since following renames requires running diff on every commit we
consider, I've added a knob to the configuration file to globally
enable/disable this feature. Note that we may consider a large number
of commits the revision walking machinery no longer performs any path
limitation so we have to examine every commit until we find a page full
of commits that affect the target path or something related to it.
Suggested-by: Ren? Neumann <necoro at necoro.eu>
Signed-off-by: John Keeping <john at keeping.me.uk>
---
Rebased onto updated master.
cgit.c | 4 ++
cgit.h | 2 +
cgitrc.5.txt | 4 ++
ui-diff.c | 35 ++++++++++++++++
ui-log.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
ui-refs.c | 2 +-
ui-repolist.c | 2 +-
ui-shared.c | 28 +++++++++++--
ui-shared.h | 2 +-
ui-tree.c | 2 +-
10 files changed, 194 insertions(+), 18 deletions(-)
diff --git a/cgit.c b/cgit.c
index baad1c8..d84b4be 100644
--- a/cgit.c
+++ b/cgit.c
@@ -152,6 +152,8 @@ static void config_cb(const char *name, const char *value)
ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
else if (!strcmp(name, "enable-filter-overrides"))
ctx.cfg.enable_filter_overrides = atoi(value);
+ else if (!strcmp(name, "enable-follow-links"))
+ ctx.cfg.enable_follow_links = atoi(value);
else if (!strcmp(name, "enable-http-clone"))
ctx.cfg.enable_http_clone = atoi(value);
else if (!strcmp(name, "enable-index-links"))
@@ -333,6 +335,8 @@ static void querystring_cb(const char *name, const char *value)
ctx.qry.context = atoi(value);
} else if (!strcmp(name, "ignorews")) {
ctx.qry.ignorews = atoi(value);
+ } else if (!strcmp(name, "follow")) {
+ ctx.qry.follow = atoi(value);
}
}
diff --git a/cgit.h b/cgit.h
index b2253d2..3120562 100644
--- a/cgit.h
+++ b/cgit.h
@@ -179,6 +179,7 @@ struct cgit_query {
int show_all;
int context;
int ignorews;
+ int follow;
char *vpath;
};
@@ -221,6 +222,7 @@ struct cgit_config {
int case_sensitive_sort;
int embedded;
int enable_filter_overrides;
+ int enable_follow_links;
int enable_http_clone;
int enable_index_links;
int enable_index_owner;
diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index e21ece9..759f353 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -150,6 +150,10 @@ enable-filter-overrides::
Flag which, when set to "1", allows all filter settings to be
overridden in repository-specific cgitrc files. Default value: none.
+enable-follow-links::
+ Flag which, when set to "1", allows users to follow a file in the log
+ view. Default value: "0".
+
enable-http-clone::
If set to "1", cgit will act as an dumb HTTP endpoint for git clones.
You can add "http://$HTTP_HOST$SCRIPT_NAME/$CGIT_REPO_URL" to clone-url
diff --git a/ui-diff.c b/ui-diff.c
index 1cf2ce0..caebd5d 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -36,6 +36,7 @@ static struct fileinfo {
static int use_ssdiff = 0;
static struct diff_filepair *current_filepair;
+static const char *current_prefix;
struct diff_filespec *cgit_get_current_old_file(void)
{
@@ -132,11 +133,30 @@ static void count_diff_lines(char *line, int len)
}
}
+static int show_filepair(struct diff_filepair *pair)
+{
+ /* Always show if we have no limiting prefix. */
+ if (!current_prefix)
+ return 1;
+
+ /* Show if either path in the pair begins with the prefix. */
+ if (starts_with(pair->one->path, current_prefix) ||
+ starts_with(pair->two->path, current_prefix))
+ return 1;
+
+ /* Otherwise we don't want to show this filepair. */
+ return 0;
+}
+
static void inspect_filepair(struct diff_filepair *pair)
{
int binary = 0;
unsigned long old_size = 0;
unsigned long new_size = 0;
+
+ if (!show_filepair(pair))
+ return;
+
files++;
lines_added = 0;
lines_removed = 0;
@@ -279,6 +299,9 @@ static void filepair_cb(struct diff_filepair *pair)
int binary = 0;
linediff_fn print_line_fn = print_line;
+ if (!show_filepair(pair))
+ return;
+
current_filepair = pair;
if (use_ssdiff) {
cgit_ssdiff_header_begin();
@@ -365,6 +388,18 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
const unsigned char *old_tree_sha1, *new_tree_sha1;
diff_type difftype;
+ /*
+ * If "follow" is set then the diff machinery needs to examine the
+ * entire commit to detect renames so we must limit the paths in our
+ * own callbacks and not pass the prefix to the diff machinery.
+ */
+ if (ctx.qry.follow && ctx.cfg.enable_follow_links) {
+ current_prefix = prefix;
+ prefix = "";
+ } else {
+ current_prefix = NULL;
+ }
+
if (!new_rev)
new_rev = ctx.qry.head;
if (get_sha1(new_rev, new_rev_sha1)) {
diff --git a/ui-log.c b/ui-log.c
index 8028b27..ff832ce 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -12,7 +12,7 @@
#include "ui-shared.h"
#include "argv-array.h"
-static int files, add_lines, rem_lines;
+static int files, add_lines, rem_lines, lines_counted;
/*
* The list of available column colors in the commit graph.
@@ -67,7 +67,7 @@ void show_commit_decorations(struct commit *commit)
strncpy(buf, deco->name + 11, sizeof(buf) - 1);
cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else if (starts_with(deco->name, "tag: refs/tags/")) {
strncpy(buf, deco->name + 15, sizeof(buf) - 1);
@@ -84,7 +84,7 @@ void show_commit_decorations(struct commit *commit)
cgit_log_link(buf, NULL, "remote-deco", NULL,
sha1_to_hex(commit->object.sha1),
ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
}
else {
strncpy(buf, deco->name, sizeof(buf) - 1);
@@ -98,6 +98,74 @@ next:
html("</span>");
}
+static void handle_rename(struct diff_filepair *pair)
+{
+ /*
+ * After we have seen a rename, we generate links to the previous
+ * name of the file so that commit & diff views get fed the path
+ * that is correct for the commit they are showing, avoiding the
+ * need to walk the entire history leading back to every commit we
+ * show in order detect renames.
+ */
+ if (0 != strcmp(ctx.qry.vpath, pair->two->path)) {
+ free(ctx.qry.vpath);
+ ctx.qry.vpath = xstrdup(pair->two->path);
+ }
+ inspect_files(pair);
+}
+
+static int show_commit(struct commit *commit, struct rev_info *revs)
+{
+ struct commit_list *parents = commit->parents;
+ struct commit *parent;
+ int found = 0, saved_fmt;
+ unsigned saved_flags = revs->diffopt.flags;
+
+
+ /* Always show if we're not in "follow" mode with a single file. */
+ if (!ctx.qry.follow)
+ return 1;
+
+ /*
+ * In "follow" mode, we don't show merges. This is consistent with
+ * "git log --follow -- <file>".
+ */
+ if (parents && parents->next)
+ return 0;
+
+ /*
+ * If this is the root commit, do what rev_info tells us.
+ */
+ if (!parents)
+ return revs->show_root_diff;
+
+ /* When we get here we have precisely one parent. */
+ parent = parents->item;
+ parse_commit(parent);
+
+ files = 0;
+ add_lines = 0;
+ rem_lines = 0;
+
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ diff_tree_sha1(parent->tree->object.sha1,
+ commit->tree->object.sha1,
+ "", &revs->diffopt);
+ diffcore_std(&revs->diffopt);
+
+ found = !diff_queue_is_empty();
+ saved_fmt = revs->diffopt.output_format;
+ revs->diffopt.output_format = DIFF_FORMAT_CALLBACK;
+ revs->diffopt.format_callback = cgit_diff_tree_cb;
+ revs->diffopt.format_callback_data = handle_rename;
+ diff_flush(&revs->diffopt);
+ revs->diffopt.output_format = saved_fmt;
+ revs->diffopt.flags = saved_flags;
+
+ lines_counted = 1;
+ return found;
+}
+
static void print_commit(struct commit *commit, struct rev_info *revs)
{
struct commitinfo *info;
@@ -177,7 +245,8 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
cgit_print_age(commit->date, TM_WEEK * 2, FMT_SHORTDATE);
}
- if (ctx.repo->enable_log_filecount || ctx.repo->enable_log_linecount) {
+ if (!lines_counted && (ctx.repo->enable_log_filecount ||
+ ctx.repo->enable_log_linecount)) {
files = 0;
add_lines = 0;
rem_lines = 0;
@@ -325,7 +394,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
}
}
}
- if (commit_graph) {
+
+ if (!path || !ctx.cfg.enable_follow_links) {
+ /*
+ * If we don't have a path, "follow" is a no-op so make sure
+ * the variable is set to false to avoid needing to check
+ * both this and whether we have a path everywhere.
+ */
+ ctx.qry.follow = 0;
+ }
+
+ if (commit_graph && !ctx.qry.follow) {
argv_array_push(&rev_argv, "--graph");
argv_array_push(&rev_argv, "--color");
graph_set_column_colors(column_colors_html,
@@ -337,6 +416,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
else if (commit_sort == 2)
argv_array_push(&rev_argv, "--topo-order");
+ if (path && ctx.qry.follow)
+ argv_array_push(&rev_argv, "--follow");
argv_array_push(&rev_argv, "--");
if (path)
argv_array_push(&rev_argv, path);
@@ -347,10 +428,17 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
rev.verbose_header = 1;
rev.show_root_diff = 0;
rev.ignore_missing = 1;
+ rev.simplify_history = 1;
setup_revisions(rev_argv.argc, rev_argv.argv, &rev, NULL);
load_ref_decorations(DECORATE_FULL_REFS);
rev.show_decorations = 1;
rev.grep_filter.regflags |= REG_ICASE;
+
+ rev.diffopt.detect_rename = 1;
+ rev.diffopt.rename_limit = ctx.cfg.renamelimit;
+ if (ctx.qry.ignorews)
+ DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
+
compile_grep_patterns(&rev.grep_filter);
prepare_revision_walk(&rev);
@@ -368,11 +456,12 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link(ctx.qry.showmsg ? "Collapse" : "Expand", NULL,
NULL, ctx.qry.head, ctx.qry.sha1,
ctx.qry.vpath, ctx.qry.ofs, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg ? 0 : 1);
+ ctx.qry.search, ctx.qry.showmsg ? 0 : 1,
+ ctx.qry.follow);
html(")");
}
html("</th><th class='left'>Author</th>");
- if (commit_graph)
+ if (rev.graph)
html("<th class='left'>Age</th>");
if (ctx.repo->enable_log_filecount) {
html("<th class='left'>Files</th>");
@@ -388,13 +477,30 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
ofs = 0;
for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; i++) {
+ if (show_commit(commit, &rev))
+ i++;
free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
}
for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; i++) {
- print_commit(commit, &rev);
+ /*
+ * In "follow" mode, we must count the files and lines the
+ * first time we invoke diff on a given commit, and we need
+ * to do that to see if the commit touches the path we care
+ * about, so we do it in show_commit. Hence we must clear
+ * lines_counted here.
+ *
+ * This has the side effect of avoiding running diff twice
+ * when we are both following renames and showing file
+ * and/or line counts.
+ */
+ lines_counted = 0;
+ if (show_commit(commit, &rev)) {
+ i++;
+ print_commit(commit, &rev);
+ }
free_commit_buffer(commit);
free_commit_list(commit->parents);
commit->parents = NULL;
@@ -406,7 +512,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs - cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
if ((commit = get_revision(&rev)) != NULL) {
@@ -414,14 +521,16 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath,
ofs + cnt, ctx.qry.grep,
- ctx.qry.search, ctx.qry.showmsg);
+ ctx.qry.search, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</li>");
}
html("</ul>");
} else if ((commit = get_revision(&rev)) != NULL) {
htmlf("<tr class='nohover'><td colspan='%d'>", columns);
cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL,
- ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg);
+ ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg,
+ ctx.qry.follow);
html("</td></tr>\n");
}
diff --git a/ui-refs.c b/ui-refs.c
index d3d71dd..73a187b 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -63,7 +63,7 @@ static int print_branch(struct refinfo *ref)
return 1;
html("<tr><td>");
cgit_log_link(name, NULL, NULL, name, NULL, NULL, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
html("</td><td>");
if (ref->object->type == OBJ_COMMIT) {
diff --git a/ui-repolist.c b/ui-repolist.c
index 2453a7f..edefc4c 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -330,7 +330,7 @@ void cgit_print_repolist(void)
html("<td>");
cgit_summary_link("summary", NULL, "button", NULL);
cgit_log_link("log", NULL, "button", NULL, NULL, NULL,
- 0, NULL, NULL, ctx.qry.showmsg);
+ 0, NULL, NULL, ctx.qry.showmsg, 0);
cgit_tree_link("tree", NULL, "button", NULL, NULL, NULL);
html("</td>");
}
diff --git a/ui-shared.c b/ui-shared.c
index 4f84b7c..6be0c2e 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -303,7 +303,8 @@ void cgit_plain_link(const char *name, const char *title, const char *class,
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, const char *pattern, int showmsg)
+ int ofs, const char *grep, const char *pattern, int showmsg,
+ int follow)
{
char *delim;
@@ -332,6 +333,11 @@ void cgit_log_link(const char *name, const char *title, const char *class,
if (showmsg) {
html(delim);
html("showmsg=1");
+ delim = "&";
+ }
+ if (follow) {
+ html(delim);
+ html("follow=1");
}
html("'>");
html_txt(name);
@@ -373,6 +379,10 @@ void cgit_commit_link(char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
if (name[0] != '\0')
html_txt(name);
@@ -429,6 +439,10 @@ void cgit_diff_link(const char *name, const char *title, const char *class,
html("ignorews=1");
delim = "&";
}
+ if (ctx.qry.follow) {
+ html(delim);
+ html("follow=1");
+ }
html("'>");
html_txt(name);
html("</a>");
@@ -469,7 +483,7 @@ static void cgit_self_link(char *name, const char *title, const char *class)
ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
ctx.qry.path, ctx.qry.ofs,
ctx.qry.grep, ctx.qry.search,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, ctx.qry.follow);
else if (!strcmp(ctx.qry.page, "commit"))
cgit_commit_link(name, title, class, ctx.qry.head,
ctx.qry.has_sha1 ? ctx.qry.sha1 : NULL,
@@ -945,7 +959,7 @@ void cgit_print_pageheader(void)
ctx.qry.sha1, NULL);
cgit_log_link("log", NULL, hc("log"), ctx.qry.head,
NULL, ctx.qry.vpath, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, ctx.qry.follow);
cgit_tree_link("tree", NULL, hc("tree"), ctx.qry.head,
ctx.qry.sha1, ctx.qry.vpath);
cgit_commit_link("commit", NULL, hc("commit"),
@@ -993,6 +1007,14 @@ void cgit_print_pageheader(void)
html("<div class='path'>");
html("path: ");
cgit_print_path_crumbs(ctx.qry.vpath);
+ if (ctx.cfg.enable_follow_links && !strcmp(ctx.qry.page, "log")) {
+ html(" (");
+ ctx.qry.follow = !ctx.qry.follow;
+ cgit_self_link(ctx.qry.follow ? "follow" : "unfollow",
+ NULL, NULL);
+ ctx.qry.follow = !ctx.qry.follow;
+ html(")");
+ }
html("</div>");
}
html("<div class='content'>");
diff --git a/ui-shared.h b/ui-shared.h
index 43d0fa6..788b1bc 100644
--- a/ui-shared.h
+++ b/ui-shared.h
@@ -31,7 +31,7 @@ extern void cgit_plain_link(const char *name, const char *title,
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,
- const char *pattern, int showmsg);
+ const char *pattern, int showmsg, int follow);
extern void cgit_commit_link(char *name, const char *title,
const char *class, const char *head,
const char *rev, const char *path);
diff --git a/ui-tree.c b/ui-tree.c
index bbc468e..c8d24f6 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -166,7 +166,7 @@ static int ls_item(const unsigned char *sha1, struct strbuf *base,
html("<td>");
cgit_log_link("log", NULL, "button", ctx.qry.head,
walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL,
- ctx.qry.showmsg);
+ ctx.qry.showmsg, 0);
if (ctx.repo->max_stats)
cgit_stats_link("stats", NULL, "button", ctx.qry.head,
fullpath.buf);
--
2.5.0.466.g9af26fa
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 2/2] log: allow users to follow a file
2015-08-12 14:55 ` [PATCH v4 2/2] log: allow users to follow a file john
@ 2015-08-12 15:01 ` Jason
0 siblings, 0 replies; 18+ messages in thread
From: Jason @ 2015-08-12 15:01 UTC (permalink / raw)
Thanks for rebasing this. Merged to master for some testing.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 1/2] shared: make cgit_diff_tree_cb public
2015-08-12 14:34 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public john
2015-08-12 14:55 ` [PATCH v4 2/2] log: allow users to follow a file john
@ 2015-08-12 15:41 ` Jason
1 sibling, 0 replies; 18+ messages in thread
From: Jason @ 2015-08-12 15:41 UTC (permalink / raw)
This series broke the tests. Send a fix?
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-08-12 15:41 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-15 11:51 log --follow
2013-04-15 12:38 ` john
2013-04-15 17:30 `
2013-04-16 8:48 ` john
2013-04-17 19:48 ` [PATCH] log: allow users to follow a file john
2013-04-18 17:31 `
2013-04-19 7:54 ` john
2013-04-20 10:14 ` [PATCH v2 0/2] Allow users to follow file renames john
2013-04-20 10:14 ` [PATCH v2 1/2] shared: make cgit_diff_tree_cb public john
2013-04-20 10:14 ` [PATCH v2 2/2] log: allow users to follow a file john
2013-04-20 11:21 `
2013-04-20 12:21 ` [PATCH 2/2 v3] " john
2015-03-24 6:02 `
2015-08-12 14:34 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public john
2015-08-12 14:55 ` [PATCH v4 2/2] log: allow users to follow a file john
2015-08-12 15:01 ` Jason
2015-08-12 15:41 ` [PATCH v4 1/2] shared: make cgit_diff_tree_cb public Jason
2013-04-26 13:52 ` [PATCH v2 0/2] Allow users to follow file renames 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).