"Wang Bing-hua via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Wang Bing-hua <louiswpf@xxxxxxxxx>
>
> Remote names exceeding a tab width could cause misalignment.
> Align --verbose output with spaces instead of a tab.
While I am still not convinced if this change is a good idea (see
my earlier comment in a separate message)...
> +static int calc_maxwidth(struct string_list *list)
> +{
> + int max = 0;
> + struct string_list_item *item;
> +
> + for_each_string_list_item (item, list) {
> + int w = utf8_strwidth(item->string);
> +
> + if (w > max)
> + max = w;
> + }
> + return max;
> +}
> +
> static int show_all(void)
> {
> struct string_list list = STRING_LIST_INIT_DUP;
> @@ -1287,16 +1302,25 @@ static int show_all(void)
> result = for_each_remote(get_one_entry, &list);
>
> if (!result) {
> - int i;
> + int maxwidth = 0;
> + struct string_list_item *item;
>
> + if (verbose)
> + maxwidth = calc_maxwidth(&list);
I wonder if it is a better idea to extend get_one_entry() interface
to take not just a string_list but something like
struct remotes_data {
int maxwidth;
struct string_list *list_of_remotes;
};
if we think it is a good idea to give richer output to show_all()
function (instead of keep it spartan and compatible for the sake of
not breaking machine readers). There may be things other than
maxwidth that future changes to "git remote [-v]" may find needed.
And with such a change, you do not need a separate iteration over
the list of remotes just to call calc_maxwidth() callback. Keeping
a tally of "max length we have seen" inside get_one_entry() regardless
of "--verbose" setting shouldn't be too costly and help reduce the
complexity of the code.
> string_list_sort(&list);
> - for (i = 0; i < list.nr; i++) {
> - struct string_list_item *item = list.items + i;
> - if (verbose)
> - printf("%s\t%s\n", item->string,
> - item->util ? (const char *)item->util : "");
> - else {
> - if (i && !strcmp((item - 1)->string, item->string))
> + for_each_string_list_item (item, &list) {
Use of for_each_string_list_item() instead of a manual iteration is
probably a good idea here. If this were a larger change, that may
deserve to be a preparatory step on its own, but it is probably OK
to do so in the same patch.
> + if (verbose) {
> + struct strbuf s = STRBUF_INIT;
> +
> + strbuf_utf8_align(&s, ALIGN_LEFT, maxwidth + 1,
> + item->string);
> + if (item->util)
> + strbuf_addstr(&s, item->util);
> + printf("%s\n", s.buf);
> + strbuf_release(&s);
Wouldn't it work to just do (totally untested code snippet below;
may have off-by-one around maxwidth)
printf("%.*s%s", maxwidth, item->string,
item->util ? "" : item->util);
without using any strbuf operation?
