Re: [PATCH v9 01/11] ref-filter: print output to strbuf for formatting

2015-08-06 Thread Eric Sunshine
On Tue, Aug 4, 2015 at 8:42 AM, Karthik Nayak karthik@gmail.com wrote:
 Introduce a strbuf `output` which will act as a substitute rather than
 printing directly to stdout. This will be used for formatting
 eventually.

 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
 diff --git a/ref-filter.c b/ref-filter.c
 index 46963a5..91482c9 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -1278,9 +1275,12 @@ 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);
 +   print_value(resetv, quote_style, output);
 }
 +   for (i = 0; i  output.len; i++)
 +   printf(%c, output.buf[i]);

Everything up to this point seems straightforward, however, it's not
clear why you need to emit 'output' one character at a time. Is it
because it might contain a NUL '\0' character and therefore you can't
use the simpler printf(%s, output.buf)?

If that's the case, then why not just use fwrite() to emit it all at once?

fwrite(output.buf, output.len, 1, stdout);

 putchar('\n');
 +   strbuf_release(output);
  }

  /*  If no sorting option is given, use refname to sort as default */
 --
 2.5.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 01/11] ref-filter: print output to strbuf for formatting

2015-08-06 Thread Karthik Nayak
On Fri, Aug 7, 2015 at 3:51 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Tue, Aug 4, 2015 at 8:42 AM, Karthik Nayak karthik@gmail.com wrote:
 Introduce a strbuf `output` which will act as a substitute rather than
 printing directly to stdout. This will be used for formatting
 eventually.

 Signed-off-by: Karthik Nayak karthik@gmail.com
 ---
 diff --git a/ref-filter.c b/ref-filter.c
 index 46963a5..91482c9 100644
 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -1278,9 +1275,12 @@ 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);
 +   print_value(resetv, quote_style, output);
 }
 +   for (i = 0; i  output.len; i++)
 +   printf(%c, output.buf[i]);

 Everything up to this point seems straightforward, however, it's not
 clear why you need to emit 'output' one character at a time. Is it
 because it might contain a NUL '\0' character and therefore you can't
 use the simpler printf(%s, output.buf)?

 If that's the case, then why not just use fwrite() to emit it all at once?

 fwrite(output.buf, output.len, 1, stdout);


It was to avoid the printing to stop at '\0' as you mentioned.
I've never come across such a situation before, so I looked for
similar implementations online, and found the individual character printing.
Thanks `fwrite` seems neater.


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