From: Junio C Hamano Date: Tue, 20 Mar 2012 17:23:52 GMT Subject: Re: [PATCH 1/1] Fix --stat width calculations to handle --graph Message-ID: <7vaa3b6v9z.fsf@alter.siamese.dyndns.org> In-Reply-To: Johannes Schindelin writes: > 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.