All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org, Toon Claes <toon@iotcl.com>
Subject: Re: [PATCH 0/4] for-each-ref: introduce seeking functionality via '--skip-until'
Date: Tue, 01 Jul 2025 14:37:40 -0700	[thread overview]
Message-ID: <xmqqms9niod7.fsf@gitster.g> (raw)
In-Reply-To: <20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com> (Karthik Nayak's message of "Tue, 01 Jul 2025 17:03:26 +0200")

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;
 				/*

  parent reply	other threads:[~2025-07-01 21:37 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-01 15:03 [PATCH 0/4] for-each-ref: introduce seeking functionality via '--skip-until' Karthik Nayak
2025-07-01 15:03 ` [PATCH 1/4] refs: expose `ref_iterator` via 'refs.h' Karthik Nayak
2025-07-01 15:03 ` [PATCH 2/4] ref-cache: remove unused function 'find_ref_entry()' Karthik Nayak
2025-07-14 15:46   ` Junio C Hamano
2025-07-01 15:03 ` [PATCH 3/4] refs: selectively set prefix in the seek functions Karthik Nayak
2025-07-03  5:55   ` Patrick Steinhardt
2025-07-03  9:40     ` Karthik Nayak
2025-07-01 15:03 ` [PATCH 4/4] for-each-ref: introduce a '--skip-until' option Karthik Nayak
2025-07-03  5:55   ` Patrick Steinhardt
2025-07-03 10:02     ` Karthik Nayak
2025-07-03 10:59       ` Patrick Steinhardt
2025-07-01 17:08 ` [PATCH 0/4] for-each-ref: introduce seeking functionality via '--skip-until' Junio C Hamano
2025-07-02 16:45   ` Karthik Nayak
2025-07-01 21:37 ` Junio C Hamano [this message]
2025-07-02 18:19   ` Karthik Nayak
2025-07-03  8:41     ` Karthik Nayak
2025-07-02 14:14 ` Phillip Wood
2025-07-02 20:33   ` Karthik Nayak
2025-07-03  5:18     ` Patrick Steinhardt
2025-07-03  5:56       ` Junio C Hamano
2025-07-03  8:19         ` Patrick Steinhardt
2025-07-03  8:48           ` Karthik Nayak
2025-07-04 13:02 ` [PATCH v2 " Karthik Nayak
2025-07-04 13:02   ` [PATCH v2 1/4] refs: expose `ref_iterator` via 'refs.h' Karthik Nayak
2025-07-04 13:02   ` [PATCH v2 2/4] ref-cache: remove unused function 'find_ref_entry()' Karthik Nayak
2025-07-04 13:02   ` [PATCH v2 3/4] refs: selectively set prefix in the seek functions Karthik Nayak
2025-07-04 13:02   ` [PATCH v2 4/4] for-each-ref: introduce a '--skip-until' option Karthik Nayak
2025-07-07 15:30     ` Junio C Hamano
2025-07-07 18:31       ` Karthik Nayak
2025-07-04 13:41   ` [PATCH v2 0/4] for-each-ref: introduce seeking functionality via '--skip-until' Andreas Schwab
2025-07-04 14:02     ` Karthik Nayak
2025-07-04 14:52       ` Andreas Schwab
2025-07-04 14:58         ` Karthik Nayak
2025-07-04 15:55           ` Andreas Schwab
2025-07-07  8:52             ` Karthik Nayak
2025-07-04 16:39       ` Junio C Hamano
2025-07-07  8:59         ` Karthik Nayak
2025-07-07  9:45           ` Phillip Wood
2025-07-08 11:39             ` Karthik Nayak
2025-07-08 13:47 ` [PATCH v3 0/4] for-each-ref: introduce seeking functionality via '--start-after' Karthik Nayak
2025-07-08 13:47   ` [PATCH v3 1/4] refs: expose `ref_iterator` via 'refs.h' Karthik Nayak
2025-07-08 13:47   ` [PATCH v3 2/4] ref-cache: remove unused function 'find_ref_entry()' Karthik Nayak
2025-07-08 13:47   ` [PATCH v3 3/4] refs: selectively set prefix in the seek functions Karthik Nayak
2025-07-10  6:44     ` Patrick Steinhardt
2025-07-11  9:44       ` Karthik Nayak
2025-07-14 16:09       ` Junio C Hamano
2025-07-15  9:49         ` Karthik Nayak
2025-07-15 16:35           ` Junio C Hamano
2025-07-16 14:40             ` Karthik Nayak
2025-07-16 15:39               ` Junio C Hamano
2025-07-16 20:02               ` Junio C Hamano
2025-07-17  9:01                 ` Karthik Nayak
2025-07-17 17:31                   ` Junio C Hamano
2025-07-08 13:47   ` [PATCH v3 4/4] for-each-ref: introduce a '--start-after' option Karthik Nayak
2025-07-08 20:25     ` Junio C Hamano
2025-07-09  9:53       ` Karthik Nayak
2025-07-11 16:18 ` [PATCH v4 0/4] for-each-ref: introduce seeking functionality via '--start-after' Karthik Nayak
2025-07-11 16:18   ` [PATCH v4 1/4] refs: expose `ref_iterator` via 'refs.h' Karthik Nayak
2025-07-11 16:18   ` [PATCH v4 2/4] ref-cache: remove unused function 'find_ref_entry()' Karthik Nayak
2025-07-11 16:18   ` [PATCH v4 3/4] refs: selectively set prefix in the seek functions Karthik Nayak
2025-07-14 10:34     ` Christian Couder
2025-07-15  8:19       ` Karthik Nayak
2025-07-11 16:18   ` [PATCH v4 4/4] for-each-ref: introduce a '--start-after' option Karthik Nayak
2025-07-14 16:04     ` Christian Couder
2025-07-14 16:42       ` Junio C Hamano
2025-07-15  8:42       ` Karthik Nayak
2025-07-14 16:34   ` [PATCH v4 0/4] for-each-ref: introduce seeking functionality via '--start-after' Christian Couder
2025-07-14 16:49     ` Junio C Hamano
2025-07-15  9:49       ` Karthik Nayak
2025-07-15 11:28 ` [PATCH v5 0/5] " Karthik Nayak
2025-07-15 11:28   ` [PATCH v5 1/5] refs: expose `ref_iterator` via 'refs.h' Karthik Nayak
2025-07-15 11:28   ` [PATCH v5 2/5] ref-cache: remove unused function 'find_ref_entry()' Karthik Nayak
2025-07-17 14:48     ` Junio C Hamano
2025-07-17 19:31       ` Karthik Nayak
2025-07-17 20:32         ` Junio C Hamano
2025-07-15 11:28   ` [PATCH v5 3/5] refs: selectively set prefix in the seek functions Karthik Nayak
2025-07-17  2:09     ` Jeff King
2025-07-17 19:49       ` Karthik Nayak
2025-07-17 21:55         ` Jeff King
2025-07-15 11:28   ` [PATCH v5 4/5] ref-filter: remove unnecessary else clause Karthik Nayak
2025-07-15 11:28   ` [PATCH v5 5/5] for-each-ref: introduce a '--start-after' option Karthik Nayak
2025-07-17 15:31     ` Junio C Hamano
2025-07-22  8:07       ` Karthik Nayak
2025-07-15 19:00   ` [PATCH v5 0/5] for-each-ref: introduce seeking functionality via '--start-after' Junio C Hamano
2025-07-17  1:19     ` Kyle Lippincott
2025-07-17  1:54       ` Jeff King
2025-07-17 17:08         ` Kyle Lippincott
2025-07-17 19:26           ` Karthik Nayak
2025-07-17 19:35             ` Kyle Lippincott
2025-07-17 22:09               ` Jeff King
2025-07-17 22:16                 ` Jeff King
2025-07-21 14:27                 ` Karthik Nayak
2025-07-21 21:22                   ` Jeff King
2025-07-22  8:44                     ` Karthik Nayak
2025-07-17 22:21             ` Junio C Hamano
2025-07-23 21:51   ` [PATCH] ref-iterator-seek: correctly initialize the prefix_state for a new level Junio C Hamano
2025-07-23 21:57     ` Kyle Lippincott
2025-07-23 23:52     ` Jeff King
2025-07-24  8:12     ` Karthik Nayak
2025-07-24 17:01       ` Junio C Hamano
2025-07-24 22:11         ` [PATCH] ref-cache: set prefix_state when seeking Karthik Nayak
2025-07-24 22:30           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqms9niod7.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    --cc=toon@iotcl.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.