Re: [PATCH 0/4] for-each-ref: introduce seeking functionality via '--skip-until'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 1, 2025, 21:37 UTC
- Message-ID
- <xmqqms9niod7.fsf@gitster.g>
- In-Reply-To
- <20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com>
Offtopic. After applying this topic, I asked clang-format if it wants to change anything.
$ git clang-format --diff $(git merge-base HEAD master)
The result was disasterous. Can "clang-format --diff" mode be taught a bit more focused to avoid touching existing entries in the same array (in this case opts[] that has tons of options for the "git for-each-ref" command), when only one new entry was added, I wonder?
Also I am not impressed by the change it made to the code that is commented out (in refs.h).
Line wrapping it did to refs_ref_iterator_begin() is an improvement, but those to ref_iterator_seek() and do_for_each_ref_iterator() are unnecessary (both of these were more readble in the original).
Even though I found its output better for Toon's "last-modified" changes, I am not impressed by what clang-format suggested for this series.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 543013cd11..39056557d4 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -8,7 +8,7 @@ #include "strbuf.h" #include "strvec.h" -static char const * const for_each_ref_usage[] = { +static char const *const for_each_ref_usage[] = { N_("git for-each-ref [<options>] [<pattern>]"), N_("git for-each-ref [--points-at <object>]"), N_("git for-each-ref [--merged [<commit>]] [--no-merged [<commit>]]"), @@ -33,32 +33,41 @@ int cmd_for_each_ref(int argc, struct option opts[] = { OPT_BIT('s', "shell", &format.quote_style, N_("quote placeholders suitably for shells"), QUOTE_SHELL), - OPT_BIT('p', "perl", &format.quote_style, + OPT_BIT('p', "perl", &format.quote_style, N_("quote placeholders suitably for perl"), QUOTE_PERL), - OPT_BIT(0 , "python", &format.quote_style, - N_("quote placeholders suitably for python"), QUOTE_PYTHON), - OPT_BIT(0 , "tcl", &format.quote_style, + OPT_BIT(0, "python", &format.quote_style, + N_("quote placeholders suitably for python"), + QUOTE_PYTHON), + OPT_BIT(0, "tcl", &format.quote_style, N_("quote placeholders suitably for Tcl"), QUOTE_TCL), - OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty, - N_("do not output a newline after empty formatted refs")), + OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty, + N_("do not output a newline after empty formatted refs")), OPT_GROUP(""), - OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")), - OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")), - OPT_STRING( 0 , "skip-until", &filter.seek, N_("skip-until"), N_("skip references until")), + OPT_INTEGER(0, "count", &format.array_opts.max_count, + N_("show only <n> matched refs")), + OPT_STRING(0, "format", &format.format, N_("format"), + N_("format to use for the output")), + OPT_STRING(0, "skip-until", &filter.seek, N_("skip-until"), + N_("skip references until")), OPT__COLOR(&format.use_color, N_("respect format colors")), OPT_REF_FILTER_EXCLUDE(&filter), OPT_REF_SORT(&sorting_options), - OPT_CALLBACK(0, "points-at", &filter.points_at, - N_("object"), N_("print only refs which points at the given object"), + OPT_CALLBACK(0, "points-at", &filter.points_at, N_("object"), + N_("print only refs which points at the given object"), parse_opt_object_name), OPT_MERGED(&filter, N_("print only refs that are merged")), OPT_NO_MERGED(&filter, N_("print only refs that are not merged")), - OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")), - OPT_NO_CONTAINS(&filter.no_commit, N_("print only refs which don't contain the commit")), - OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")), - OPT_BOOL(0, "stdin", &from_stdin, N_("read reference patterns from stdin")), - OPT_BOOL(0, "include-root-refs", &include_root_refs, N_("also include HEAD ref and pseudorefs")), + OPT_CONTAINS(&filter.with_commit, + N_("print only refs which contain the commit")), + OPT_NO_CONTAINS(&filter.no_commit, + N_("print only refs which don't contain the commit")), + OPT_BOOL(0, "ignore-case", &icase, + N_("sorting and filtering are case insensitive")), + OPT_BOOL(0, "stdin", &from_stdin, + N_("read reference patterns from stdin")), + OPT_BOOL(0, "include-root-refs", &include_root_refs, + N_("also include HEAD ref and pseudorefs")), OPT_END(), }; diff --git a/refs.h b/refs.h index c5e08db0ff..518b17c748 100644 --- a/refs.h +++ b/refs.h @@ -1229,7 +1229,8 @@ int repo_migrate_ref_storage_format(struct repository *repo, * * // Access information about the current reference: * if (!(iter->flags & REF_ISSYMREF)) - * printf("%s is %s\n", iter->refname, oid_to_hex(iter->oid)); + * printf("%s is %s\n", iter->refname, + * oid_to_hex(iter->oid)); * * // If you need to peel the reference: * ref_iterator_peel(iter, &oid); @@ -1284,10 +1285,11 @@ enum do_for_each_ref_flags { * trim that many characters off the beginning of each refname. * The output is ordered by refname. */ -struct ref_iterator *refs_ref_iterator_begin( - struct ref_store *refs, - const char *prefix, const char **exclude_patterns, - int trim, enum do_for_each_ref_flags flags); +struct ref_iterator *refs_ref_iterator_begin(struct ref_store *refs, + const char *prefix, + const char **exclude_patterns, + int trim, + enum do_for_each_ref_flags flags); /* * Advance the iterator to the first or next item and return ITER_OK. @@ -1317,8 +1319,8 @@ int ref_iterator_advance(struct ref_iterator *ref_iterator); * * Returns 0 on success, a negative error code otherwise. */ -int ref_iterator_seek(struct ref_iterator *ref_iterator, - const char *seek, int set_prefix); +int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *seek, + int set_prefix); /* * If possible, peel the reference currently being viewed by the @@ -1339,8 +1341,7 @@ void ref_iterator_free(struct ref_iterator *ref_iterator); * adapter between the callback style of reference iteration and the * iterator style. */ -int do_for_each_ref_iterator(struct ref_iterator *iter, - each_ref_fn fn, void *cb_data); - +int do_for_each_ref_iterator(struct ref_iterator *iter, each_ref_fn fn, + void *cb_data); #endif /* REFS_H */ diff --git a/refs/iterator.c b/refs/iterator.c index 1f99045d40..2b7f019c3e 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -15,8 +15,8 @@ int ref_iterator_advance(struct ref_iterator *ref_iterator) return ref_iterator->vtable->advance(ref_iterator); } -int ref_iterator_seek(struct ref_iterator *ref_iterator, - const char *seek, int set_prefix) +int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *seek, + int set_prefix) { return ref_iterator->vtable->seek(ref_iterator, seek, set_prefix); } @@ -57,8 +57,7 @@ static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator UNUSED) } static int empty_ref_iterator_seek(struct ref_iterator *ref_iterator UNUSED, - const char *seek UNUSED, - int set_prefix UNUSED) + const char *seek UNUSED, int set_prefix UNUSED) { return 0; } diff --git a/refs/ref-cache.c b/refs/ref-cache.c index 656e6cd9ff..b812520dc7 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -525,7 +525,8 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator, level->dir = dir; level->index = -1; } else { - /* reduce the index so the leaf node is iterated over */ + /* reduce the index so the leaf node is iterated + * over */ if (cmp <= 0 && !slash) level->index = idx - 1; /*