Re: [PATCH 1/1] Fix --stat width calculations to handle --graph

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 20, 2012, 17:23 UTC
Message-ID: <7vaa3b6v9z.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.1203201109370.3340@s15462909.onlinehome-server.info>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 14 quoted lines

> However, a little nit, since this list is so famous for "just a little
> nit": I'd prefer to factor-out the indent width measuring, like so:
>
> static int count_pipes_and_spaces(const char *string)
> {
> 	int count;
>
> 	for (count = 0; *string; string++)
> 		if (*string == '|' || *string == ' ')
> 			count++;
>
> 	return count;
> }
>

I agree that this is much better than the original by Lucian, but if we were to go this route, I would prefer to see it *not* count pipes and spaces, but actually measure the display width of the string. Both the name of the function and the implementation would have to change, of course.

Even though I didn't look very closely, I do not think it should be too hard for graph.c to tell the diff_options structure how wide a prefix it placed in the output_prefix, so use of such a "display_columns()" function would be wasteful for this particular case, but for a more general case, it would come in handy as a helper function, and at that point, this should not hide in diff.c as a static function.

Thanks.