Re: [WIP/PATCH v4 4/8] for-each-ref: introduce new structures for better organisation

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

On 05/31/2015 08:44 AM, Eric Sunshine wrote:
> On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> Intoduce 'ref_filter_cbdata' which will hold 'ref_filter'
>
> s/Intoduce/Introduce/
>
>> (Conditions to filter the refs on) and 'ref_array' (The array
>
> s/Conditions/conditions/
> s/The/the/
>
>> of ref_array_items). Modify the code to use these new structures.
>>
>> This is a preparatory patch to eventually move code from 'for-each-ref'
>> to 'ref-filter' and making it publically available.
>
> s/publically/publicly/
>
>> 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>
>> ---
>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index e634fd2..ef54c90 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -85,7 +99,7 @@ static struct {
>>    * a "*" to denote deref_tag().
>>    *
>>    * We parse given format string and sort specifiers, and make a list
>> - * of properties that we need to extract out of objects.  ref_array_item
>> + * of properties that we need to extract out of objects. ref_array_item
>
> Sneaking in whitespace change?
>
>>    * structure will hold an array of values extracted that can be
>>    * indexed with the "atom number", which is an index into this
>>    * array.
>> @@ -1076,12 +1085,12 @@ static char const * const for_each_ref_usage[] = {
>>
>>   int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>   {
>> -       int i, num_refs;
>> +       int i;
>>          const char *format = "%(objectname) %(objecttype)\t%(refname)";
>>          struct ref_sort *sort = NULL, **sort_tail = &sort;
>>          int maxcount = 0, quote_style = 0;
>> -       struct ref_array_item **refs;
>> -       struct grab_ref_cbdata cbdata;
>> +       struct ref_filter_cbdata ref_cbdata;
>> +       memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>>
>>          struct option opts[] = {
>
> Declaration (struct option opts[]) after statement (memset). I think
> you want to leave the memset() where it was below.
>
>>                  OPT_BIT('s', "shell", &quote_style,
>> @@ -1119,17 +1128,14 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>          /* for warn_ambiguous_refs */
>>          git_config(git_default_config, NULL);
>>
>> -       memset(&cbdata, 0, sizeof(cbdata));
>> -       cbdata.grab_pattern = argv;
>> -       for_each_rawref(grab_single_ref, &cbdata);
>> -       refs = cbdata.grab_array;
>> -       num_refs = cbdata.grab_cnt;
>> +       ref_cbdata.filter.name_patterns = argv;
>> +       for_each_rawref(grab_single_ref, &ref_cbdata);
>>
>> -       sort_refs(sort, refs, num_refs);
>> +       sort_refs(sort, &ref_cbdata.array);
>>
>> -       if (!maxcount || num_refs < maxcount)
>> -               maxcount = num_refs;
>> +       if (!maxcount || ref_cbdata.array.nr < maxcount)
>> +               maxcount = ref_cbdata.array.nr;
>>          for (i = 0; i < maxcount; i++)
>> -               show_ref(refs[i], format, quote_style);
>> +               show_ref(ref_cbdata.array.items[i], format, quote_style);
>>          return 0;
>>   }
>> --
>> 2.4.2

Will change these, thanks!

-- 
Regards,
Karthik