Re: [GSoC PATCH v5 2/5] repo: add the field references.format

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jul 21, 2025 at 09:28:32PM -0300, Lucas Seiki Oshiro wrote:
> diff --git a/builtin/repo.c b/builtin/repo.c
> index d4f01e35e2..5beae0f781 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -1,12 +1,83 @@
>  #include "builtin.h"
>  #include "parse-options.h"
> +#include "refs.h"
>  
> -static int repo_info(int argc UNUSED, const char **argv UNUSED,
> -		     const char *prefix UNUSED, struct repository *repo UNUSED)
> +typedef const char *get_value_fn(struct repository *repo);
> +
> +struct field {
> +	const char *key;
> +	get_value_fn *add_field_callback;
> +};
> +
> +static const char *get_references_format(struct repository *repo)
> +{
> +	return ref_storage_format_to_name(repo->ref_storage_format);
> +}
> +
> +/* repo_info_fields keys should be in lexicographical order */
> +static const struct field repo_info_fields[] = {
> +	{ "references.format", get_references_format },
> +};

One problem that we'll eventually face is that we want to add a callback
that needs to return an allocated string. With the current design we
cannot really handle that without creating amemory leak. So I guess it
would make more sense if the callback thus received a pointer to a
strbuf that it is expected to print its value to.

Also, we should be able to return errors. While that's not needed right
now, it may be in the future. So how about:

	typedef int get_value_fn(struct repository *repo, struct strbuf *buf);

	static int get_references_format(struct repository *repo,
					 struct strbuf *buf)
	{
		strbuf_addstr(buf, ref_storage_format_to_name(repo->ref_storage_format));
		return 0;
	}

> +static int repo_info_fields_cmp(const void *va, const void *vb)
> +{
> +	const struct field *a = va;
> +	const struct field *b = vb;
> +
> +	return strcmp(a->key, b->key);
> +}
> +
> +static get_value_fn *get_value_callback(const char *key)
>  {
> +	const struct field search_key = { key, NULL };
> +	const struct field *found = bsearch(&search_key, repo_info_fields,
> +					    ARRAY_SIZE(repo_info_fields),
> +					    sizeof(struct field),

Nit: let's rather use `sizeof(*repo_info_fields)`. Makes it more
trivially correct without having to double check whether
`repo_info_fields` actually is a `struct field`..

Patrick