Re: [PATCH] Move color option parsing out of diff.c and into color.[ch]
- From
Junio C Hamano <junkio@cox.net>
- Date
- Sep 8, 2006, 08:49 UTC
- Message-ID
- <7vk64eivzj.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <20060908080318.GA3771@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> +static int color_vprintf(const char *color, const char *fmt,
> + va_list args, const char *trail)
> +{
> + int r = 0;
> +
> + if (*color)
> + r += printf("%s", color);
> + r += vprintf(fmt, args);
> + if (trail)
> + r += printf("%s", trail);
> + if (*color)
> + r += printf("%s", COLOR_RESET);
> + return r;
> +}Hmm,... don't you mean RESET first and then trail (which is often "\n")?
Show 9 quoted lines
> +int color_printf(const char *color, const char *fmt, ...)
> +{
> + va_list args;
> + int r;
> + va_start(args, fmt);
> + r = color_vprintf(color, fmt, args, 0);
> + va_end(args);
> + return r;
> +}Please spell NULL not 0 (please do not argue that writing a NULL pointer as integral constant 0 is perfectly valid C -- we all know that. This is not the language-lawyers correctness issue, but ../linux-2.6/Documentation/CodingStyle thing). Using pointer as boolean seems to be Ok style (e.g. "if (trail)" does not have to be written "if (trail == NULL)").