Re: [PATCH v2 3/4] log --count: added test

[Junio C Hamano <gitster@pobox.com>]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Also, some revision-limiting options can reduce the count like
>
> git log --grep whatever
>
> and you should check that you actually count the right number here.
>
> (I don't know this part of the code enough, but I'm not sure you
> actually deal with this properly)

Yes, "rev-list", when the revision range is "limited" (with or
without pathspec), can give "--count" once limit_list() finishes,
but "log" filters the result of limit_list() further with at least
three separate phases.

 - options in the "grep" family (--grep/--author/etc.) lets you skip
   commits based on the contents of the commit object;

 - options in the "diff" family (-w/-b/etc.) may let "git log"
   consider a commit because the pathspec limit thought two blobs
   were different at byte-by-byte level, but after running "diff"
   with these "looser" comparison, "git log" may realize that there
   weren't any interesting change introduced by the commit [*1*];

 - and finally, of course "log --max-count=20" may further limit the
   maximum number of commits that are shown.  This of course is not
   interesting in the context of "--count" in the sense that "git
   log --count -20 --grep=foo maint..master" may not be immediately
   a sensible thing to do (but we never know.  Perhaps your user may
   be asking "do we have 20 or more commits that say 'foo' in the
   range?")

An implementation of "--count" to take the first and the third ones
in account may not be too hard, but I am fairly familiar with the
codepath for the second one and I think it would be very tricky.

Note that these additional things "log" does over "rev-list" *DO*
justify addition of "--count" to "log" (because "rev-list --count"
cannot emulate these); I am however not sure if it is worth the
additional complexity we need to add to the codepath (especially for
the second phase).  I'd need to take another look at the codepaths
involved myself to be sure, but I suspect the damage to the codepath
for the second may end up to be extensive when we do decide to fix
the possible bug in it.


[Footnote]

*1* They may still show the log message in such a case where "-b/-w"
was asked and commit had only whitespace changes, but I think we
should consider that a bug.