From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 07 May 2013 15:20:51 GMT
Subject: Re: [PATCH v6 4/7] git-clean: use a git-add-interactive compatible UI
Message-ID: <7vmws6lsto.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CANYiYbEE90zvhoRp3utOzRWXnpVGTcaruWDuCCHE77FmGwB2_w@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> 2013/5/7 Jiang Xin <worldhello.net@gmail.com>:
>> Rewrite menu using a new method `list_and_choose`, which is borrowed
>> from `git-add--interactive.perl`. We can reused this method later for
>> more actions.
>>
>> Please NOTE:
>>
>>  * Method `list_and_choose` return an array of integers, and
>>  * it is up to you to free the allocated memory of the array.
>>  * The array ends with EOF.
>>  * If user pressed CTRL-D (i.e. EOF), no selection returned.
>>
>> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
>> ---
>>  builtin/clean.c | 410 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 367 insertions(+), 43 deletions(-)
>...
>> -void edit_by_patterns_cmd()
>> +void pretty_print_menus(struct string_list *menu_list)
>> +{
>> +       struct strbuf buf = STRBUF_INIT;
> unused buf should be deleted.
>
>> +       unsigned int local_colopts = 0;
>> +       struct column_options copts;
>> +
>> +       /*
>> +        * always enable column display, we only consult column.*
>> +        * about layout strategy and stuff
>> +        */
> remove the above comments.
>
>> +       local_colopts = COL_ENABLED | COL_ROW;
>> +       memset(&copts, 0, sizeof(copts));
>> +       copts.indent = "  ";
>> +       copts.padding = 2;
>> +       print_columns(menu_list, local_colopts, &copts);
>> +       strbuf_release(&buf);
> remove strbuf_release of unused variable : buf.
>
>> +}
>...
>> +               } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
>> +                       struct string_list_item *item;
>> +                       struct strbuf buf = STRBUF_INIT;
> should call strbuf_release later
> ...
>> +       } else {
>> +               result = xmalloc(sizeof(int) * (nr + 1));
>> +               memset(result, 0, sizeof(int) * (nr + 1));
>
> Add initial for j here:


What is this message trying to achieve?  "self review"???

A bit puzzled....