Re: [RFC PATCH v4] revision: new rev^-n shorthand for rev^n..rev

From: Jeff King <peff@peff.net>
Date: Sep 27, 2016, 06:10 UTC
Message-ID: <20160927061038.pgb4uyx4sillmq5r@sigill.intra.peff.net>
In-Reply-To: <xmqqh992pbq6.fsf@gitster.mtv.corp.google.com>

On Mon, Sep 26, 2016 at 02:55:45PM -0700, Junio C Hamano wrote:

Show 23 quoted lines

> Taking these two together, perhaps squashing this in may be
> sufficient.
> [...]
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 2c3da19..9474c37 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -333,8 +333,22 @@ static int try_parent_shorthands(const char *arg)
>  	if (include_rev)
>  		show_rev(NORMAL, sha1, arg);
>  	commit = lookup_commit_reference(sha1);
> +
> +	if (exclude_parent) {
> +		/* do we have enough parents? */
> +		for (parent_number = 0, parents = commit->parents;
> +		     parents;
> +		     parents = parents->next)
> +			parent_number++;
> +		if (parent_number < exclude_parent) {
> +			*dotdot = '^';
> +			return 0;
> +		}
> +	}

I think you can use commit_list_count() to make this a bit shorter, like:

  if (exclude_parent &&
      commit_list_count(commit->parents) < exclude_parent) {
          *dotdot = '^';
	  return 0;
  }

Technically you can drop the first half of the &&, but it is probably a good idea to avoid the traversal when exclude_parent is not in use.

Also technically, you can stop counting when you hit exclude_parent (which is only possible with a custom traversal), but it is unlikely enough that it is probably not worth caring about.

-Peff