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

[Matthieu Moy]

Lawrence Siebert lawrencesieb...@gmail.com 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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lawrence Siebert]

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
matthieu@grenoble-inp.fr wrote:
 Lawrence Siebert lawrencesieb...@gmail.com 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/



-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Matthieu Moy]

Hi,

Please, don't top-post on this list. Quote the parts you're replying to
and reply below.

Lawrence Siebert lawrencesieb...@gmail.com writes:

 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.

If possible, passing NULL would be best.

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

OK, that was me answering emails before coffee. I thought --count was
producing the count _in addition_ to the normal output. But then I don't
understand by looking at the code how you prevent the normal output. I
just tested, and indeed it does work (I guess all the magic is already
there from rev-list --count, and it was more or less a bug that log
--count was not using it properly), but you may want to explain better
what's going on in the commit message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lawrence Siebert]

adds --count from git rev-list to git log, without --use-bitmap-index
for the moment.

Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com
---
 builtin/log.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index 8781049..4aaff3a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -25,6 +25,7 @@
 #include version.h
 #include mailmap.h
 #include gpg-interface.h
+#include list-objects.h
 
 /* Set a default date-time format for git log (log.date config variable) */
 static const char *default_date_mode = NULL;
@@ -317,12 +318,40 @@ static void finish_early_output(struct rev_info *rev)
show_early_header(rev, done, n);
 }
 
+static void show_object(struct object *obj,
+   const struct name_path *path, const char *component,
+   void *cb_data)
+{
+   return;
+}
+
+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);
+   }
if (rev-early_output)
setup_early_output(rev);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html