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  wrote:
> On Tue, Aug 4, 2015 at 8:42 AM, Karthik Nayak  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 
>> ---
>> 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


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  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 
> ---
> 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


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

2015-08-04 Thread Karthik Nayak
Introduce a strbuf `output` which will act as a substitute rather than
printing directly to stdout. This will be used for formatting
eventually.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 46963a5..91482c9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1190,30 +1190,25 @@ 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)
+static void print_value(struct atom_value *v, int quote_style, struct strbuf 
*output)
 {
-   struct strbuf sb = STRBUF_INIT;
switch (quote_style) {
case QUOTE_NONE:
-   fputs(v->s, stdout);
+   strbuf_addstr(output, v->s);
break;
case QUOTE_SHELL:
-   sq_quote_buf(&sb, v->s);
+   sq_quote_buf(output, v->s);
break;
case QUOTE_PERL:
-   perl_quote_buf(&sb, v->s);
+   perl_quote_buf(output, v->s);
break;
case QUOTE_PYTHON:
-   python_quote_buf(&sb, v->s);
+   python_quote_buf(output, v->s);
break;
case QUOTE_TCL:
-   tcl_quote_buf(&sb, v->s);
+   tcl_quote_buf(output, v->s);
break;
}
-   if (quote_style != QUOTE_NONE) {
-   fputs(sb.buf, stdout);
-   strbuf_release(&sb);
-   }
 }
 
 static int hex1(char ch)
@@ -1234,7 +1229,7 @@ static int hex2(const char *cp)
return -1;
 }
 
-static void emit(const char *cp, const char *ep)
+static void emit(const char *cp, const char *ep, struct strbuf *output)
 {
while (*cp && (!ep || cp < ep)) {
if (*cp == '%') {
@@ -1243,13 +1238,13 @@ static void emit(const char *cp, const char *ep)
else {
int ch = hex2(cp + 1);
if (0 <= ch) {
-   putchar(ch);
+   strbuf_addch(output, ch);
cp += 3;
continue;
}
}
}
-   putchar(*cp);
+   strbuf_addch(output, *cp);
cp++;
}
 }
@@ -1257,19 +1252,21 @@ static void emit(const char *cp, const char *ep)
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
 {
const char *cp, *sp, *ep;
+   struct strbuf output = STRBUF_INIT;
+   int i;
 
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
struct atom_value *atomv;
 
ep = strchr(sp, ')');
if (cp < sp)
-   emit(cp, sp);
+   emit(cp, sp, &output);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
&atomv);
-   print_value(atomv, quote_style);
+   print_value(atomv, quote_style, &output);
}
if (*cp) {
sp = cp + strlen(cp);
-   emit(cp, sp);
+   emit(cp, sp, &output);
}
if (need_color_reset_at_eol) {
struct atom_value resetv;
@@ -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]);
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