Re: [PATCH v8 02/11] ref-filter: introduce ref_formatting_state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 4, 2015 at 2:12 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> Introduce a ref_formatting_state which will eventually hold the values
>> of modifier atoms. Implement this within ref-filter.
>>
>> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
>> Mentored-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
>> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
>> ---
>>  ref-filter.c | 49 +++++++++++++++++++++++++++++++++++++------------
>>  ref-filter.h |  4 ++++
>>  2 files changed, 41 insertions(+), 12 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index febdc45..c4c7064 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1190,9 +1190,10 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
>>       qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
>>  }
>>
>> -static void print_value(struct atom_value *v, int quote_style, struct strbuf *output)
>> +static void print_value(struct atom_value *v, struct ref_formatting_state *state,
>> +                     struct strbuf *output)
>>  {
>
> I expect that the state would eventually become a stack of states
> (i.e. the topmost one passed around, each having a pointer to the
> previous level) in order to implement that "divert" mechanism for
> (possibly nested) if ... end construct.
>

Makes sense :)

> With that in mind, I suspect that state->output should be "where the
> current level would output to", i.e. no need to pass state and
> output around separately.
>

Will do!

>> +static void apply_formatting_state(struct ref_formatting_state *state, struct strbuf *value,
>> +                                struct strbuf *format)
>> +{
>
> The name "format" feels quite misleading; the readers would expect
> that you would use it in "strbuf_addf(format, value)", but that is
> not what is going on here.
>

will change it to final i guess.

>> @@ -1275,12 +1299,13 @@ void show_ref_array_item(struct ref_array_item *info, const char *format, int qu
>>               if (color_parse("reset", color) < 0)
>>                       die("BUG: couldn't parse 'reset' as a color");
>>               resetv.s = color;
>> -             print_value(&resetv, quote_style, &output);
>> +             print_value(&resetv, &state, &value);
>> +             apply_formatting_state(&state, &value, &final_buf);
>>       }
>> -     for (i = 0; i < output.len; i++)
>> -             printf("%c", output.buf[i]);
>> +     for (i = 0; i < final_buf.len; i++)
>> +             printf("%c", final_buf.buf[i]);
>>       putchar('\n');
>> -     strbuf_release(&output);
>> +     strbuf_release(&final_buf);
>>  }
>>
>>  /*  If no sorting option is given, use refname to sort as default */
>> diff --git a/ref-filter.h b/ref-filter.h
>> index 6bf27d8..b64677f 100644
>> --- a/ref-filter.h
>> +++ b/ref-filter.h
>> @@ -16,6 +16,10 @@
>>  #define FILTER_REFS_INCLUDE_BROKEN 0x1
>>  #define FILTER_REFS_ALL 0x2
>>
>> +struct ref_formatting_state {
>> +     int quote_style;
>> +};
>> +
>>  struct atom_value {
>>       const char *s;
>>       unsigned long ul; /* used for sorting when not FIELD_STR */



-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]