Re: [PATCHv2 2/3] revision.c: introduce --min-parents and --max-parents

[Michael J Gruber <git@drmicha.warpmail.net>]

Michael J Gruber venit, vidit, dixit 21.03.2011 11:14:
> --merges and --no-merges are named confusingly and cannot be overridden
> by each other, but they are there to stay for plumbers' sake.
> 
> Introduce --min-parents and --max-parents which limit the revisions to
> those commits which have at least resp. at most that many commits, where
> negative arguments for --max-parents= denote infinity (i.e. no upper
> limit).
> 
> In particular:
> 
> --max-parents=1: no merges
> --min-parents=2: merges only
> --max-parents=0: only roots
> --min-parents=3: only octopusses
> 
> --min-parents=n --max-parents=m with n>m gives you what you ask for
> (nothing) just like --merges --no-merges does, but at least for an
> obvious reason.
> 
> Implementation note:
> 
> We compute the number of parents only when we limit by that, so there
> is no performance impact when there are no limiters.
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
>  builtin/log.c       |    2 +-
>  builtin/rev-list.c  |    2 ++
>  builtin/rev-parse.c |    2 ++
>  revision.c          |   24 +++++++++++++++++-------
>  revision.h          |    4 ++--
>  5 files changed, 24 insertions(+), 10 deletions(-)
...
> @@ -2029,10 +2034,15 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
>  		return commit_ignore;
>  	if (revs->min_age != -1 && (commit->date > revs->min_age))
>  		return commit_ignore;
> -	if (revs->no_merges && commit->parents && commit->parents->next)
> -		return commit_ignore;
> -	if (revs->merges_only && !(commit->parents && commit->parents->next))
> -		return commit_ignore;
> +	if (revs->min_parents || (revs->max_parents >= 0)) {
> +		int n = 0;
> +		struct commit_list *p;
> +		for (p = commit->parents; p; p = p->next)
> +			n++;
> +		if ((n < revs->min_parents) ||
> +		    ((revs->max_parents >= 0) && (n > revs->max_parents)))
> +			return commit_ignore;
> +	}
>  	if (!commit_match(commit, revs))
>  		return commit_ignore;
>  	if (revs->prune && revs->dense) {

BTW: Version v2-eps had this conditional:

(n < revs->min_parents) || (n > (unsigned int) revs->max_parents)

I actually compared runs of "git rev-list origin/next >/dev/null"
without options and with "--merges" or "--no-merges" for git without
this patch and with it (v2) and do not see any measurable difference
(between without or with the patch; the runs with limiting options are
faster, so I redid it with --count, again no difference with or without
patch).

So, the clearer version above should be preferred over the unsigned
trick, and special casing seems not worth it.

Michael