Re: [PATCH v2 2/4] log: add --count option to git log

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2 2/4] log: add --count option to git log

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:05:38

Lawrence Siebert [off-list ref] writes:
+static void show_object(struct object *obj,
+			const struct name_path *path, const char *component,
+			void *cb_data)
+{
+	return;
+}
It seems streange to me to have a function named show_object when it
does not show anything. Maybe name it null_travers_cb to make it clear
it's a callback and it does nothing?

Not a strong objection though, it's only a static function.
+static void show_commit(struct commit *commit, void *data)
+{
+	struct rev_info *revs = (struct rev_info *)data;
+	if (commit->object.flags & PATCHSAME)
+		revs->count_same++;
+	else if (commit->object.flags & SYMMETRIC_LEFT)
+		revs->count_left++;
+	else
+		revs->count_right++;
+	if (commit->parents) {
+		free_commit_list(commit->parents);
+		commit->parents = NULL;
+	}
+	free_commit_buffer(commit);
+}
+
 static int cmd_log_walk(struct rev_info *rev)
 {
 	struct commit *commit;
 	int saved_nrl = 0;
 	int saved_dcctc = 0;
 
+	if (rev->count) {
+		prepare_revision_walk(rev);
+		traverse_commit_list(rev, show_commit, show_object, rev);
+		get_commit_count(rev);
+	}
I didn't test, but it seems this does a full graph traversal before
starting the output. A very important property of "git log" is that it
starts showing revisions immediately, even when ran on a very long
history (it shows the first screen immediately and continues working in
the background while the first page is displayed in the pager).

Is it the case? If so, it should be changed. If not, perhaps explain why
in the commit message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v2 2/4] log: add --count option to git log

From: Lawrence Siebert <hidden>
Date: 2016-06-15 23:05:38

traverse_commit_list requires a function to be passed to it.  That
said, after further review it can probably pass NULL and have it work
fine.  If not, I'll use your naming convention.

`git rev-list --count` (or actually `git rev-list --count HEAD`, which
this borrows the code from, produces a single value, a numeric count.
I think walking the entire list is necessary to get the final value,
which is what we want with --count.

Thanks,
Lawrence Siebert





On Fri, Jul 3, 2015 at 12:29 AM, Matthieu Moy
[off-list ref] wrote:
Lawrence Siebert [off-list ref] writes:
quoted
+static void show_object(struct object *obj,
+                     const struct name_path *path, const char *component,
+                     void *cb_data)
+{
+     return;
+}
It seems streange to me to have a function named show_object when it
does not show anything. Maybe name it null_travers_cb to make it clear
it's a callback and it does nothing?

Not a strong objection though, it's only a static function.
quoted
+static void show_commit(struct commit *commit, void *data)
+{
+     struct rev_info *revs = (struct rev_info *)data;
+     if (commit->object.flags & PATCHSAME)
+             revs->count_same++;
+     else if (commit->object.flags & SYMMETRIC_LEFT)
+             revs->count_left++;
+     else
+             revs->count_right++;
+     if (commit->parents) {
+             free_commit_list(commit->parents);
+             commit->parents = NULL;
+     }
+     free_commit_buffer(commit);
+}
+
 static int cmd_log_walk(struct rev_info *rev)
 {
      struct commit *commit;
      int saved_nrl = 0;
      int saved_dcctc = 0;

+     if (rev->count) {
+             prepare_revision_walk(rev);
+             traverse_commit_list(rev, show_commit, show_object, rev);
+             get_commit_count(rev);
+     }
I didn't test, but it seems this does a full graph traversal before
starting the output. A very important property of "git log" is that it
starts showing revisions immediately, even when ran on a very long
history (it shows the first screen immediately and continues working in
the background while the first page is displayed in the pager).

Is it the case? If so, it should be changed. If not, perhaps explain why
in the commit message.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/


-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help