From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 28 Jul 2015 16:19:23 GMT
Subject: Re: [RFC/PATCH 01/11] ref-filter: add "%(objectname:size=X)" option
Message-ID: <xmqqvbd4fe5w.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1438066594-5620-1-git-send-email-Karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

> From: Karthik Nayak <karthik.188@gmail.com>
>
> Add support for %(objectname:size=X) where X is a number.
> This will print the first X characters of an objectname.
> The minimum value for X is 5. Hence any value lesser than
> 5 will default to 5 characters.

Where does this hardcoded 5 come from?

I'd agree that we would want some minimum for sanity (not safety),
but I do not think we want random callers of find-unique-abbrev
arbitrarily imposing their own minimum, making different codepaths
behave inconsistently without a good reason.

It seems that the minimum we use for sanity at the core level is
MINIMUM_ABBREV.  Is there a reason why that value is inappropriate
for this codepath?

>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  ref-filter.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 0a34924..4c5e3f8 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -196,6 +196,8 @@ static void *get_obj(const unsigned char *sha1, struct object **obj, unsigned lo
>  static int grab_objectname(const char *name, const unsigned char *sha1,
>  			    struct atom_value *v)
>  {
> +	const char *p;
> +
>  	if (!strcmp(name, "objectname")) {
>  		char *s = xmalloc(41);
>  		strcpy(s, sha1_to_hex(sha1));
> @@ -206,6 +208,13 @@ static int grab_objectname(const char *name, const unsigned char *sha1,
>  		v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
>  		return 1;
>  	}
> +	if (skip_prefix(name, "objectname:size=", &p)) {
> +		unsigned int size = atoi(p);
> +		if (size < 5)
> +			size = 5;
> +		v->s = xstrdup(find_unique_abbrev(sha1, size));
> +		return 1;
> +	}
>  	return 0;
>  }