On Tue, Dec 17, 2024 at 12:39:36PM +0000, Wang Bing-hua via GitGitGadget wrote: > From: Wang Bing-hua <louiswpf@xxxxxxxxx> > > Remote names exceeding a tab width could cause misalignment. > Align --verbose output with spaces instead of a tab. > Good enhancement. > Signed-off-by: Wang Bing-hua <louiswpf@xxxxxxxxx> > --- > remote: align --verbose output with spaces > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1837%2Flouiswpf%2Fremote-align-verbose-output-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1837/louiswpf/remote-align-verbose-output-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1837 > > builtin/remote.c | 30 ++++++++++++++++++++++++++---- > t/t5505-remote.sh | 4 ++-- > 2 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/builtin/remote.c b/builtin/remote.c > index 1ad3e70a6b4..876274d9dca 100644 > --- a/builtin/remote.c > +++ b/builtin/remote.c > @@ -16,6 +16,7 @@ > #include "strvec.h" > #include "commit-reach.h" > #include "progress.h" > +#include "utf8.h" > > static const char * const builtin_remote_usage[] = { > "git remote [-v | --verbose]", > @@ -1279,6 +1280,20 @@ static int get_one_entry(struct remote *remote, void *priv) > return 0; > } > > +static int calc_maxwidth(struct string_list *list) > +{ > + int max = 0; > + > + for (int i = 0; i < list->nr; i++) { Nit: we should use "size_t" to declare/define loop variable `i` because the type of `list-nr` is `size_t`. Recently, Patrick has provided a patch to start warn unsigned value compared with signed value in [1] which has not been merged into the master. [1] https://lore.kernel.org/git/20241206-pks-sign-compare-v4-0-0344c6dfb219@xxxxxx > + struct string_list_item *item = list->items + i; > + int w = utf8_strwidth(item->string); > + > + if (w > max) > + max = w; > + } > + return max; > +} > + So, here we traverse the list to find the max "utf8_strwidth". However, we should not EXPLICITLY traverse the string list. There are two ways you could do: 1. Use the helper macro "for_each_string_list_item" in "string-list.h" to do above. 2. Use the helper function "for_each_string_list" in "string-list.c" to do above. > static int show_all(void) > { > struct string_list list = STRING_LIST_INIT_DUP; > @@ -1292,10 +1307,17 @@ static int show_all(void) > 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 (verbose) { > + struct strbuf s = STRBUF_INIT; > + > + strbuf_utf8_align(&s, ALIGN_LEFT, > + calc_maxwidth(&list) + 1, > + item->string); So, here we call `calc_maxwidth` in the loop. That does not make sense. We should not call this function when we are traversing the string list. I think we should firstly calculate the max width outside of the loop. Thanks, Jialuo
