Re: [PATCH v2 04/10] ref-filter: modify "%(objectname:short)" to take length

[Karthik Nayak <karthik.188@gmail.com>]

On Fri, Oct 9, 2015 at 12:28 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -464,14 +464,28 @@ 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)
>>  {
>> -     if (!strcmp(name, "objectname")) {
>> -             char *s = xmalloc(41);
>> -             strcpy(s, sha1_to_hex(sha1));
>> -             v->s = s;
>> -             return 1;
>> -     }
>> -     if (!strcmp(name, "objectname:short")) {
>> -             v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
>> +     const char *p;
>> +
>> +     if (match_atom_name(name, "objectname", &p)) {
>> +             /*  No arguments given, copy the entire objectname */
>> +             if (!p) {
>> +                     char *s = xmalloc(41);
>
> While you're there, you may want to get rid of this hardcoded 41 and
> write GIT_SHA1_HEXSZ + 1 instead.
>

Sure, will change that.

>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
>> index 7c9bec7..6fc569e 100755
>> --- a/t/t6300-for-each-ref.sh
>> +++ b/t/t6300-for-each-ref.sh
>> @@ -385,6 +385,28 @@ test_expect_success 'Check short objectname format' '
>>       test_cmp expected actual
>>  '
>>
>> +cat >expected <<EOF
>> +$(git rev-parse --short=1 HEAD)
>> +EOF
>
> Please write all code within test_expect_success including this
> (t/README:
>
>  - Put all code inside test_expect_success and other assertions.
>
>    Even code that isn't a test per se, but merely some setup code
>    should be inside a test assertion.
> ).
>

Was just following the previous syntax, should have read that. fixed it

>> +test_expect_success 'Check objectname:short format when size is less than MINIMUM_ABBREV' '
>> +     git for-each-ref --format="%(objectname:short=1)" refs/heads >actual &&
>> +     test_cmp expected actual
>> +'
>> +
>> +cat >expected <<EOF
>> +$(git rev-parse --short=10 HEAD)
>> +EOF
>
> Likewise.
>

fixed it here too. Thanks

-- 
Regards,
Karthik Nayak