From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 08 Dec 2010 22:50:57 GMT
Subject: Re: [PATCH 2/2] get_sha1: support ref^{/regex} syntax
Message-ID: <7vvd33j2um.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1291820319-12455-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/sha1_name.c b/sha1_name.c
> index f4ccdc5..00e52b0 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -562,6 +563,11 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
>  		expected_type = OBJ_BLOB;
>  	else if (sp[0] == '}')
>  		expected_type = OBJ_NONE;
> +	else if (sp[0] == '/') {
> +		if (sp[1] == '}')
> +			return -1;

Why?  $commit^{/} may be a no-op but I do not see a strong reason to
waste extra two lines to forbid it.

> @@ -584,11 +590,23 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
>  		 * barf.
>  		 */
>  		o = peel_to_type(name, len, o, expected_type);
> -		if (o) {
> -			hashcpy(sha1, o->sha1);
> -			return 0;
> +		if (!o)
> +			return -1;

I can see you are trying to reduce nesting of

        if (o) {
		do true thing
                return 0
	}
        return -1;

but then we should apply the same to outer "if (!expected_type) ... else",
too, to unnest the "else" clause by returning from the true branch of that
"if".