Re: [PATCH v2 3/6] builtin/repo: introduce stats subcommand
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 25, 2025, 05:38 UTC
- Message-ID
- <aNTVa3-RtYNPlcHc@pks.im>
- In-Reply-To
- <20250924212426.2930029-4-jltobler@gmail.com>
On Wed, Sep 24, 2025 at 04:24:23PM -0500, Justin Tobler wrote:
Show 31 quoted lines
> diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
> index 209afd1b61..a009bf8cf1 100644
> --- a/Documentation/git-repo.adoc
> +++ b/Documentation/git-repo.adoc
> @@ -156,12 +159,188 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix,
> return print_fields(argc, argv, repo, format);
> }
>
> +struct ref_stats {
> + size_t branches;
> + size_t remotes;
> + size_t tags;
> + size_t others;
> +};
> +
> +struct stats_table {
> + struct string_list rows;
> +
> + size_t name_col_width;
> + size_t value_col_width;
> +};
> +
> +/*
> + * Holds column data that gets stored for each row.
> + */
> +struct stats_table_entry {
> + char *value;
> +};
> +
> +static void stats_table_add(struct stats_table *table, const char *format,
> + const char *name, struct stats_table_entry *entry)We could of course accept varargs right from the start and thus allow the caller to pass arbitrary formatting directives. But I guess we don't need it now, so it's fine to not do it.
[snip]
> +static void stats_table_print(struct stats_table *table)
Nit: The table can be marked as `const` as we don't modify it.
Show 39 quoted lines
> +{
> + const char *name_col_title = _("Repository stats");
> + const char *value_col_title = _("Value");
> + size_t name_title_len = strlen(name_col_title);
> + size_t value_title_len = strlen(value_col_title);
> + struct strbuf buf = STRBUF_INIT;
> + struct string_list_item *item;
> + int name_col_width;
> + int value_col_width;
> +
> + name_col_width = cast_size_t_to_int(
> + max_size_t(table->name_col_width, name_title_len));
> + value_col_width = cast_size_t_to_int(
> + max_size_t(table->value_col_width, value_title_len));
> +
> + strbuf_addf(&buf, "| %-*s | %-*s |\n", name_col_width, name_col_title,
> + value_col_width, value_col_title);
> + strbuf_addstr(&buf, "| ");
> + strbuf_addchars(&buf, '-', name_col_width);
> + strbuf_addstr(&buf, " | ");
> + strbuf_addchars(&buf, '-', value_col_width);
> + strbuf_addstr(&buf, " |\n");
> +
> + for_each_string_list_item(item, &table->rows) {
> + struct stats_table_entry *entry = item->util;
> + const char *value = "";
> +
> + if (entry) {
> + struct stats_table_entry *entry = item->util;
> + value = entry->value;
> + }
> +
> + strbuf_addf(&buf, "| %-*s | %*s |\n", name_col_width,
> + item->string, value_col_width, value);
> + }
> +
> + fputs(buf.buf, stdout);
> + strbuf_release(&buf);
> +}By the way, is there any specific reason we do the detour via the strbuf instead of printing the data to stdout directly?
Show 13 quoted lines
> +static void stats_table_clear(struct stats_table *table)
> +{
> + struct stats_table_entry *entry;
> + struct string_list_item *item;
> +
> + for_each_string_list_item(item, &table->rows) {
> + entry = item->util;
> + if (entry)
> + free(entry->value);
> + }
> +
> + string_list_clear(&table->rows, 1);
> +}Yeah, this is much nicer now.
Show 23 quoted lines
> +static void stats_count_references(struct ref_stats *stats, struct ref_array *refs)
> +{
> + for (int i = 0; i < refs->nr; i++) {
> + struct ref_array_item *ref = refs->items[i];
> +
> + switch (ref->kind) {
> + case FILTER_REFS_BRANCHES:
> + stats->branches++;
> + break;
> + case FILTER_REFS_REMOTES:
> + stats->remotes++;
> + break;
> + case FILTER_REFS_TAGS:
> + stats->tags++;
> + break;
> + case FILTER_REFS_OTHERS:
> + stats->others++;
> + break;
> + default:
> + BUG("unexpected reference type");
> + }
> + }
> +}Here we're now being defensive. Good.
Patrick