From: Karthik Nayak <karthik.188@gmail.com>
Date: Sun, 30 Aug 2015 06:39:36 GMT
Subject: Re: [PATCH v14 06/13] ref-filter: introduce format_ref_array_item()
Message-ID: <CAOLa=ZTiVS=3M8ckr2Ouz_46h7zaHBcRyxqHMbhGEAt3uWNaCQ@mail.gmail.com>
In-Reply-To: <CAPig+cT2ySwYtTjv7Xe3uv35OH8Bhdyj3m_eqyJjMe_mB6Xj=w@mail.gmail.com>

On Sun, Aug 30, 2015 at 9:12 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Aug 29, 2015 at 10:12 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> Create format_ref_array_item() out of show_ref_array_item(). This will
>> store the output format for the given ref_array_item into the provided
>> strbuf. Make show_ref_array_item() a wrapper around this to print the
>> given ref_array_item with linefeed.
>
> Perhaps you could explain why this change is a good idea, such as that
> a future patch, for <fill-in-the-blank> reason, will need the
> formatting capability of format_ref_array_item() but not the printing
> with newline done by show_ref_array_item().
>

Yeah sure.

>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 5d4f93d..1e6754a 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -153,6 +153,27 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
>>         return at;
>>  }
>>
>> +static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
>> +{
>> +       switch (quote_style) {
>> +       case QUOTE_NONE:
>> +               strbuf_addstr(s, str);
>> +               break;
>> +       case QUOTE_SHELL:
>> +               sq_quote_buf(s, str);
>> +               break;
>> +       case QUOTE_PERL:
>> +               perl_quote_buf(s, str);
>> +               break;
>> +       case QUOTE_PYTHON:
>> +               python_quote_buf(s, str);
>> +               break;
>> +       case QUOTE_TCL:
>> +               tcl_quote_buf(s, str);
>> +               break;
>> +       }
>> +}
>
> This code was already relocated once in patch 4/13, and is now being
> relocated again in 6/13. If you instead place the code at the final
> desired location in 4/13, then this patch will become less noisy.
>

Will do.

> More below.
>
>>  static void push_stack_element(struct ref_formatting_stack **stack)
>>  {
>>         struct ref_formatting_stack *s = xcalloc(1, sizeof(struct ref_formatting_stack));
>> @@ -665,27 +686,6 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s
>>         new->cb_data = atomv->align;
>>  }
>>
>> -static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
>> -{
>> -       switch (quote_style) {
>> -       case QUOTE_NONE:
>> -               strbuf_addstr(s, str);
>> -               break;
>> -       case QUOTE_SHELL:
>> -               sq_quote_buf(s, str);
>> -               break;
>> -       case QUOTE_PERL:
>> -               perl_quote_buf(s, str);
>> -               break;
>> -       case QUOTE_PYTHON:
>> -               python_quote_buf(s, str);
>> -               break;
>> -       case QUOTE_TCL:
>> -               tcl_quote_buf(s, str);
>> -               break;
>> -       }
>> -}
>> -
>>  static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
>>  {
>>         /*
>> @@ -1478,10 +1478,17 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
>>         }
>>         if (state.stack->prev)
>>                 die(_("format: `end` atom missing"));
>> -       final_buf = &state.stack->output;
>> -       fwrite(final_buf->buf, 1, final_buf->len, stdout);
>> +       strbuf_addbuf(out, &state.stack->output);
>>         pop_stack_element(&state.stack);
>> -       putchar('\n');
>> +}
>> +
>> +void show_ref_array_item(struct ref_array_item *item, const char *format, unsigned int quote_style)
>> +{
>> +       struct strbuf out = STRBUF_INIT;
>> +       format_ref_array_item(&out, item, format, quote_style);
>> +       fwrite(out.buf, out.len, 1, stdout);
>> +       printf("\n");
>
> putchar('\n');

Thanks for the review.

-- 
Regards,
Karthik Nayak