Re: [GSoC RFC PATCH v4 0/4] repo: add new command for retrieving repository info
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 16, 2025, 20:33 UTC
- Message-ID
- <xmqqjz47anad.fsf@gitster.g>
- In-Reply-To
- <xmqq7c07c2fu.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 45 quoted lines
> The below is from
>
> $ git clang-format --diff $(git merge-base master HEAD) -- builtin/repo.c
>
> I removed some obviously bad suggestions but many were improvements.
>
> I'll follow this message up as if I were reviewing what clang-format
> produced.
>
> Thanks.
>
>
> builtin/repo.c | 47 ++++++++++++++++++++---------------------------
> 1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git c/builtin/repo.c w/builtin/repo.c
> index d75417a48b..e8cf465da6 100644
> --- c/builtin/repo.c
> +++ w/builtin/repo.c
> @@ -14,27 +14,23 @@ struct field {
> add_field_fn *add_field_callback;
> };
>
> -static void add_string(struct strbuf *buf,
> - const char *key, const char *value)
> +static void add_string(struct strbuf *buf, const char *key, const char *value)
> {
> strbuf_addf(buf, "%s\n%s%c", key, value, '\0');
> }
>
> -static void add_bool(struct strbuf *buf,
> - const char *key, const int value)
> +static void add_bool(struct strbuf *buf, const char *key, const int value)
> {
> const char *output_value = value ? "true" : "false";
> strbuf_addf(buf, "%s\n%s%c", key, output_value, '\0');
> }
>
> -static void add_references_format(struct strbuf *buf,
> - struct repository *repo)
> +static void add_references_format(struct strbuf *buf, struct repository *repo)
> {
> add_string(buf, "references.format",
> ref_storage_format_to_name(repo->ref_storage_format));
> }If the parameter list fits on a single line not just helps readers of a code who reads from top to bottom, but those who run "grep" for the function name.
Show 5 quoted lines
>
> -
> static void add_layout_bare(struct strbuf *buf, struct repository *repo UNUSED)
> {
> add_bool(buf, "layout.bare", is_bare_repository());No reason to give double-blank between these two functions; it is not like add_string/bool/references/format are closer together than this add_layout_bare
Show 6 quoted lines
> @@ -45,11 +41,11 @@ static void add_layout_shallow(struct strbuf *buf, struct repository *repo) > add_bool(buf, "layout.shallow", is_repository_shallow(repo)); > } > > -// repo_info_fields keys should be in lexicographical order > +/* repo_info_fields keys should be in lexicographical order */
We don't do // comments
Show 8 quoted lines
> static const struct field repo_info_fields[] = {
> - {"layout.bare", add_layout_bare},
> - {"layout.shallow", add_layout_shallow},
> - {"references.format", add_references_format},
> + { "layout.bare", add_layout_bare },
> + { "layout.shallow", add_layout_shallow },
> + { "references.format", add_references_format },
> };Spaces are used inside {braces} like the above.Show 13 quoted lines
> static int repo_info_fields_cmp(const void *va, const void *vb)
> @@ -60,12 +56,13 @@ static int repo_info_fields_cmp(const void *va, const void *vb)
> return strcmp(a->key, b->key);
> }
>
> -static add_field_fn *get_append_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),
> - repo_info_fields_cmp);
> +static add_field_fn *get_append_callback(const char *key)
> +{"{" opening and "}" closing braces around a function body sit on their own line alone without anybody else. This is unlike the multi-statement blocks in control structures (e.g. opening brace for the block executed when condition holds in "if (cond) {" comes after the closing parenthesis ")" for the condition after a SP on the same line).
> + const struct field search_key = { key, NULL };Use of spaces inside brace pair again.
Show 14 quoted lines
> + const struct field *found =
> + bsearch(&search_key, repo_info_fields,
> + ARRAY_SIZE(repo_info_fields), sizeof(struct field),
> + repo_info_fields_cmp);
> return found ? found->add_field_callback : NULL;
> }
>
> @@ -77,7 +74,8 @@ static int qsort_strcmp(const void *va, const void *vb)
> return strcmp(a, b);
> }
>
> -static void print_fields(int argc, const char **argv, struct repository *repo) {
> +static void print_fields(int argc, const char **argv, struct repository *repo)
> +{Ditto.
Show 15 quoted lines
> const char *last = "";
> struct strbuf buf;
> strbuf_init(&buf, 256);
> @@ -107,19 +105,14 @@ static void print_fields(int argc, const char **argv, struct repository *repo) {
> strbuf_release(&buf);
> }
>
> -static int repo_info(int argc,
> - const char **argv,
> - const char *prefix UNUSED,
> +static int repo_info(int argc, const char **argv, const char *prefix UNUSED,
> struct repository *repo)
> {
> -
> - print_fields(argc - 1 , argv + 1, repo);Extra SP before ",".
Show 11 quoted lines
> + print_fields(argc - 1, argv + 1, repo);
> return 0;
> }
>
> -int cmd_repo(int argc,
> - const char **argv,
> - const char *prefix,
> +int cmd_repo(int argc, const char **argv, const char *prefix,
> struct repository *repo)
> {
> parse_opt_subcommand_fn *fn = NULL;