[PATCH v2 0/3] refspec: centralize refspec-related logic

[PATCH v2 0/3] refspec: centralize refspec-related logic

[Meet Soni]

Thank you for reviewing :)

I've added documentation comments for various function signatures to
better understand what they do.

Meet Soni (3):
  refspec: relocate omit_name_by_refspec and related functions
  refspec: relocate query related functions
  refspec: relocate apply_refspecs and related funtions

 refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  41 +++++++++++
 remote.c  | 201 -----------------------------------------------------
 remote.h  |  15 ----
 4 files changed, 244 insertions(+), 216 deletions(-)

Range-diff against v1:
1:  97c98f5a38 ! 1:  8e393ea1c2 refspec: relocate omit_name_by_refspec and related functions
    @@ refspec.h: struct strvec;
     + * name matches at least one negative refspec, and 0 otherwise.
     + */
     +int omit_name_by_refspec(const char *name, struct refspec *rs);
    ++
    ++/*
    ++ * Checks whether a name matches a pattern and optionally generates a result.
    ++ * Returns 1 if the name matches the pattern, 0 otherwise.
    ++ */
     +int match_name_with_pattern(const char *key, const char *name,
     +				   const char *value, char **result);
     +
2:  4f0080aad6 ! 2:  ef6edbc15b refspec: relocate query related functions
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +}
     
      ## refspec.h ##
    -@@
    - #ifndef REFSPEC_H
    - #define REFSPEC_H
    +@@ refspec.h: struct refspec_item {
    + 	char *raw;
    + };
      
    -+#include "string-list.h"
    ++struct string_list;
     +
    - #define TAG_REFSPEC "refs/tags/*:refs/tags/*"
    + #define REFSPEC_FETCH 1
    + #define REFSPEC_PUSH 0
      
    - /**
     @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
      int match_name_with_pattern(const char *key, const char *name,
      				   const char *value, char **result);
      
    ++/*
    ++ * Queries a refspec for a match and updates the query item.
    ++ * Returns 0 on success, -1 if no match is found or negative refspec matches.
    ++ */
     +int query_refspecs(struct refspec *rs, struct refspec_item *query);
    ++
    ++/*
    ++ * Queries a refspec for all matches and appends results to the provided string
    ++ * list.
    ++ */
     +void query_refspecs_multiple(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results);
3:  f89328fa66 ! 3:  ea72647439 refspec: relocate apply_refspecs and related funtions
    @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
     + */
     +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     +
    ++/*
    ++ * Applies refspecs to a name and returns the corresponding destination.
    ++ * Returns the destination string if a match is found, NULL otherwise.
    ++ */
     +char *apply_refspecs(struct refspec *rs, const char *name);
     +
      #endif /* REFSPEC_H */
-- 
2.34.1

[PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions

[Meet Soni]

Move the functions `omit_name_by_refspec()`, `refspec_match()`, and
`match_name_with_pattern()` from `remote.c` to `refspec.c`. These
functions focus on refspec matching, so placing them in `refspec.c`
aligns with the separation of concerns. Keep refspec-related logic in
`refspec.c` and remote-specific logic in `remote.c` for better code
organization.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h | 13 +++++++++++++
 remote.c  | 48 ------------------------------------------------
 remote.h  |  6 ------
 4 files changed, 61 insertions(+), 54 deletions(-)

diff --git a/refspec.c b/refspec.c
index 6d86e04442..66989a1d75 100644
--- a/refspec.c
+++ b/refspec.c
@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,
 		}
 	}
 }
+
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result)
+{
+	const char *kstar = strchr(key, '*');
+	size_t klen;
+	size_t ksuffixlen;
+	size_t namelen;
+	int ret;
+	if (!kstar)
+		die(_("key '%s' of pattern had no '*'"), key);
+	klen = kstar - key;
+	ksuffixlen = strlen(kstar + 1);
+	namelen = strlen(name);
+	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
+		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
+	if (ret && value) {
+		struct strbuf sb = STRBUF_INIT;
+		const char *vstar = strchr(value, '*');
+		if (!vstar)
+			die(_("value '%s' of pattern has no '*'"), value);
+		strbuf_add(&sb, value, vstar - value);
+		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
+		strbuf_addstr(&sb, vstar + 1);
+		*result = strbuf_detach(&sb, NULL);
+	}
+	return ret;
+}
+
+static int refspec_match(const struct refspec_item *refspec,
+			 const char *name)
+{
+	if (refspec->pattern)
+		return match_name_with_pattern(refspec->src, name, NULL, NULL);
+
+	return !strcmp(refspec->src, name);
+}
+
+int omit_name_by_refspec(const char *name, struct refspec *rs)
+{
+	int i;
+
+	for (i = 0; i < rs->nr; i++) {
+		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
+			return 1;
+	}
+	return 0;
+}
diff --git a/refspec.h b/refspec.h
index 69d693c87d..891d50b159 100644
--- a/refspec.h
+++ b/refspec.h
@@ -71,4 +71,17 @@ struct strvec;
 void refspec_ref_prefixes(const struct refspec *rs,
 			  struct strvec *ref_prefixes);
 
+/*
+ * Check whether a name matches any negative refspec in rs. Returns 1 if the
+ * name matches at least one negative refspec, and 0 otherwise.
+ */
+int omit_name_by_refspec(const char *name, struct refspec *rs);
+
+/*
+ * Checks whether a name matches a pattern and optionally generates a result.
+ * Returns 1 if the name matches the pattern, 0 otherwise.
+ */
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 0f6fba8562..40c2418065 100644
--- a/remote.c
+++ b/remote.c
@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-static int match_name_with_pattern(const char *key, const char *name,
-				   const char *value, char **result)
-{
-	const char *kstar = strchr(key, '*');
-	size_t klen;
-	size_t ksuffixlen;
-	size_t namelen;
-	int ret;
-	if (!kstar)
-		die(_("key '%s' of pattern had no '*'"), key);
-	klen = kstar - key;
-	ksuffixlen = strlen(kstar + 1);
-	namelen = strlen(name);
-	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
-		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
-	if (ret && value) {
-		struct strbuf sb = STRBUF_INIT;
-		const char *vstar = strchr(value, '*');
-		if (!vstar)
-			die(_("value '%s' of pattern has no '*'"), value);
-		strbuf_add(&sb, value, vstar - value);
-		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
-		strbuf_addstr(&sb, vstar + 1);
-		*result = strbuf_detach(&sb, NULL);
-	}
-	return ret;
-}
-
-static int refspec_match(const struct refspec_item *refspec,
-			 const char *name)
-{
-	if (refspec->pattern)
-		return match_name_with_pattern(refspec->src, name, NULL, NULL);
-
-	return !strcmp(refspec->src, name);
-}
-
-int omit_name_by_refspec(const char *name, struct refspec *rs)
-{
-	int i;
-
-	for (i = 0; i < rs->nr; i++) {
-		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
-			return 1;
-	}
-	return 0;
-}
-
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 {
 	struct ref **tail;
diff --git a/remote.h b/remote.h
index bda10dd5c8..0d109fa9c9 100644
--- a/remote.h
+++ b/remote.h
@@ -261,12 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-/*
- * Check whether a name matches any negative refspec in rs. Returns 1 if the
- * name matches at least one negative refspec, and 0 otherwise.
- */
-int omit_name_by_refspec(const char *name, struct refspec *rs);
-
 /*
  * Remove all entries in the input list which match any negative refspec in
  * the refspec list.
-- 
2.34.1

[PATCH v2 2/3] refspec: relocate query related functions

[Meet Soni]

Move the functions `query_refspecs()`, `query_refspecs_multiple()` and
`query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These
functions focus on querying refspecs, so centralizing them in `refspec.c`
improves code organization by keeping refspec-related logic in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  16 +++++++
 remote.c  | 122 -----------------------------------------------------
 remote.h  |   1 -
 4 files changed, 139 insertions(+), 123 deletions(-)

diff --git a/refspec.c b/refspec.c
index 66989a1d75..72b3911110 100644
--- a/refspec.c
+++ b/refspec.c
@@ -5,6 +5,7 @@
 #include "gettext.h"
 #include "hash.h"
 #include "hex.h"
+#include "string-list.h"
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
@@ -324,3 +325,125 @@ int omit_name_by_refspec(const char *name, struct refspec *rs)
 	}
 	return 0;
 }
+
+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
+{
+	int i, matched_negative = 0;
+	int find_src = !query->src;
+	struct string_list reversed = STRING_LIST_INIT_DUP;
+	const char *needle = find_src ? query->dst : query->src;
+
+	/*
+	 * Check whether the queried ref matches any negative refpsec. If so,
+	 * then we should ultimately treat this as not matching the query at
+	 * all.
+	 *
+	 * Note that negative refspecs always match the source, but the query
+	 * item uses the destination. To handle this, we apply pattern
+	 * refspecs in reverse to figure out if the query source matches any
+	 * of the negative refspecs.
+	 *
+	 * The first loop finds and expands all positive refspecs
+	 * matched by the queried ref.
+	 *
+	 * The second loop checks if any of the results of the first loop
+	 * match any negative refspec.
+	 */
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		char *expn_name;
+
+		if (refspec->negative)
+			continue;
+
+		/* Note the reversal of src and dst */
+		if (refspec->pattern) {
+			const char *key = refspec->dst ? refspec->dst : refspec->src;
+			const char *value = refspec->src;
+
+			if (match_name_with_pattern(key, needle, value, &expn_name))
+				string_list_append_nodup(&reversed, expn_name);
+		} else if (refspec->matching) {
+			/* For the special matching refspec, any query should match */
+			string_list_append(&reversed, needle);
+		} else if (!refspec->src) {
+			BUG("refspec->src should not be null here");
+		} else if (!strcmp(needle, refspec->src)) {
+			string_list_append(&reversed, refspec->src);
+		}
+	}
+
+	for (i = 0; !matched_negative && i < reversed.nr; i++) {
+		if (omit_name_by_refspec(reversed.items[i].string, rs))
+			matched_negative = 1;
+	}
+
+	string_list_clear(&reversed, 0);
+
+	return matched_negative;
+}
+
+void query_refspecs_multiple(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results)
+{
+	int i;
+	int find_src = !query->src;
+
+	if (find_src && !query->dst)
+		BUG("query_refspecs_multiple: need either src or dst");
+
+	if (query_matches_negative_refspec(rs, query))
+		return;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+		const char *needle = find_src ? query->dst : query->src;
+		char **result = find_src ? &query->src : &query->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result))
+				string_list_append_nodup(results, *result);
+		} else if (!strcmp(needle, key)) {
+			string_list_append(results, value);
+		}
+	}
+}
+
+int query_refspecs(struct refspec *rs, struct refspec_item *query)
+{
+	int i;
+	int find_src = !query->src;
+	const char *needle = find_src ? query->dst : query->src;
+	char **result = find_src ? &query->src : &query->dst;
+
+	if (find_src && !query->dst)
+		BUG("query_refspecs: need either src or dst");
+
+	if (query_matches_negative_refspec(rs, query))
+		return -1;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result)) {
+				query->force = refspec->force;
+				return 0;
+			}
+		} else if (!strcmp(needle, key)) {
+			*result = xstrdup(value);
+			query->force = refspec->force;
+			return 0;
+		}
+	}
+	return -1;
+}
diff --git a/refspec.h b/refspec.h
index 891d50b159..d0788de782 100644
--- a/refspec.h
+++ b/refspec.h
@@ -30,6 +30,8 @@ struct refspec_item {
 	char *raw;
 };
 
+struct string_list;
+
 #define REFSPEC_FETCH 1
 #define REFSPEC_PUSH 0
 
@@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);
 int match_name_with_pattern(const char *key, const char *name,
 				   const char *value, char **result);
 
+/*
+ * Queries a refspec for a match and updates the query item.
+ * Returns 0 on success, -1 if no match is found or negative refspec matches.
+ */
+int query_refspecs(struct refspec *rs, struct refspec_item *query);
+
+/*
+ * Queries a refspec for all matches and appends results to the provided string
+ * list.
+ */
+void query_refspecs_multiple(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 40c2418065..2c46611821 100644
--- a/remote.c
+++ b/remote.c
@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	return ref_map;
 }
 
-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
-{
-	int i, matched_negative = 0;
-	int find_src = !query->src;
-	struct string_list reversed = STRING_LIST_INIT_DUP;
-	const char *needle = find_src ? query->dst : query->src;
-
-	/*
-	 * Check whether the queried ref matches any negative refpsec. If so,
-	 * then we should ultimately treat this as not matching the query at
-	 * all.
-	 *
-	 * Note that negative refspecs always match the source, but the query
-	 * item uses the destination. To handle this, we apply pattern
-	 * refspecs in reverse to figure out if the query source matches any
-	 * of the negative refspecs.
-	 *
-	 * The first loop finds and expands all positive refspecs
-	 * matched by the queried ref.
-	 *
-	 * The second loop checks if any of the results of the first loop
-	 * match any negative refspec.
-	 */
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		char *expn_name;
-
-		if (refspec->negative)
-			continue;
-
-		/* Note the reversal of src and dst */
-		if (refspec->pattern) {
-			const char *key = refspec->dst ? refspec->dst : refspec->src;
-			const char *value = refspec->src;
-
-			if (match_name_with_pattern(key, needle, value, &expn_name))
-				string_list_append_nodup(&reversed, expn_name);
-		} else if (refspec->matching) {
-			/* For the special matching refspec, any query should match */
-			string_list_append(&reversed, needle);
-		} else if (!refspec->src) {
-			BUG("refspec->src should not be null here");
-		} else if (!strcmp(needle, refspec->src)) {
-			string_list_append(&reversed, refspec->src);
-		}
-	}
-
-	for (i = 0; !matched_negative && i < reversed.nr; i++) {
-		if (omit_name_by_refspec(reversed.items[i].string, rs))
-			matched_negative = 1;
-	}
-
-	string_list_clear(&reversed, 0);
-
-	return matched_negative;
-}
-
-static void query_refspecs_multiple(struct refspec *rs,
-				    struct refspec_item *query,
-				    struct string_list *results)
-{
-	int i;
-	int find_src = !query->src;
-
-	if (find_src && !query->dst)
-		BUG("query_refspecs_multiple: need either src or dst");
-
-	if (query_matches_negative_refspec(rs, query))
-		return;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-		const char *needle = find_src ? query->dst : query->src;
-		char **result = find_src ? &query->src : &query->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result))
-				string_list_append_nodup(results, *result);
-		} else if (!strcmp(needle, key)) {
-			string_list_append(results, value);
-		}
-	}
-}
-
-int query_refspecs(struct refspec *rs, struct refspec_item *query)
-{
-	int i;
-	int find_src = !query->src;
-	const char *needle = find_src ? query->dst : query->src;
-	char **result = find_src ? &query->src : &query->dst;
-
-	if (find_src && !query->dst)
-		BUG("query_refspecs: need either src or dst");
-
-	if (query_matches_negative_refspec(rs, query))
-		return -1;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result)) {
-				query->force = refspec->force;
-				return 0;
-			}
-		} else if (!strcmp(needle, key)) {
-			*result = xstrdup(value);
-			query->force = refspec->force;
-			return 0;
-		}
-	}
-	return -1;
-}
-
 char *apply_refspecs(struct refspec *rs, const char *name)
 {
 	struct refspec_item query;
diff --git a/remote.h b/remote.h
index 0d109fa9c9..f3da64dc41 100644
--- a/remote.h
+++ b/remote.h
@@ -267,7 +267,6 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);
  */
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
 
-int query_refspecs(struct refspec *rs, struct refspec_item *query);
 char *apply_refspecs(struct refspec *rs, const char *name);
 
 int check_push_refs(struct ref *src, struct refspec *rs);
-- 
2.34.1

[PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions

[Meet Soni]

Move the functions `apply_refspecs()` and `apply_negative_refspecs()`
from `remote.c` to `refspec.c`. These functions focus on applying
refspecs, so centralizing them in `refspec.c` improves code organization
by keeping refspec-related logic in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 32 ++++++++++++++++++++++++++++++++
 refspec.h | 12 ++++++++++++
 remote.c  | 31 -------------------------------
 remote.h  |  8 --------
 4 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/refspec.c b/refspec.c
index 72b3911110..d279d6032a 100644
--- a/refspec.c
+++ b/refspec.c
@@ -9,6 +9,7 @@
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
+#include "remote.h"
 #include "strbuf.h"
 
 /*
@@ -447,3 +448,34 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)
 	}
 	return -1;
 }
+
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
+{
+	struct ref **tail;
+
+	for (tail = &ref_map; *tail; ) {
+		struct ref *ref = *tail;
+
+		if (omit_name_by_refspec(ref->name, rs)) {
+			*tail = ref->next;
+			free(ref->peer_ref);
+			free(ref);
+		} else
+			tail = &ref->next;
+	}
+
+	return ref_map;
+}
+
+char *apply_refspecs(struct refspec *rs, const char *name)
+{
+	struct refspec_item query;
+
+	memset(&query, 0, sizeof(struct refspec_item));
+	query.src = (char *)name;
+
+	if (query_refspecs(rs, &query))
+		return NULL;
+
+	return query.dst;
+}
diff --git a/refspec.h b/refspec.h
index d0788de782..231bcfb33e 100644
--- a/refspec.h
+++ b/refspec.h
@@ -100,4 +100,16 @@ void query_refspecs_multiple(struct refspec *rs,
 				    struct refspec_item *query,
 				    struct string_list *results);
 
+/*
+ * Remove all entries in the input list which match any negative refspec in
+ * the refspec list.
+ */
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
+
+/*
+ * Applies refspecs to a name and returns the corresponding destination.
+ * Returns the destination string if a match is found, NULL otherwise.
+ */
+char *apply_refspecs(struct refspec *rs, const char *name);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 2c46611821..641dd1125f 100644
--- a/remote.c
+++ b/remote.c
@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
-{
-	struct ref **tail;
-
-	for (tail = &ref_map; *tail; ) {
-		struct ref *ref = *tail;
-
-		if (omit_name_by_refspec(ref->name, rs)) {
-			*tail = ref->next;
-			free(ref->peer_ref);
-			free(ref);
-		} else
-			tail = &ref->next;
-	}
-
-	return ref_map;
-}
-
-char *apply_refspecs(struct refspec *rs, const char *name)
-{
-	struct refspec_item query;
-
-	memset(&query, 0, sizeof(struct refspec_item));
-	query.src = (char *)name;
-
-	if (query_refspecs(rs, &query))
-		return NULL;
-
-	return query.dst;
-}
-
 int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
 {
 	return query_refspecs(&remote->fetch, refspec);
diff --git a/remote.h b/remote.h
index f3da64dc41..b4bb16af0e 100644
--- a/remote.h
+++ b/remote.h
@@ -261,14 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-/*
- * Remove all entries in the input list which match any negative refspec in
- * the refspec list.
- */
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
-
-char *apply_refspecs(struct refspec *rs, const char *name);
-
 int check_push_refs(struct ref *src, struct refspec *rs);
 int match_push_refs(struct ref *src, struct ref **dst,
 		    struct refspec *rs, int flags);
-- 
2.34.1

Re: [PATCH v2 0/3] refspec: centralize refspec-related logic

[Meet Soni]

On Mon, 27 Jan 2025 at 16:06, Meet Soni <meetsoni3017@gmail.com> wrote:
>
> Thank you for reviewing :)
>
> I've added documentation comments for various function signatures to
> better understand what they do.
>
> Meet Soni (3):
>   refspec: relocate omit_name_by_refspec and related functions
>   refspec: relocate query related functions
>   refspec: relocate apply_refspecs and related funtions
>
>  refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  refspec.h |  41 +++++++++++
>  remote.c  | 201 -----------------------------------------------------
>  remote.h  |  15 ----
>  4 files changed, 244 insertions(+), 216 deletions(-)
>
> Range-diff against v1:
> 1:  97c98f5a38 ! 1:  8e393ea1c2 refspec: relocate omit_name_by_refspec and related functions
>     @@ refspec.h: struct strvec;
>      + * name matches at least one negative refspec, and 0 otherwise.
>      + */
>      +int omit_name_by_refspec(const char *name, struct refspec *rs);
>     ++
>     ++/*
>     ++ * Checks whether a name matches a pattern and optionally generates a result.
>     ++ * Returns 1 if the name matches the pattern, 0 otherwise.
>     ++ */
>      +int match_name_with_pattern(const char *key, const char *name,
>      +                             const char *value, char **result);
>      +
> 2:  4f0080aad6 ! 2:  ef6edbc15b refspec: relocate query related functions
>     @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
>      +}
>
>       ## refspec.h ##
>     -@@
>     - #ifndef REFSPEC_H
>     - #define REFSPEC_H
>     +@@ refspec.h: struct refspec_item {
>     +   char *raw;
>     + };
>
>     -+#include "string-list.h"
>     ++struct string_list;
>      +
>     - #define TAG_REFSPEC "refs/tags/*:refs/tags/*"
>     + #define REFSPEC_FETCH 1
>     + #define REFSPEC_PUSH 0
>
>     - /**
>      @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
>       int match_name_with_pattern(const char *key, const char *name,
>                                    const char *value, char **result);
>
>     ++/*
>     ++ * Queries a refspec for a match and updates the query item.
>     ++ * Returns 0 on success, -1 if no match is found or negative refspec matches.
>     ++ */
>      +int query_refspecs(struct refspec *rs, struct refspec_item *query);
>     ++
>     ++/*
>     ++ * Queries a refspec for all matches and appends results to the provided string
>     ++ * list.
>     ++ */
>      +void query_refspecs_multiple(struct refspec *rs,
>      +                              struct refspec_item *query,
>      +                              struct string_list *results);
> 3:  f89328fa66 ! 3:  ea72647439 refspec: relocate apply_refspecs and related funtions
>     @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
>      + */
>      +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
>      +
>     ++/*
>     ++ * Applies refspecs to a name and returns the corresponding destination.
>     ++ * Returns the destination string if a match is found, NULL otherwise.
>     ++ */
>      +char *apply_refspecs(struct refspec *rs, const char *name);
>      +
>       #endif /* REFSPEC_H */
> --
> 2.34.1
>
Hi everyone,
I realized I forgot to include the In-Reply-To header in my v2 submission,
which would have linked this series to v1. My apologies for the oversight!

For reference, the v1 cover letter can be found here [1]
[1]: https://lore.kernel.org/git/20250122075154.5697-1-meetsoni3017@gmail.com/

Please consider this email as a manual link between the v1 and v2 series.
Thank you for your understanding.

Best regards,
Meet

Re: [PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions

[Junio C Hamano]

Meet Soni <meetsoni3017@gmail.com> writes:

> Move the functions `omit_name_by_refspec()`, `refspec_match()`, and
> `match_name_with_pattern()` from `remote.c` to `refspec.c`. These
> functions focus on refspec matching, so placing them in `refspec.c`
> aligns with the separation of concerns. Keep refspec-related logic in
> `refspec.c` and remote-specific logic in `remote.c` for better code
> organization.
>
> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> ---
> ...
> diff --git a/refspec.h b/refspec.h
> index 69d693c87d..891d50b159 100644
> --- a/refspec.h
> +++ b/refspec.h
> @@ -71,4 +71,17 @@ struct strvec;
>  void refspec_ref_prefixes(const struct refspec *rs,
>  			  struct strvec *ref_prefixes);

Back when these functions were mere local helper functions in
remote.c, their name being less descriptive of what they do may have
been OK (because readers have more context to understand them), but
when we make it a part of a public API, we should re-evaluate if
their names are good enough.

> +/*
> + * Check whether a name matches any negative refspec in rs. Returns 1 if the
> + * name matches at least one negative refspec, and 0 otherwise.
> + */
> +int omit_name_by_refspec(const char *name, struct refspec *rs);

Imagine you found this description in the header file and are trying
to figure out if it helps you writing the feature you are adding to
Git.  Are the above description and the name of the function useful
enough to you?

The first question that came to my mind was "what is exactly a 'name'?"

In the context of the original, the caller iterates over a list of
"struct ref" and feeds the "name" member of the struct, but this
caller does not even have to know it is getting a part of "struct
ref"; it only cares about its parameter being a character string.

In that context, is "name" the best identifer you can give to this
parameter?  At least calling it "refname" might boost the signal the
name gives to the reader a bit better (and it is in line with how
refs.h calls these things).

Another thing to consider is if the comment describes the purpose of
the function well, instead of just rephrasing what its
implementation does.  What does it mean to return true iff there is
even one negative refspec that matches?  What is the conceivable use
a caller would want to use such a function?

As I said, calling it "omit" was probably OK in the context of the
original file, but it was already sloppy.  This function merely
provides one bit of information (i.e. "does it match any nagative
refspec---Yes or No?"), and it is up to its caller how to use that
piece of information form.

One of its callers, apply_negative_refspecs(), happens to use it to
filter a list of "struct ref" it received from its caller to drop
the refs from the list that match any negative refspec, but the
other existing caller does not even filter or omit anything from a
collection it has.

My personal preference is to do this kind of change in two separate
patches:

 (1) as a preliminary clean-up, we rename functions and their
     parameters in the original place; if needed, add clarifying
     comments.

 (2) move the resulting functions with the comments to their new
     home.

If these two step conversions results in

extern int refname_matches_negative_refspec_item
	(const char *refname, struct refspec *refspec);

I suspect that it is clear enough that there is no need for any
extra comment to explain what it does.

> +/*
> + * Checks whether a name matches a pattern and optionally generates a result.
> + * Returns 1 if the name matches the pattern, 0 otherwise.
> + */
> +int match_name_with_pattern(const char *key, const char *name,
> +				   const char *value, char **result);
> +

As this is merely moved from an existing header, I am tempted to say
I'll leave it as an exercise to the readers to improve this one, as
improving it is outside the scope of this work.

Some hints for those who want to tackle the clean-up for extra
points, perhaps after the dust settles from this series.

The "pattern" in the name refers to the src side of a globbing
refspec and is passed in the parameter "key", so we are calling the
same thing in three different names, which is already triply bad.

"optionally generates a result" does not convey any meaning outside
the context of the original, as it does not even talk about what
computation is creating the result.  It does not even say what
controls the optionality---without reading the implementation, it is
likely your readers would assume passing NULL to result is all it
takes to skip that optional feature, but that is not the case.

If I understand correctly, here is what this one does.

   It takes the source side of a globbing refspec item (e.g.
   "refs/heads/*" in "refs/heads/*:refs/remotes/origin/*"), a
   refname that might match the glob pattern, the destination side
   of the refspec item (e.g. "refs/remotes/origin/*" in the same
   example), and a pointer that points at a variable to receive the
   result.  If the source pattern matches the given refname, apply
   the source-to-destination mapping rule to compute the resulting
   destination refname and store it in the result.

   The destination side is optional; if you do not need to map the
   refname to another refname, but are merely interested if the
   refname matches the glob pattern, you can pass NULL and result
   location is not touched.

   In either case, returns true iff the source side of the globbing
   refspec item matches the given refname.

So "name" in the function name should probably become a bit
narrower, like "refname".  Also the names of its parameters need to
be better thought out.

Re: [PATCH v2 0/3] refspec: centralize refspec-related logic

[Junio C Hamano]

Meet Soni <meetsoni3017@gmail.com> writes:

> Thank you for reviewing :)
>
> I've added documentation comments for various function signatures to
> better understand what they do.

Before saying all that, please help those who haven't read the
previous round (which wasn't even v1 IIRC but RFC and may have been
skipped by some potential reviewers) by summarizing what this series
is about.  For other's convenience, here is a key excerpt from the
cover letter of the previous iteration:

    As Patrick pointed out in [1], the logic related to refspec is currently
    split across multiple headers. This patch series addresses that by
    relocating refspec-related logic from remote to refspec for improved
    cohesion.

While I was working on an unrelated issue, I noticed that there is
one function, "extern int valid_remote_name(const char *);" declared
in <refspec.h> which is only about a remote and should probably be
moved to <remote.h>; cleaning it up does not have to be part of this
series, but since you are doing a similar clean-up effort, I thought
you would want to be aware of it.

Thanks.

Re: [PATCH v2 2/3] refspec: relocate query related functions

[Junio C Hamano]

Meet Soni <meetsoni3017@gmail.com> writes:

> Move the functions `query_refspecs()`, `query_refspecs_multiple()` and
> `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These
> functions focus on querying refspecs, so centralizing them in `refspec.c`
> improves code organization by keeping refspec-related logic in one place.

I think query_matches_negative_refspec() is appropriate named (not
that it matters much, as it becomes a mere private helper in the
file), unlike the ones in the first patch that are suboptimally
named.  query_refspecs() could probalby lose the plural 's' at the
end---there is only single refspec, which is a collection of refspec
items, involved and it makes a single query---but otherwise it also
has an appropriate name (this matters a bit more, but not that much,
as it was already public).

query_refspecs_multiple() is not a great name, though.  It does not
convey what is multiple.  Does it make multiple questions in one go?
Does it ask a question that can have multiple answers?

> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> ---
>  refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  refspec.h |  16 +++++++
>  remote.c  | 122 -----------------------------------------------------
>  remote.h  |   1 -
>  4 files changed, 139 insertions(+), 123 deletions(-)

> diff --git a/refspec.h b/refspec.h
> index 891d50b159..d0788de782 100644
> --- a/refspec.h
> +++ b/refspec.h
> @@ -30,6 +30,8 @@ struct refspec_item {
>  	char *raw;
>  };
>  
> +struct string_list;
> +
>  #define REFSPEC_FETCH 1
>  #define REFSPEC_PUSH 0
>  
> @@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);
>  int match_name_with_pattern(const char *key, const char *name,
>  				   const char *value, char **result);
>  
> +/*
> + * Queries a refspec for a match and updates the query item.
> + * Returns 0 on success, -1 if no match is found or negative refspec matches.
> + */
> +int query_refspecs(struct refspec *rs, struct refspec_item *query);

This one now has an excellent comment.  Great job.

Thanks.

Re: [PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions

[Junio C Hamano]

Meet Soni <meetsoni3017@gmail.com> writes:

> +/*
> + * Remove all entries in the input list which match any negative refspec in
> + * the refspec list.
> + */
> +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);

Excellent.  

    ... it is merely moved from the original so it is not entirely
    your achievement, but still, this is good.

> +/*
> + * Applies refspecs to a name and returns the corresponding destination.
> + * Returns the destination string if a match is found, NULL otherwise.
> + */
> +char *apply_refspecs(struct refspec *rs, const char *name);

Explaining a function whose name has "apply" with a comment that
uses "apply" as the verb does not add as much information as a
comment with a bit rephrased explanation.  What does it mean to
"apply refspec to a name" in the context of this function?

Re: [PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions

[Meet Soni]

On Mon, 27 Jan 2025 at 22:51, Junio C Hamano <gitster@pobox.com> wrote:
>
> Meet Soni <meetsoni3017@gmail.com> writes:
>
> > Move the functions `omit_name_by_refspec()`, `refspec_match()`, and
> > `match_name_with_pattern()` from `remote.c` to `refspec.c`. These
> > functions focus on refspec matching, so placing them in `refspec.c`
> > aligns with the separation of concerns. Keep refspec-related logic in
> > `refspec.c` and remote-specific logic in `remote.c` for better code
> > organization.
> >
> > Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> > ---
> > ...
> > diff --git a/refspec.h b/refspec.h
> > index 69d693c87d..891d50b159 100644
> > --- a/refspec.h
> > +++ b/refspec.h
> > @@ -71,4 +71,17 @@ struct strvec;
> >  void refspec_ref_prefixes(const struct refspec *rs,
> >                         struct strvec *ref_prefixes);
>
> Back when these functions were mere local helper functions in
> remote.c, their name being less descriptive of what they do may have
> been OK (because readers have more context to understand them), but
> when we make it a part of a public API, we should re-evaluate if
> their names are good enough.
>
> > +/*
> > + * Check whether a name matches any negative refspec in rs. Returns 1 if the
> > + * name matches at least one negative refspec, and 0 otherwise.
> > + */
> > +int omit_name_by_refspec(const char *name, struct refspec *rs);
>
> Imagine you found this description in the header file and are trying
> to figure out if it helps you writing the feature you are adding to
> Git.  Are the above description and the name of the function useful
> enough to you?
>
> The first question that came to my mind was "what is exactly a 'name'?"
>
> In the context of the original, the caller iterates over a list of
> "struct ref" and feeds the "name" member of the struct, but this
> caller does not even have to know it is getting a part of "struct
> ref"; it only cares about its parameter being a character string.
>
> In that context, is "name" the best identifer you can give to this
> parameter?  At least calling it "refname" might boost the signal the
> name gives to the reader a bit better (and it is in line with how
> refs.h calls these things).
>
> Another thing to consider is if the comment describes the purpose of
> the function well, instead of just rephrasing what its
> implementation does.  What does it mean to return true iff there is
> even one negative refspec that matches?  What is the conceivable use
> a caller would want to use such a function?
>
> As I said, calling it "omit" was probably OK in the context of the
> original file, but it was already sloppy.  This function merely
> provides one bit of information (i.e. "does it match any nagative
> refspec---Yes or No?"), and it is up to its caller how to use that
> piece of information form.
>
> One of its callers, apply_negative_refspecs(), happens to use it to
> filter a list of "struct ref" it received from its caller to drop
> the refs from the list that match any negative refspec, but the
> other existing caller does not even filter or omit anything from a
> collection it has.
>
> My personal preference is to do this kind of change in two separate
> patches:
>
>  (1) as a preliminary clean-up, we rename functions and their
>      parameters in the original place; if needed, add clarifying
>      comments.
>
>  (2) move the resulting functions with the comments to their new
>      home.
>
> If these two step conversions results in
>
> extern int refname_matches_negative_refspec_item
>         (const char *refname, struct refspec *refspec);
>
> I suspect that it is clear enough that there is no need for any
> extra comment to explain what it does.
>
Makes sense. I'll implement this in the upcoming version of this patch.

Since I’ve already prepared a patch for moving the function in the current
series, I’ll add a commit to handle the renaming and changing comments.

> > +/*
> > + * Checks whether a name matches a pattern and optionally generates a result.
> > + * Returns 1 if the name matches the pattern, 0 otherwise.
> > + */
> > +int match_name_with_pattern(const char *key, const char *name,
> > +                                const char *value, char **result);
> > +
>
> As this is merely moved from an existing header, I am tempted to say
> I'll leave it as an exercise to the readers to improve this one, as
> improving it is outside the scope of this work.
>
> Some hints for those who want to tackle the clean-up for extra
> points, perhaps after the dust settles from this series.
>
> The "pattern" in the name refers to the src side of a globbing
> refspec and is passed in the parameter "key", so we are calling the
> same thing in three different names, which is already triply bad.
>
> "optionally generates a result" does not convey any meaning outside
> the context of the original, as it does not even talk about what
> computation is creating the result.  It does not even say what
> controls the optionality---without reading the implementation, it is
> likely your readers would assume passing NULL to result is all it
> takes to skip that optional feature, but that is not the case.
>
> If I understand correctly, here is what this one does.
>
>    It takes the source side of a globbing refspec item (e.g.
>    "refs/heads/*" in "refs/heads/*:refs/remotes/origin/*"), a
>    refname that might match the glob pattern, the destination side
>    of the refspec item (e.g. "refs/remotes/origin/*" in the same
>    example), and a pointer that points at a variable to receive the
>    result.  If the source pattern matches the given refname, apply
>    the source-to-destination mapping rule to compute the resulting
>    destination refname and store it in the result.
>
>    The destination side is optional; if you do not need to map the
>    refname to another refname, but are merely interested if the
>    refname matches the glob pattern, you can pass NULL and result
>    location is not touched.
>
>    In either case, returns true iff the source side of the globbing
>    refspec item matches the given refname.
>
> So "name" in the function name should probably become a bit
> narrower, like "refname".  Also the names of its parameters need to
> be better thought out.

I agree that the function and its parameters could be improved for clarity.
Since you mentioned leaving it as an exercise for readers, I’m happy to
take it up and write a follow-up patch to address these issues after
finishing the current series, if that works.

Re: [PATCH v2 0/3] refspec: centralize refspec-related logic

[Meet Soni]

On Mon, 27 Jan 2025 at 23:40, Junio C Hamano <gitster@pobox.com> wrote:
>
> Meet Soni <meetsoni3017@gmail.com> writes:
>
> > Thank you for reviewing :)
> >
> > I've added documentation comments for various function signatures to
> > better understand what they do.
>
> Before saying all that, please help those who haven't read the
> previous round (which wasn't even v1 IIRC but RFC and may have been
> skipped by some potential reviewers) by summarizing what this series
> is about.  For other's convenience, here is a key excerpt from the
> cover letter of the previous iteration:
>
>     As Patrick pointed out in [1], the logic related to refspec is currently
>     split across multiple headers. This patch series addresses that by
>     relocating refspec-related logic from remote to refspec for improved
>     cohesion.
>
Understood.

> While I was working on an unrelated issue, I noticed that there is
> one function, "extern int valid_remote_name(const char *);" declared
> in <refspec.h> which is only about a remote and should probably be
> moved to <remote.h>; cleaning it up does not have to be part of this
> series, but since you are doing a similar clean-up effort, I thought
> you would want to be aware of it.
>
> Thanks.
Thank you for pointing this out. I’ll be happy to write up a patch after
this series is done.

Re: [PATCH v2 2/3] refspec: relocate query related functions

[Meet Soni]

On Tue, 28 Jan 2025 at 00:55, Junio C Hamano <gitster@pobox.com> wrote:
>
> Meet Soni <meetsoni3017@gmail.com> writes:
>
> > Move the functions `query_refspecs()`, `query_refspecs_multiple()` and
> > `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These
> > functions focus on querying refspecs, so centralizing them in `refspec.c`
> > improves code organization by keeping refspec-related logic in one place.
>
> I think query_matches_negative_refspec() is appropriate named (not
> that it matters much, as it becomes a mere private helper in the
> file), unlike the ones in the first patch that are suboptimally
> named.  query_refspecs() could probalby lose the plural 's' at the
> end---there is only single refspec, which is a collection of refspec
> items, involved and it makes a single query---but otherwise it also
> has an appropriate name (this matters a bit more, but not that much,
> as it was already public).
>
> query_refspecs_multiple() is not a great name, though.  It does not
> convey what is multiple.  Does it make multiple questions in one go?
> Does it ask a question that can have multiple answers?
>
I agree that the original names are ambiguous. query_refspecs_multiple()
is similar to query_refspecs(), but instead of returning the first match, it
collects all matching results.

To improve clarity and consistency, I’d like to propose the following
renames:
    *query_refspecs() -> find_refspec_match()
        `find` better describes its purpose than `query` and `match`
        clarifies that it’s looking for a single result.

    *query_refspecs_multiple() -> find_all_refspec_matches()
        Unlike the previous function, this one collects all matching results
        instead of stopping at the first match. The new name highlights that
        it returns multiple matches.
Let me know what you think!

> > Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
> > ---
> >  refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  refspec.h |  16 +++++++
> >  remote.c  | 122 -----------------------------------------------------
> >  remote.h  |   1 -
> >  4 files changed, 139 insertions(+), 123 deletions(-)
>
> > diff --git a/refspec.h b/refspec.h
> > index 891d50b159..d0788de782 100644
> > --- a/refspec.h
> > +++ b/refspec.h
> > @@ -30,6 +30,8 @@ struct refspec_item {
> >       char *raw;
> >  };
> >
> > +struct string_list;
> > +
> >  #define REFSPEC_FETCH 1
> >  #define REFSPEC_PUSH 0
> >
> > @@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);
> >  int match_name_with_pattern(const char *key, const char *name,
> >                                  const char *value, char **result);
> >
> > +/*
> > + * Queries a refspec for a match and updates the query item.
> > + * Returns 0 on success, -1 if no match is found or negative refspec matches.
> > + */
> > +int query_refspecs(struct refspec *rs, struct refspec_item *query);
>
> This one now has an excellent comment.  Great job.
>
> Thanks.

Re: [PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions

[Meet Soni]

On Tue, 28 Jan 2025 at 01:44, Junio C Hamano <gitster@pobox.com> wrote:
>
> Meet Soni <meetsoni3017@gmail.com> writes:
>
> > +/*
> > + * Applies refspecs to a name and returns the corresponding destination.
> > + * Returns the destination string if a match is found, NULL otherwise.
> > + */
> > +char *apply_refspecs(struct refspec *rs, const char *name);
>
> Explaining a function whose name has "apply" with a comment that
> uses "apply" as the verb does not add as much information as a
> comment with a bit rephrased explanation.  What does it mean to
> "apply refspec to a name" in the context of this function?

The term "apply" was intended to convey the idea of mapping the
refspec with the given name, but it’s more helpful to describe the
function’s behavior more explicitly.

I’ll update the comment in the next version of this series.
Here’s the revised comment I plan to use:

/*
 * Search for a refspec that matches the given name and return the
 * corresponding destination (dst) if a match is found, NULL otherwise.
 */

[PATCH v3 0/5] refspec: centralize refspec-related logic

[Meet Soni]

As Patrick pointed out in [1], the logic related to refspec is currently
split across multiple headers. This patch series addresses that by
renaming and relocating refspec-related logic from remote to refspec for
improved cohesion.

[1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/

Specifically, the following changes have been made:

    Refactoring and renaming functions: Functions such as
    omit_name_by_refspec() have been renamed to better reflect their
    functionality. 

    Relocation of functions: Functions that are primarily responsible
    for refspec related functionality, have been relocated from remote.c
    to refspec.c to maintain a clear separation of concerns.

Thank you for considering this patch.
Meet


Meet Soni (5):
  refactor(remote): rename function omit_name_by_refspec
  refspec: relocate refname_matches_negative_refspec_item
  refactor(remote): rename query_refspecs functions
  refspec: relocate matching related functions
  refspec: relocate apply_refspecs and related funtions

 builtin/push.c   |   2 +-
 builtin/remote.c |   2 +-
 refspec.c        | 203 ++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h        |  37 +++++++++
 remote.c         | 205 +----------------------------------------------
 remote.h         |  15 ----
 6 files changed, 244 insertions(+), 220 deletions(-)

Range-diff against v2:
-:  ---------- > 1:  399e59ff67 refactor(remote): rename function omit_name_by_refspec
1:  8e393ea1c2 ! 2:  4109b2bd1c refspec: relocate omit_name_by_refspec and related functions
    @@ Metadata
     Author: Meet Soni <meetsoni3017@gmail.com>
     
      ## Commit message ##
    -    refspec: relocate omit_name_by_refspec and related functions
    +    refspec: relocate refname_matches_negative_refspec_item
     
    -    Move the functions `omit_name_by_refspec()`, `refspec_match()`, and
    -    `match_name_with_pattern()` from `remote.c` to `refspec.c`. These
    -    functions focus on refspec matching, so placing them in `refspec.c`
    -    aligns with the separation of concerns. Keep refspec-related logic in
    -    `refspec.c` and remote-specific logic in `remote.c` for better code
    -    organization.
    +    Move the functions `refname_matches_negative_refspec_item()`,
    +    `refspec_match()`, and `match_name_with_pattern()` from `remote.c` to
    +    `refspec.c`. These functions focus on refspec matching, so placing them
    +    in `refspec.c` aligns with the separation of concerns. Keep
    +    refspec-related logic in `refspec.c` and remote-specific logic in
    +    `remote.c` for better code organization.
     
         Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
     
    @@ refspec.c: void refspec_ref_prefixes(const struct refspec *rs,
     +	return !strcmp(refspec->src, name);
     +}
     +
    -+int omit_name_by_refspec(const char *name, struct refspec *rs)
    ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
     +{
     +	int i;
     +
     +	for (i = 0; i < rs->nr; i++) {
    -+		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
    ++		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
     +			return 1;
     +	}
     +	return 0;
    @@ refspec.h: struct strvec;
      void refspec_ref_prefixes(const struct refspec *rs,
      			  struct strvec *ref_prefixes);
      
    -+/*
    -+ * Check whether a name matches any negative refspec in rs. Returns 1 if the
    -+ * name matches at least one negative refspec, and 0 otherwise.
    -+ */
    -+int omit_name_by_refspec(const char *name, struct refspec *rs);
    ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
     +
     +/*
     + * Checks whether a name matches a pattern and optionally generates a result.
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	return !strcmp(refspec->src, name);
     -}
     -
    --int omit_name_by_refspec(const char *name, struct refspec *rs)
    +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
     -{
     -	int i;
     -
     -	for (i = 0; i < rs->nr; i++) {
    --		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
    +-		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
     -			return 1;
     -	}
     -	return 0;
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
      struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
      {
      	struct ref **tail;
    -
    - ## remote.h ##
    -@@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
    -  */
    - struct ref *ref_remove_duplicates(struct ref *ref_map);
    - 
    --/*
    -- * Check whether a name matches any negative refspec in rs. Returns 1 if the
    -- * name matches at least one negative refspec, and 0 otherwise.
    -- */
    --int omit_name_by_refspec(const char *name, struct refspec *rs);
    --
    - /*
    -  * Remove all entries in the input list which match any negative refspec in
    -  * the refspec list.
-:  ---------- > 3:  559224864f refactor(remote): rename query_refspecs functions
2:  ef6edbc15b ! 4:  13e49509fc refspec: relocate query related functions
    @@ Metadata
     Author: Meet Soni <meetsoni3017@gmail.com>
     
      ## Commit message ##
    -    refspec: relocate query related functions
    +    refspec: relocate matching related functions
     
    -    Move the functions `query_refspecs()`, `query_refspecs_multiple()` and
    -    `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These
    -    functions focus on querying refspecs, so centralizing them in `refspec.c`
    -    improves code organization by keeping refspec-related logic in one place.
    +    Move the functions `find_refspec_match()`, `find_all_refspec_matches()`
    +    and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.
    +    These functions focus on matching refspecs, so centralizing them in
    +    `refspec.c` improves code organization by keeping refspec-related logic
    +    in one place.
     
         Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
     
    @@ refspec.c
      #include "strvec.h"
      #include "refs.h"
      #include "refspec.h"
    -@@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
    +@@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
      	}
      	return 0;
      }
     +
    -+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
    ++static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i, matched_negative = 0;
     +	int find_src = !query->src;
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	}
     +
     +	for (i = 0; !matched_negative && i < reversed.nr; i++) {
    -+		if (omit_name_by_refspec(reversed.items[i].string, rs))
    ++		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
     +			matched_negative = 1;
     +	}
     +
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	return matched_negative;
     +}
     +
    -+void query_refspecs_multiple(struct refspec *rs,
    ++void find_all_refspec_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results)
     +{
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	int find_src = !query->src;
     +
     +	if (find_src && !query->dst)
    -+		BUG("query_refspecs_multiple: need either src or dst");
    ++		BUG("find_all_refspec_matches: need either src or dst");
     +
    -+	if (query_matches_negative_refspec(rs, query))
    ++	if (find_negative_refspec_match(rs, query))
     +		return;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	}
     +}
     +
    -+int query_refspecs(struct refspec *rs, struct refspec_item *query)
    ++int find_refspec_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i;
     +	int find_src = !query->src;
    @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)
     +	char **result = find_src ? &query->src : &query->dst;
     +
     +	if (find_src && !query->dst)
    -+		BUG("query_refspecs: need either src or dst");
    ++		BUG("find_refspec_match: need either src or dst");
     +
    -+	if (query_matches_negative_refspec(rs, query))
    ++	if (find_negative_refspec_match(rs, query))
     +		return -1;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.h: struct refspec_item {
      #define REFSPEC_FETCH 1
      #define REFSPEC_PUSH 0
      
    -@@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
    +@@ refspec.h: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
      int match_name_with_pattern(const char *key, const char *name,
      				   const char *value, char **result);
      
    @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);
     + * Queries a refspec for a match and updates the query item.
     + * Returns 0 on success, -1 if no match is found or negative refspec matches.
     + */
    -+int query_refspecs(struct refspec *rs, struct refspec_item *query);
    ++int find_refspec_match(struct refspec *rs, struct refspec_item *query);
     +
     +/*
     + * Queries a refspec for all matches and appends results to the provided string
     + * list.
     + */
    -+void query_refspecs_multiple(struct refspec *rs,
    ++void find_all_refspec_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results);
     +
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      	return ref_map;
      }
      
    --static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
    +-static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i, matched_negative = 0;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	}
     -
     -	for (i = 0; !matched_negative && i < reversed.nr; i++) {
    --		if (omit_name_by_refspec(reversed.items[i].string, rs))
    +-		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
     -			matched_negative = 1;
     -	}
     -
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	return matched_negative;
     -}
     -
    --static void query_refspecs_multiple(struct refspec *rs,
    +-static void find_all_refspec_matches(struct refspec *rs,
     -				    struct refspec_item *query,
     -				    struct string_list *results)
     -{
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	int find_src = !query->src;
     -
     -	if (find_src && !query->dst)
    --		BUG("query_refspecs_multiple: need either src or dst");
    +-		BUG("find_all_refspec_matches: need either src or dst");
     -
    --	if (query_matches_negative_refspec(rs, query))
    +-	if (find_negative_refspec_match(rs, query))
     -		return;
     -
     -	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	}
     -}
     -
    --int query_refspecs(struct refspec *rs, struct refspec_item *query)
    +-int find_refspec_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	char **result = find_src ? &query->src : &query->dst;
     -
     -	if (find_src && !query->dst)
    --		BUG("query_refspecs: need either src or dst");
    +-		BUG("find_refspec_match: need either src or dst");
     -
    --	if (query_matches_negative_refspec(rs, query))
    +-	if (find_negative_refspec_match(rs, query))
     -		return -1;
     -
     -	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      char *apply_refspecs(struct refspec *rs, const char *name)
      {
      	struct refspec_item query;
    -
    - ## remote.h ##
    -@@ remote.h: struct ref *ref_remove_duplicates(struct ref *ref_map);
    -  */
    - struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
    - 
    --int query_refspecs(struct refspec *rs, struct refspec_item *query);
    - char *apply_refspecs(struct refspec *rs, const char *name);
    - 
    - int check_push_refs(struct ref *src, struct refspec *rs);
3:  ea72647439 ! 5:  891e01be93 refspec: relocate apply_refspecs and related funtions
    @@ refspec.c
      #include "strbuf.h"
      
      /*
    -@@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
    +@@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query)
      	}
      	return -1;
      }
    @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
     +	for (tail = &ref_map; *tail; ) {
     +		struct ref *ref = *tail;
     +
    -+		if (omit_name_by_refspec(ref->name, rs)) {
    ++		if (refname_matches_negative_refspec_item(ref->name, rs)) {
     +			*tail = ref->next;
     +			free(ref->peer_ref);
     +			free(ref);
    @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
     +	memset(&query, 0, sizeof(struct refspec_item));
     +	query.src = (char *)name;
     +
    -+	if (query_refspecs(rs, &query))
    ++	if (find_refspec_match(rs, &query))
     +		return NULL;
     +
     +	return query.dst;
     +}
     
      ## refspec.h ##
    -@@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
    +@@ refspec.h: void find_all_refspec_matches(struct refspec *rs,
      				    struct refspec_item *query,
      				    struct string_list *results);
      
    @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,
     +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     +
     +/*
    -+ * Applies refspecs to a name and returns the corresponding destination.
    -+ * Returns the destination string if a match is found, NULL otherwise.
    ++ * Search for a refspec that matches the given name and return the
    ++ * corresponding destination (dst) if a match is found, NULL otherwise.
     + */
     +char *apply_refspecs(struct refspec *rs, const char *name);
     +
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	for (tail = &ref_map; *tail; ) {
     -		struct ref *ref = *tail;
     -
    --		if (omit_name_by_refspec(ref->name, rs)) {
    +-		if (refname_matches_negative_refspec_item(ref->name, rs)) {
     -			*tail = ref->next;
     -			free(ref->peer_ref);
     -			free(ref);
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	memset(&query, 0, sizeof(struct refspec_item));
     -	query.src = (char *)name;
     -
    --	if (query_refspecs(rs, &query))
    +-	if (find_refspec_match(rs, &query))
     -		return NULL;
     -
     -	return query.dst;
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -
      int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
      {
    - 	return query_refspecs(&remote->fetch, refspec);
    + 	return find_refspec_match(&remote->fetch, refspec);
     
      ## remote.h ##
     @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
       */
      struct ref *ref_remove_duplicates(struct ref *ref_map);
      
    +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
    +-
     -/*
     - * Remove all entries in the input list which match any negative refspec in
     - * the refspec list.
     - */
     -struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     -
    +-int find_refspec_match(struct refspec *rs, struct refspec_item *query);
     -char *apply_refspecs(struct refspec *rs, const char *name);
     -
      int check_push_refs(struct ref *src, struct refspec *rs);
-- 
2.34.1

[PATCH v3 1/5] refactor(remote): rename function omit_name_by_refspec

[Meet Soni]

Rename the function `omit_name_by_refspec()` to
`refname_matches_negative_refspec_item()` to provide clearer intent.
The previous function name was vague and did not accurately describe its
purpose. By using `refname_matches_negative_refspec_item`, make the
function's purpose more intuitive, clarifying that it checks if a
reference name matches any negative refspec.

Rename function parameters for consistency with existing naming
conventions. Use `refname` instead of `name` to align with terminology
in `refs.h`.

Remove the redundant doc comment since the function name is now
self-explanatory.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 builtin/remote.c | 2 +-
 remote.c         | 8 ++++----
 remote.h         | 6 +-----
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 0435963286..258b8895cd 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -383,7 +383,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 				states->remote->fetch.items[i].raw);
 
 	for (ref = fetch_map; ref; ref = ref->next) {
-		if (omit_name_by_refspec(ref->name, &states->remote->fetch))
+		if (refname_matches_negative_refspec_item(ref->name, &states->remote->fetch))
 			string_list_append(&states->skipped, abbrev_branch(ref->name));
 		else if (!ref->peer_ref || !refs_ref_exists(get_main_ref_store(the_repository), ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
diff --git a/remote.c b/remote.c
index 0f6fba8562..cb70ce6f3b 100644
--- a/remote.c
+++ b/remote.c
@@ -944,12 +944,12 @@ static int refspec_match(const struct refspec_item *refspec,
 	return !strcmp(refspec->src, name);
 }
 
-int omit_name_by_refspec(const char *name, struct refspec *rs)
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
 {
 	int i;
 
 	for (i = 0; i < rs->nr; i++) {
-		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
+		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
 			return 1;
 	}
 	return 0;
@@ -962,7 +962,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	for (tail = &ref_map; *tail; ) {
 		struct ref *ref = *tail;
 
-		if (omit_name_by_refspec(ref->name, rs)) {
+		if (refname_matches_negative_refspec_item(ref->name, rs)) {
 			*tail = ref->next;
 			free(ref->peer_ref);
 			free(ref);
@@ -1021,7 +1021,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
 	}
 
 	for (i = 0; !matched_negative && i < reversed.nr; i++) {
-		if (omit_name_by_refspec(reversed.items[i].string, rs))
+		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
 			matched_negative = 1;
 	}
 
diff --git a/remote.h b/remote.h
index bda10dd5c8..66ee53411d 100644
--- a/remote.h
+++ b/remote.h
@@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-/*
- * Check whether a name matches any negative refspec in rs. Returns 1 if the
- * name matches at least one negative refspec, and 0 otherwise.
- */
-int omit_name_by_refspec(const char *name, struct refspec *rs);
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
 
 /*
  * Remove all entries in the input list which match any negative refspec in
-- 
2.34.1

[PATCH v3 2/5] refspec: relocate refname_matches_negative_refspec_item

[Meet Soni]

Move the functions `refname_matches_negative_refspec_item()`,
`refspec_match()`, and `match_name_with_pattern()` from `remote.c` to
`refspec.c`. These functions focus on refspec matching, so placing them
in `refspec.c` aligns with the separation of concerns. Keep
refspec-related logic in `refspec.c` and remote-specific logic in
`remote.c` for better code organization.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  9 +++++++++
 remote.c  | 48 ------------------------------------------------
 3 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/refspec.c b/refspec.c
index 6d86e04442..b447768304 100644
--- a/refspec.c
+++ b/refspec.c
@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,
 		}
 	}
 }
+
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result)
+{
+	const char *kstar = strchr(key, '*');
+	size_t klen;
+	size_t ksuffixlen;
+	size_t namelen;
+	int ret;
+	if (!kstar)
+		die(_("key '%s' of pattern had no '*'"), key);
+	klen = kstar - key;
+	ksuffixlen = strlen(kstar + 1);
+	namelen = strlen(name);
+	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
+		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
+	if (ret && value) {
+		struct strbuf sb = STRBUF_INIT;
+		const char *vstar = strchr(value, '*');
+		if (!vstar)
+			die(_("value '%s' of pattern has no '*'"), value);
+		strbuf_add(&sb, value, vstar - value);
+		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
+		strbuf_addstr(&sb, vstar + 1);
+		*result = strbuf_detach(&sb, NULL);
+	}
+	return ret;
+}
+
+static int refspec_match(const struct refspec_item *refspec,
+			 const char *name)
+{
+	if (refspec->pattern)
+		return match_name_with_pattern(refspec->src, name, NULL, NULL);
+
+	return !strcmp(refspec->src, name);
+}
+
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
+{
+	int i;
+
+	for (i = 0; i < rs->nr; i++) {
+		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
+			return 1;
+	}
+	return 0;
+}
diff --git a/refspec.h b/refspec.h
index 69d693c87d..584d9c9eb5 100644
--- a/refspec.h
+++ b/refspec.h
@@ -71,4 +71,13 @@ struct strvec;
 void refspec_ref_prefixes(const struct refspec *rs,
 			  struct strvec *ref_prefixes);
 
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
+
+/*
+ * Checks whether a name matches a pattern and optionally generates a result.
+ * Returns 1 if the name matches the pattern, 0 otherwise.
+ */
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index cb70ce6f3b..1da8ec7037 100644
--- a/remote.c
+++ b/remote.c
@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-static int match_name_with_pattern(const char *key, const char *name,
-				   const char *value, char **result)
-{
-	const char *kstar = strchr(key, '*');
-	size_t klen;
-	size_t ksuffixlen;
-	size_t namelen;
-	int ret;
-	if (!kstar)
-		die(_("key '%s' of pattern had no '*'"), key);
-	klen = kstar - key;
-	ksuffixlen = strlen(kstar + 1);
-	namelen = strlen(name);
-	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
-		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
-	if (ret && value) {
-		struct strbuf sb = STRBUF_INIT;
-		const char *vstar = strchr(value, '*');
-		if (!vstar)
-			die(_("value '%s' of pattern has no '*'"), value);
-		strbuf_add(&sb, value, vstar - value);
-		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
-		strbuf_addstr(&sb, vstar + 1);
-		*result = strbuf_detach(&sb, NULL);
-	}
-	return ret;
-}
-
-static int refspec_match(const struct refspec_item *refspec,
-			 const char *name)
-{
-	if (refspec->pattern)
-		return match_name_with_pattern(refspec->src, name, NULL, NULL);
-
-	return !strcmp(refspec->src, name);
-}
-
-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
-{
-	int i;
-
-	for (i = 0; i < rs->nr; i++) {
-		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
-			return 1;
-	}
-	return 0;
-}
-
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 {
 	struct ref **tail;
-- 
2.34.1

[PATCH v3 3/5] refactor(remote): rename query_refspecs functions

[Meet Soni]

Rename `query_refspecs()` to `find_refspec_match` for clarity, as it
finds a single matching refspec.

Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to
better reflect that it collects all matching refspecs instead of
returning just the first match.

Rename `query_matches_negative_refspec()` to
`find_negative_refspec_match` for consistency with the updated naming
convention.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 builtin/push.c |  2 +-
 remote.c       | 20 ++++++++++----------
 remote.h       |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 90de3746b5..e6527b0b04 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -78,7 +78,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
 			.src = matched->name,
 		};
 
-		if (!query_refspecs(&remote->push, &query) && query.dst) {
+		if (!find_refspec_match(&remote->push, &query) && query.dst) {
 			refspec_appendf(refspec, "%s%s:%s",
 					query.force ? "+" : "",
 					query.src, query.dst);
diff --git a/remote.c b/remote.c
index 1da8ec7037..4654bce5d4 100644
--- a/remote.c
+++ b/remote.c
@@ -925,7 +925,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	return ref_map;
 }
 
-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
 {
 	int i, matched_negative = 0;
 	int find_src = !query->src;
@@ -982,7 +982,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
 	return matched_negative;
 }
 
-static void query_refspecs_multiple(struct refspec *rs,
+static void find_all_refspec_matches(struct refspec *rs,
 				    struct refspec_item *query,
 				    struct string_list *results)
 {
@@ -990,9 +990,9 @@ static void query_refspecs_multiple(struct refspec *rs,
 	int find_src = !query->src;
 
 	if (find_src && !query->dst)
-		BUG("query_refspecs_multiple: need either src or dst");
+		BUG("find_all_refspec_matches: need either src or dst");
 
-	if (query_matches_negative_refspec(rs, query))
+	if (find_negative_refspec_match(rs, query))
 		return;
 
 	for (i = 0; i < rs->nr; i++) {
@@ -1013,7 +1013,7 @@ static void query_refspecs_multiple(struct refspec *rs,
 	}
 }
 
-int query_refspecs(struct refspec *rs, struct refspec_item *query)
+int find_refspec_match(struct refspec *rs, struct refspec_item *query)
 {
 	int i;
 	int find_src = !query->src;
@@ -1021,9 +1021,9 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)
 	char **result = find_src ? &query->src : &query->dst;
 
 	if (find_src && !query->dst)
-		BUG("query_refspecs: need either src or dst");
+		BUG("find_refspec_match: need either src or dst");
 
-	if (query_matches_negative_refspec(rs, query))
+	if (find_negative_refspec_match(rs, query))
 		return -1;
 
 	for (i = 0; i < rs->nr; i++) {
@@ -1054,7 +1054,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)
 	memset(&query, 0, sizeof(struct refspec_item));
 	query.src = (char *)name;
 
-	if (query_refspecs(rs, &query))
+	if (find_refspec_match(rs, &query))
 		return NULL;
 
 	return query.dst;
@@ -1062,7 +1062,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)
 
 int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
 {
-	return query_refspecs(&remote->fetch, refspec);
+	return find_refspec_match(&remote->fetch, refspec);
 }
 
 static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
@@ -2487,7 +2487,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED,
 	memset(&query, 0, sizeof(struct refspec_item));
 	query.dst = (char *)refname;
 
-	query_refspecs_multiple(info->rs, &query, &matches);
+	find_all_refspec_matches(info->rs, &query, &matches);
 	if (matches.nr == 0)
 		goto clean_exit; /* No matches */
 
diff --git a/remote.h b/remote.h
index 66ee53411d..f109310eda 100644
--- a/remote.h
+++ b/remote.h
@@ -269,7 +269,7 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
  */
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
 
-int query_refspecs(struct refspec *rs, struct refspec_item *query);
+int find_refspec_match(struct refspec *rs, struct refspec_item *query);
 char *apply_refspecs(struct refspec *rs, const char *name);
 
 int check_push_refs(struct ref *src, struct refspec *rs);
-- 
2.34.1

[PATCH v3 4/5] refspec: relocate matching related functions

[Meet Soni]

Move the functions `find_refspec_match()`, `find_all_refspec_matches()`
and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.
These functions focus on matching refspecs, so centralizing them in
`refspec.c` improves code organization by keeping refspec-related logic
in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  16 +++++++
 remote.c  | 122 -----------------------------------------------------
 3 files changed, 139 insertions(+), 122 deletions(-)

diff --git a/refspec.c b/refspec.c
index b447768304..6634e7765d 100644
--- a/refspec.c
+++ b/refspec.c
@@ -5,6 +5,7 @@
 #include "gettext.h"
 #include "hash.h"
 #include "hex.h"
+#include "string-list.h"
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
@@ -324,3 +325,125 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
 	}
 	return 0;
 }
+
+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
+{
+	int i, matched_negative = 0;
+	int find_src = !query->src;
+	struct string_list reversed = STRING_LIST_INIT_DUP;
+	const char *needle = find_src ? query->dst : query->src;
+
+	/*
+	 * Check whether the queried ref matches any negative refpsec. If so,
+	 * then we should ultimately treat this as not matching the query at
+	 * all.
+	 *
+	 * Note that negative refspecs always match the source, but the query
+	 * item uses the destination. To handle this, we apply pattern
+	 * refspecs in reverse to figure out if the query source matches any
+	 * of the negative refspecs.
+	 *
+	 * The first loop finds and expands all positive refspecs
+	 * matched by the queried ref.
+	 *
+	 * The second loop checks if any of the results of the first loop
+	 * match any negative refspec.
+	 */
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		char *expn_name;
+
+		if (refspec->negative)
+			continue;
+
+		/* Note the reversal of src and dst */
+		if (refspec->pattern) {
+			const char *key = refspec->dst ? refspec->dst : refspec->src;
+			const char *value = refspec->src;
+
+			if (match_name_with_pattern(key, needle, value, &expn_name))
+				string_list_append_nodup(&reversed, expn_name);
+		} else if (refspec->matching) {
+			/* For the special matching refspec, any query should match */
+			string_list_append(&reversed, needle);
+		} else if (!refspec->src) {
+			BUG("refspec->src should not be null here");
+		} else if (!strcmp(needle, refspec->src)) {
+			string_list_append(&reversed, refspec->src);
+		}
+	}
+
+	for (i = 0; !matched_negative && i < reversed.nr; i++) {
+		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
+			matched_negative = 1;
+	}
+
+	string_list_clear(&reversed, 0);
+
+	return matched_negative;
+}
+
+void find_all_refspec_matches(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results)
+{
+	int i;
+	int find_src = !query->src;
+
+	if (find_src && !query->dst)
+		BUG("find_all_refspec_matches: need either src or dst");
+
+	if (find_negative_refspec_match(rs, query))
+		return;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+		const char *needle = find_src ? query->dst : query->src;
+		char **result = find_src ? &query->src : &query->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result))
+				string_list_append_nodup(results, *result);
+		} else if (!strcmp(needle, key)) {
+			string_list_append(results, value);
+		}
+	}
+}
+
+int find_refspec_match(struct refspec *rs, struct refspec_item *query)
+{
+	int i;
+	int find_src = !query->src;
+	const char *needle = find_src ? query->dst : query->src;
+	char **result = find_src ? &query->src : &query->dst;
+
+	if (find_src && !query->dst)
+		BUG("find_refspec_match: need either src or dst");
+
+	if (find_negative_refspec_match(rs, query))
+		return -1;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result)) {
+				query->force = refspec->force;
+				return 0;
+			}
+		} else if (!strcmp(needle, key)) {
+			*result = xstrdup(value);
+			query->force = refspec->force;
+			return 0;
+		}
+	}
+	return -1;
+}
diff --git a/refspec.h b/refspec.h
index 584d9c9eb5..0393643bc8 100644
--- a/refspec.h
+++ b/refspec.h
@@ -30,6 +30,8 @@ struct refspec_item {
 	char *raw;
 };
 
+struct string_list;
+
 #define REFSPEC_FETCH 1
 #define REFSPEC_PUSH 0
 
@@ -80,4 +82,18 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
 int match_name_with_pattern(const char *key, const char *name,
 				   const char *value, char **result);
 
+/*
+ * Queries a refspec for a match and updates the query item.
+ * Returns 0 on success, -1 if no match is found or negative refspec matches.
+ */
+int find_refspec_match(struct refspec *rs, struct refspec_item *query);
+
+/*
+ * Queries a refspec for all matches and appends results to the provided string
+ * list.
+ */
+void find_all_refspec_matches(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 4654bce5d4..858ab39471 100644
--- a/remote.c
+++ b/remote.c
@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	return ref_map;
 }
 
-static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
-{
-	int i, matched_negative = 0;
-	int find_src = !query->src;
-	struct string_list reversed = STRING_LIST_INIT_DUP;
-	const char *needle = find_src ? query->dst : query->src;
-
-	/*
-	 * Check whether the queried ref matches any negative refpsec. If so,
-	 * then we should ultimately treat this as not matching the query at
-	 * all.
-	 *
-	 * Note that negative refspecs always match the source, but the query
-	 * item uses the destination. To handle this, we apply pattern
-	 * refspecs in reverse to figure out if the query source matches any
-	 * of the negative refspecs.
-	 *
-	 * The first loop finds and expands all positive refspecs
-	 * matched by the queried ref.
-	 *
-	 * The second loop checks if any of the results of the first loop
-	 * match any negative refspec.
-	 */
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		char *expn_name;
-
-		if (refspec->negative)
-			continue;
-
-		/* Note the reversal of src and dst */
-		if (refspec->pattern) {
-			const char *key = refspec->dst ? refspec->dst : refspec->src;
-			const char *value = refspec->src;
-
-			if (match_name_with_pattern(key, needle, value, &expn_name))
-				string_list_append_nodup(&reversed, expn_name);
-		} else if (refspec->matching) {
-			/* For the special matching refspec, any query should match */
-			string_list_append(&reversed, needle);
-		} else if (!refspec->src) {
-			BUG("refspec->src should not be null here");
-		} else if (!strcmp(needle, refspec->src)) {
-			string_list_append(&reversed, refspec->src);
-		}
-	}
-
-	for (i = 0; !matched_negative && i < reversed.nr; i++) {
-		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
-			matched_negative = 1;
-	}
-
-	string_list_clear(&reversed, 0);
-
-	return matched_negative;
-}
-
-static void find_all_refspec_matches(struct refspec *rs,
-				    struct refspec_item *query,
-				    struct string_list *results)
-{
-	int i;
-	int find_src = !query->src;
-
-	if (find_src && !query->dst)
-		BUG("find_all_refspec_matches: need either src or dst");
-
-	if (find_negative_refspec_match(rs, query))
-		return;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-		const char *needle = find_src ? query->dst : query->src;
-		char **result = find_src ? &query->src : &query->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result))
-				string_list_append_nodup(results, *result);
-		} else if (!strcmp(needle, key)) {
-			string_list_append(results, value);
-		}
-	}
-}
-
-int find_refspec_match(struct refspec *rs, struct refspec_item *query)
-{
-	int i;
-	int find_src = !query->src;
-	const char *needle = find_src ? query->dst : query->src;
-	char **result = find_src ? &query->src : &query->dst;
-
-	if (find_src && !query->dst)
-		BUG("find_refspec_match: need either src or dst");
-
-	if (find_negative_refspec_match(rs, query))
-		return -1;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result)) {
-				query->force = refspec->force;
-				return 0;
-			}
-		} else if (!strcmp(needle, key)) {
-			*result = xstrdup(value);
-			query->force = refspec->force;
-			return 0;
-		}
-	}
-	return -1;
-}
-
 char *apply_refspecs(struct refspec *rs, const char *name)
 {
 	struct refspec_item query;
-- 
2.34.1

[PATCH v3 5/5] refspec: relocate apply_refspecs and related funtions

[Meet Soni]

Move the functions `apply_refspecs()` and `apply_negative_refspecs()`
from `remote.c` to `refspec.c`. These functions focus on applying
refspecs, so centralizing them in `refspec.c` improves code organization
by keeping refspec-related logic in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 32 ++++++++++++++++++++++++++++++++
 refspec.h | 12 ++++++++++++
 remote.c  | 31 -------------------------------
 remote.h  | 11 -----------
 4 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/refspec.c b/refspec.c
index 6634e7765d..47974e86f0 100644
--- a/refspec.c
+++ b/refspec.c
@@ -9,6 +9,7 @@
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
+#include "remote.h"
 #include "strbuf.h"
 
 /*
@@ -447,3 +448,34 @@ int find_refspec_match(struct refspec *rs, struct refspec_item *query)
 	}
 	return -1;
 }
+
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
+{
+	struct ref **tail;
+
+	for (tail = &ref_map; *tail; ) {
+		struct ref *ref = *tail;
+
+		if (refname_matches_negative_refspec_item(ref->name, rs)) {
+			*tail = ref->next;
+			free(ref->peer_ref);
+			free(ref);
+		} else
+			tail = &ref->next;
+	}
+
+	return ref_map;
+}
+
+char *apply_refspecs(struct refspec *rs, const char *name)
+{
+	struct refspec_item query;
+
+	memset(&query, 0, sizeof(struct refspec_item));
+	query.src = (char *)name;
+
+	if (find_refspec_match(rs, &query))
+		return NULL;
+
+	return query.dst;
+}
diff --git a/refspec.h b/refspec.h
index 0393643bc8..5cbdc5f622 100644
--- a/refspec.h
+++ b/refspec.h
@@ -96,4 +96,16 @@ void find_all_refspec_matches(struct refspec *rs,
 				    struct refspec_item *query,
 				    struct string_list *results);
 
+/*
+ * Remove all entries in the input list which match any negative refspec in
+ * the refspec list.
+ */
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
+
+/*
+ * Search for a refspec that matches the given name and return the
+ * corresponding destination (dst) if a match is found, NULL otherwise.
+ */
+char *apply_refspecs(struct refspec *rs, const char *name);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 858ab39471..ad16d2493d 100644
--- a/remote.c
+++ b/remote.c
@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
-{
-	struct ref **tail;
-
-	for (tail = &ref_map; *tail; ) {
-		struct ref *ref = *tail;
-
-		if (refname_matches_negative_refspec_item(ref->name, rs)) {
-			*tail = ref->next;
-			free(ref->peer_ref);
-			free(ref);
-		} else
-			tail = &ref->next;
-	}
-
-	return ref_map;
-}
-
-char *apply_refspecs(struct refspec *rs, const char *name)
-{
-	struct refspec_item query;
-
-	memset(&query, 0, sizeof(struct refspec_item));
-	query.src = (char *)name;
-
-	if (find_refspec_match(rs, &query))
-		return NULL;
-
-	return query.dst;
-}
-
 int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
 {
 	return find_refspec_match(&remote->fetch, refspec);
diff --git a/remote.h b/remote.h
index f109310eda..b4bb16af0e 100644
--- a/remote.h
+++ b/remote.h
@@ -261,17 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
-
-/*
- * Remove all entries in the input list which match any negative refspec in
- * the refspec list.
- */
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
-
-int find_refspec_match(struct refspec *rs, struct refspec_item *query);
-char *apply_refspecs(struct refspec *rs, const char *name);
-
 int check_push_refs(struct ref *src, struct refspec *rs);
 int match_push_refs(struct ref *src, struct ref **dst,
 		    struct refspec *rs, int flags);
-- 
2.34.1

Re: [PATCH v3 1/5] refactor(remote): rename function omit_name_by_refspec

[Patrick Steinhardt]

On Sat, Feb 01, 2025 at 12:11:58PM +0530, Meet Soni wrote:

Please drop the `refactor()` bit from the commit subject, we don't use
these prefixes here. You also did the same in a later commit.

> Rename the function `omit_name_by_refspec()` to
> `refname_matches_negative_refspec_item()` to provide clearer intent.
> The previous function name was vague and did not accurately describe its
> purpose. By using `refname_matches_negative_refspec_item`, make the
> function's purpose more intuitive, clarifying that it checks if a
> reference name matches any negative refspec.

The new name certainly reads way better, and the changes themselves look

Patrick

Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions

[Patrick Steinhardt]

On Sat, Feb 01, 2025 at 12:12:00PM +0530, Meet Soni wrote:
> Rename `query_refspecs()` to `find_refspec_match` for clarity, as it
> finds a single matching refspec.
> 
> Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to
> better reflect that it collects all matching refspecs instead of
> returning just the first match.
> 
> Rename `query_matches_negative_refspec()` to
> `find_negative_refspec_match` for consistency with the updated naming
> convention.

Okay. The message might've read a tiny bit easier if it was a bulleted
list of renames. E.g.:

    We're about to move a couple of functions related to handling of
    refspecs from "remote.c" into "refspec.c". In preparation for this
    move, rename them to better reflect their intent:

      - `query_refspecs()` becomes `find_refspec_match()` for clarity,
        as it finds a single matching refspec.

    ...

I was wondering a bit about why we rename the static functions, as we
wouldn't have to expose them in a subsequent step anyway. Other than
that I think we should adhere to our coding guidelines with the renamed
public functions:

    The primary data structure that a subsystem 'S' deals with is called
    `struct S`. Functions that operate on `struct S` are named
    `S_<verb>()` and should generally receive a pointer to `struct S` as
    first parameter. E.g.

So:

  - `query_refspecs()` would be renamed to `refspec_find_match()`.

  - `query_refspecs_multiple()` would be renamed to
    `refspec_find_all_matches()`.

  - `find_negative_refspec_match()` would be renamed to
    `refspec_find_negative_match()`.

Patrick

Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions

[Meet Soni]

On Mon, 3 Feb 2025 at 12:16, Patrick Steinhardt <ps@pks.im> wrote:
>
> On Sat, Feb 01, 2025 at 12:12:00PM +0530, Meet Soni wrote:
> > Rename `query_refspecs()` to `find_refspec_match` for clarity, as it
> > finds a single matching refspec.
> >
> > Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to
> > better reflect that it collects all matching refspecs instead of
> > returning just the first match.
> >
> > Rename `query_matches_negative_refspec()` to
> > `find_negative_refspec_match` for consistency with the updated naming
> > convention.
>
> Okay. The message might've read a tiny bit easier if it was a bulleted
> list of renames. E.g.:
>
>     We're about to move a couple of functions related to handling of
>     refspecs from "remote.c" into "refspec.c". In preparation for this
>     move, rename them to better reflect their intent:
>
>       - `query_refspecs()` becomes `find_refspec_match()` for clarity,
>         as it finds a single matching refspec.
>
>     ...
Makes sense.
>
> I was wondering a bit about why we rename the static functions, as we
> wouldn't have to expose them in a subsequent step anyway. Other than
> that I think we should adhere to our coding guidelines with the renamed
> public functions:
Since we were renaming the query_* functions that are exposed, I updated
the static ones as well to maintain naming consistency across related functions.
>
>     The primary data structure that a subsystem 'S' deals with is called
>     `struct S`. Functions that operate on `struct S` are named
>     `S_<verb>()` and should generally receive a pointer to `struct S` as
>     first parameter. E.g.
>
> So:
>
>   - `query_refspecs()` would be renamed to `refspec_find_match()`.
>
>   - `query_refspecs_multiple()` would be renamed to
>     `refspec_find_all_matches()`.
>
>   - `find_negative_refspec_match()` would be renamed to
>     `refspec_find_negative_match()`.
>
> Patrick

Thanks for the review.
Meet

[GSoC][PATCH v4 0/5] refspec: centralize refspec-related logic

[Meet Soni]

Changes since v3:
    - updated commit message.
    - renamed functions as per review.
    - added GSoC mark , since the announcement has been made by google and
      we've started the discussion regarding the same.

Additional context of the series:
    Patrick pointed out in [1], the logic related to refspec is currently
    split across multiple headers. This patch series addresses that by
    renaming and relocating refspec-related logic from remote to refspec for
    improved cohesion.

    [1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/

    Specifically, the following changes have been made:

        Refactoring and renaming functions: Functions such as
        omit_name_by_refspec() have been renamed to better reflect their
        functionality. 

        Relocation of functions: Functions that are primarily responsible
        for refspec related functionality, have been relocated from remote.c
        to refspec.c to maintain a clear separation of concerns.

Meet Soni (5):
  remote: rename function omit_name_by_refspec
  refspec: relocate refname_matches_negative_refspec_item
  remote: rename query_refspecs functions
  refspec: relocate matching related functions
  refspec: relocate apply_refspecs and related funtions

 builtin/push.c   |   2 +-
 builtin/remote.c |   2 +-
 refspec.c        | 203 ++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h        |  37 +++++++++
 remote.c         | 205 +----------------------------------------------
 remote.h         |  15 ----
 6 files changed, 244 insertions(+), 220 deletions(-)

Range-diff against v3:
1:  399e59ff67 ! 1:  1b8606ffcb refactor(remote): rename function omit_name_by_refspec
    @@ Metadata
     Author: Meet Soni <meetsoni3017@gmail.com>
     
      ## Commit message ##
    -    refactor(remote): rename function omit_name_by_refspec
    +    remote: rename function omit_name_by_refspec
     
         Rename the function `omit_name_by_refspec()` to
         `refname_matches_negative_refspec_item()` to provide clearer intent.
2:  4109b2bd1c = 2:  3da817839c refspec: relocate refname_matches_negative_refspec_item
3:  559224864f ! 3:  bad0c43c96 refactor(remote): rename query_refspecs functions
    @@ Metadata
     Author: Meet Soni <meetsoni3017@gmail.com>
     
      ## Commit message ##
    -    refactor(remote): rename query_refspecs functions
    +    remote: rename query_refspecs functions
     
    -    Rename `query_refspecs()` to `find_refspec_match` for clarity, as it
    -    finds a single matching refspec.
    +    Rename functions related to handling refspecs in preparation for their
    +    move from `remote.c` to `refspec.c`. Update their names to better
    +    reflect their intent:
     
    -    Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to
    -    better reflect that it collects all matching refspecs instead of
    -    returning just the first match.
    +        - `query_refspecs()` -> `refspec_find_match()` for clarity, as it
    +          finds a single matching refspec.
     
    -    Rename `query_matches_negative_refspec()` to
    -    `find_negative_refspec_match` for consistency with the updated naming
    -    convention.
    +        - `query_refspecs_multiple()` -> `refspec_find_all_matches()` to
    +          better reflect that it collects all matching refspecs instead of
    +          returning just the first match.
    +
    +        - `query_matches_negative_refspec()` ->
    +          `refspec_find_negative_match()` for consistency with the
    +          updated naming convention, even though this static function
    +          didn't strictly require renaming.
     
         Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
     
    @@ builtin/push.c: static void refspec_append_mapped(struct refspec *refspec, const
      		};
      
     -		if (!query_refspecs(&remote->push, &query) && query.dst) {
    -+		if (!find_refspec_match(&remote->push, &query) && query.dst) {
    ++		if (!refspec_find_match(&remote->push, &query) && query.dst) {
      			refspec_appendf(refspec, "%s%s:%s",
      					query.force ? "+" : "",
      					query.src, query.dst);
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      }
      
     -static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
    -+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
    ++static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
      {
      	int i, matched_negative = 0;
      	int find_src = !query->src;
    @@ remote.c: static int query_matches_negative_refspec(struct refspec *rs, struct r
      }
      
     -static void query_refspecs_multiple(struct refspec *rs,
    -+static void find_all_refspec_matches(struct refspec *rs,
    ++static void refspec_find_all_matches(struct refspec *rs,
      				    struct refspec_item *query,
      				    struct string_list *results)
      {
    @@ remote.c: static void query_refspecs_multiple(struct refspec *rs,
      
      	if (find_src && !query->dst)
     -		BUG("query_refspecs_multiple: need either src or dst");
    -+		BUG("find_all_refspec_matches: need either src or dst");
    ++		BUG("refspec_find_all_matches: need either src or dst");
      
     -	if (query_matches_negative_refspec(rs, query))
    -+	if (find_negative_refspec_match(rs, query))
    ++	if (refspec_find_negative_match(rs, query))
      		return;
      
      	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: static void query_refspecs_multiple(struct refspec *rs,
      }
      
     -int query_refspecs(struct refspec *rs, struct refspec_item *query)
    -+int find_refspec_match(struct refspec *rs, struct refspec_item *query)
    ++int refspec_find_match(struct refspec *rs, struct refspec_item *query)
      {
      	int i;
      	int find_src = !query->src;
    @@ remote.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)
      
      	if (find_src && !query->dst)
     -		BUG("query_refspecs: need either src or dst");
    -+		BUG("find_refspec_match: need either src or dst");
    ++		BUG("refspec_find_match: need either src or dst");
      
     -	if (query_matches_negative_refspec(rs, query))
    -+	if (find_negative_refspec_match(rs, query))
    ++	if (refspec_find_negative_match(rs, query))
      		return -1;
      
      	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: char *apply_refspecs(struct refspec *rs, const char *name)
      	query.src = (char *)name;
      
     -	if (query_refspecs(rs, &query))
    -+	if (find_refspec_match(rs, &query))
    ++	if (refspec_find_match(rs, &query))
      		return NULL;
      
      	return query.dst;
    @@ remote.c: char *apply_refspecs(struct refspec *rs, const char *name)
      int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
      {
     -	return query_refspecs(&remote->fetch, refspec);
    -+	return find_refspec_match(&remote->fetch, refspec);
    ++	return refspec_find_match(&remote->fetch, refspec);
      }
      
      static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
    @@ remote.c: static int get_stale_heads_cb(const char *refname, const char *referen
      	query.dst = (char *)refname;
      
     -	query_refspecs_multiple(info->rs, &query, &matches);
    -+	find_all_refspec_matches(info->rs, &query, &matches);
    ++	refspec_find_all_matches(info->rs, &query, &matches);
      	if (matches.nr == 0)
      		goto clean_exit; /* No matches */
      
    @@ remote.h: int refname_matches_negative_refspec_item(const char *refname, struct
      struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
      
     -int query_refspecs(struct refspec *rs, struct refspec_item *query);
    -+int find_refspec_match(struct refspec *rs, struct refspec_item *query);
    ++int refspec_find_match(struct refspec *rs, struct refspec_item *query);
      char *apply_refspecs(struct refspec *rs, const char *name);
      
      int check_push_refs(struct ref *src, struct refspec *rs);
4:  13e49509fc ! 4:  9a5dc26731 refspec: relocate matching related functions
    @@ Metadata
      ## Commit message ##
         refspec: relocate matching related functions
     
    -    Move the functions `find_refspec_match()`, `find_all_refspec_matches()`
    -    and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.
    +    Move the functions `refspec_find_match()`, `refspec_find_all_matches()`
    +    and `refspec_find_negative_match()` from `remote.c` to `refspec.c`.
         These functions focus on matching refspecs, so centralizing them in
         `refspec.c` improves code organization by keeping refspec-related logic
         in one place.
    @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct
      	return 0;
      }
     +
    -+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
    ++static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i, matched_negative = 0;
     +	int find_src = !query->src;
    @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct
     +	return matched_negative;
     +}
     +
    -+void find_all_refspec_matches(struct refspec *rs,
    ++void refspec_find_all_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results)
     +{
    @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct
     +	int find_src = !query->src;
     +
     +	if (find_src && !query->dst)
    -+		BUG("find_all_refspec_matches: need either src or dst");
    ++		BUG("refspec_find_all_matches: need either src or dst");
     +
    -+	if (find_negative_refspec_match(rs, query))
    ++	if (refspec_find_negative_match(rs, query))
     +		return;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct
     +	}
     +}
     +
    -+int find_refspec_match(struct refspec *rs, struct refspec_item *query)
    ++int refspec_find_match(struct refspec *rs, struct refspec_item *query)
     +{
     +	int i;
     +	int find_src = !query->src;
    @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct
     +	char **result = find_src ? &query->src : &query->dst;
     +
     +	if (find_src && !query->dst)
    -+		BUG("find_refspec_match: need either src or dst");
    ++		BUG("refspec_find_match: need either src or dst");
     +
    -+	if (find_negative_refspec_match(rs, query))
    ++	if (refspec_find_negative_match(rs, query))
     +		return -1;
     +
     +	for (i = 0; i < rs->nr; i++) {
    @@ refspec.h: int refname_matches_negative_refspec_item(const char *refname, struct
     + * Queries a refspec for a match and updates the query item.
     + * Returns 0 on success, -1 if no match is found or negative refspec matches.
     + */
    -+int find_refspec_match(struct refspec *rs, struct refspec_item *query);
    ++int refspec_find_match(struct refspec *rs, struct refspec_item *query);
     +
     +/*
     + * Queries a refspec for all matches and appends results to the provided string
     + * list.
     + */
    -+void find_all_refspec_matches(struct refspec *rs,
    ++void refspec_find_all_matches(struct refspec *rs,
     +				    struct refspec_item *query,
     +				    struct string_list *results);
     +
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
      	return ref_map;
      }
      
    --static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)
    +-static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i, matched_negative = 0;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	return matched_negative;
     -}
     -
    --static void find_all_refspec_matches(struct refspec *rs,
    +-static void refspec_find_all_matches(struct refspec *rs,
     -				    struct refspec_item *query,
     -				    struct string_list *results)
     -{
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	int find_src = !query->src;
     -
     -	if (find_src && !query->dst)
    --		BUG("find_all_refspec_matches: need either src or dst");
    +-		BUG("refspec_find_all_matches: need either src or dst");
     -
    --	if (find_negative_refspec_match(rs, query))
    +-	if (refspec_find_negative_match(rs, query))
     -		return;
     -
     -	for (i = 0; i < rs->nr; i++) {
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	}
     -}
     -
    --int find_refspec_match(struct refspec *rs, struct refspec_item *query)
    +-int refspec_find_match(struct refspec *rs, struct refspec_item *query)
     -{
     -	int i;
     -	int find_src = !query->src;
    @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe
     -	char **result = find_src ? &query->src : &query->dst;
     -
     -	if (find_src && !query->dst)
    --		BUG("find_refspec_match: need either src or dst");
    +-		BUG("refspec_find_match: need either src or dst");
     -
    --	if (find_negative_refspec_match(rs, query))
    +-	if (refspec_find_negative_match(rs, query))
     -		return -1;
     -
     -	for (i = 0; i < rs->nr; i++) {
5:  891e01be93 ! 5:  f13ac6f11f refspec: relocate apply_refspecs and related funtions
    @@ refspec.c
      #include "strbuf.h"
      
      /*
    -@@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query)
    +@@ refspec.c: int refspec_find_match(struct refspec *rs, struct refspec_item *query)
      	}
      	return -1;
      }
    @@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query
     +	memset(&query, 0, sizeof(struct refspec_item));
     +	query.src = (char *)name;
     +
    -+	if (find_refspec_match(rs, &query))
    ++	if (refspec_find_match(rs, &query))
     +		return NULL;
     +
     +	return query.dst;
     +}
     
      ## refspec.h ##
    -@@ refspec.h: void find_all_refspec_matches(struct refspec *rs,
    +@@ refspec.h: void refspec_find_all_matches(struct refspec *rs,
      				    struct refspec_item *query,
      				    struct string_list *results);
      
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -	memset(&query, 0, sizeof(struct refspec_item));
     -	query.src = (char *)name;
     -
    --	if (find_refspec_match(rs, &query))
    +-	if (refspec_find_match(rs, &query))
     -		return NULL;
     -
     -	return query.dst;
    @@ remote.c: void ref_push_report_free(struct ref_push_report *report)
     -
      int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
      {
    - 	return find_refspec_match(&remote->fetch, refspec);
    + 	return refspec_find_match(&remote->fetch, refspec);
     
      ## remote.h ##
     @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
    @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);
     - */
     -struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
     -
    --int find_refspec_match(struct refspec *rs, struct refspec_item *query);
    +-int refspec_find_match(struct refspec *rs, struct refspec_item *query);
     -char *apply_refspecs(struct refspec *rs, const char *name);
     -
      int check_push_refs(struct ref *src, struct refspec *rs);
-- 
2.34.1

[GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec

[Meet Soni]

Rename the function `omit_name_by_refspec()` to
`refname_matches_negative_refspec_item()` to provide clearer intent.
The previous function name was vague and did not accurately describe its
purpose. By using `refname_matches_negative_refspec_item`, make the
function's purpose more intuitive, clarifying that it checks if a
reference name matches any negative refspec.

Rename function parameters for consistency with existing naming
conventions. Use `refname` instead of `name` to align with terminology
in `refs.h`.

Remove the redundant doc comment since the function name is now
self-explanatory.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 builtin/remote.c | 2 +-
 remote.c         | 8 ++++----
 remote.h         | 6 +-----
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 0435963286..258b8895cd 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -383,7 +383,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 				states->remote->fetch.items[i].raw);
 
 	for (ref = fetch_map; ref; ref = ref->next) {
-		if (omit_name_by_refspec(ref->name, &states->remote->fetch))
+		if (refname_matches_negative_refspec_item(ref->name, &states->remote->fetch))
 			string_list_append(&states->skipped, abbrev_branch(ref->name));
 		else if (!ref->peer_ref || !refs_ref_exists(get_main_ref_store(the_repository), ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
diff --git a/remote.c b/remote.c
index 0f6fba8562..cb70ce6f3b 100644
--- a/remote.c
+++ b/remote.c
@@ -944,12 +944,12 @@ static int refspec_match(const struct refspec_item *refspec,
 	return !strcmp(refspec->src, name);
 }
 
-int omit_name_by_refspec(const char *name, struct refspec *rs)
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
 {
 	int i;
 
 	for (i = 0; i < rs->nr; i++) {
-		if (rs->items[i].negative && refspec_match(&rs->items[i], name))
+		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
 			return 1;
 	}
 	return 0;
@@ -962,7 +962,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	for (tail = &ref_map; *tail; ) {
 		struct ref *ref = *tail;
 
-		if (omit_name_by_refspec(ref->name, rs)) {
+		if (refname_matches_negative_refspec_item(ref->name, rs)) {
 			*tail = ref->next;
 			free(ref->peer_ref);
 			free(ref);
@@ -1021,7 +1021,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
 	}
 
 	for (i = 0; !matched_negative && i < reversed.nr; i++) {
-		if (omit_name_by_refspec(reversed.items[i].string, rs))
+		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
 			matched_negative = 1;
 	}
 
diff --git a/remote.h b/remote.h
index bda10dd5c8..66ee53411d 100644
--- a/remote.h
+++ b/remote.h
@@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-/*
- * Check whether a name matches any negative refspec in rs. Returns 1 if the
- * name matches at least one negative refspec, and 0 otherwise.
- */
-int omit_name_by_refspec(const char *name, struct refspec *rs);
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
 
 /*
  * Remove all entries in the input list which match any negative refspec in
-- 
2.34.1

[GSoC][PATCH v4 2/5] refspec: relocate refname_matches_negative_refspec_item

[Meet Soni]

Move the functions `refname_matches_negative_refspec_item()`,
`refspec_match()`, and `match_name_with_pattern()` from `remote.c` to
`refspec.c`. These functions focus on refspec matching, so placing them
in `refspec.c` aligns with the separation of concerns. Keep
refspec-related logic in `refspec.c` and remote-specific logic in
`remote.c` for better code organization.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  9 +++++++++
 remote.c  | 48 ------------------------------------------------
 3 files changed, 57 insertions(+), 48 deletions(-)

diff --git a/refspec.c b/refspec.c
index 6d86e04442..b447768304 100644
--- a/refspec.c
+++ b/refspec.c
@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,
 		}
 	}
 }
+
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result)
+{
+	const char *kstar = strchr(key, '*');
+	size_t klen;
+	size_t ksuffixlen;
+	size_t namelen;
+	int ret;
+	if (!kstar)
+		die(_("key '%s' of pattern had no '*'"), key);
+	klen = kstar - key;
+	ksuffixlen = strlen(kstar + 1);
+	namelen = strlen(name);
+	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
+		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
+	if (ret && value) {
+		struct strbuf sb = STRBUF_INIT;
+		const char *vstar = strchr(value, '*');
+		if (!vstar)
+			die(_("value '%s' of pattern has no '*'"), value);
+		strbuf_add(&sb, value, vstar - value);
+		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
+		strbuf_addstr(&sb, vstar + 1);
+		*result = strbuf_detach(&sb, NULL);
+	}
+	return ret;
+}
+
+static int refspec_match(const struct refspec_item *refspec,
+			 const char *name)
+{
+	if (refspec->pattern)
+		return match_name_with_pattern(refspec->src, name, NULL, NULL);
+
+	return !strcmp(refspec->src, name);
+}
+
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
+{
+	int i;
+
+	for (i = 0; i < rs->nr; i++) {
+		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
+			return 1;
+	}
+	return 0;
+}
diff --git a/refspec.h b/refspec.h
index 69d693c87d..584d9c9eb5 100644
--- a/refspec.h
+++ b/refspec.h
@@ -71,4 +71,13 @@ struct strvec;
 void refspec_ref_prefixes(const struct refspec *rs,
 			  struct strvec *ref_prefixes);
 
+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
+
+/*
+ * Checks whether a name matches a pattern and optionally generates a result.
+ * Returns 1 if the name matches the pattern, 0 otherwise.
+ */
+int match_name_with_pattern(const char *key, const char *name,
+				   const char *value, char **result);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index cb70ce6f3b..1da8ec7037 100644
--- a/remote.c
+++ b/remote.c
@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-static int match_name_with_pattern(const char *key, const char *name,
-				   const char *value, char **result)
-{
-	const char *kstar = strchr(key, '*');
-	size_t klen;
-	size_t ksuffixlen;
-	size_t namelen;
-	int ret;
-	if (!kstar)
-		die(_("key '%s' of pattern had no '*'"), key);
-	klen = kstar - key;
-	ksuffixlen = strlen(kstar + 1);
-	namelen = strlen(name);
-	ret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&
-		!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);
-	if (ret && value) {
-		struct strbuf sb = STRBUF_INIT;
-		const char *vstar = strchr(value, '*');
-		if (!vstar)
-			die(_("value '%s' of pattern has no '*'"), value);
-		strbuf_add(&sb, value, vstar - value);
-		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
-		strbuf_addstr(&sb, vstar + 1);
-		*result = strbuf_detach(&sb, NULL);
-	}
-	return ret;
-}
-
-static int refspec_match(const struct refspec_item *refspec,
-			 const char *name)
-{
-	if (refspec->pattern)
-		return match_name_with_pattern(refspec->src, name, NULL, NULL);
-
-	return !strcmp(refspec->src, name);
-}
-
-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)
-{
-	int i;
-
-	for (i = 0; i < rs->nr; i++) {
-		if (rs->items[i].negative && refspec_match(&rs->items[i], refname))
-			return 1;
-	}
-	return 0;
-}
-
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 {
 	struct ref **tail;
-- 
2.34.1

[GSoC][PATCH v4 3/5] remote: rename query_refspecs functions

[Meet Soni]

Rename functions related to handling refspecs in preparation for their
move from `remote.c` to `refspec.c`. Update their names to better
reflect their intent:

    - `query_refspecs()` -> `refspec_find_match()` for clarity, as it
      finds a single matching refspec.

    - `query_refspecs_multiple()` -> `refspec_find_all_matches()` to
      better reflect that it collects all matching refspecs instead of
      returning just the first match.

    - `query_matches_negative_refspec()` ->
      `refspec_find_negative_match()` for consistency with the
      updated naming convention, even though this static function
      didn't strictly require renaming.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 builtin/push.c |  2 +-
 remote.c       | 20 ++++++++++----------
 remote.h       |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 90de3746b5..92d530e5c4 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -78,7 +78,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
 			.src = matched->name,
 		};
 
-		if (!query_refspecs(&remote->push, &query) && query.dst) {
+		if (!refspec_find_match(&remote->push, &query) && query.dst) {
 			refspec_appendf(refspec, "%s%s:%s",
 					query.force ? "+" : "",
 					query.src, query.dst);
diff --git a/remote.c b/remote.c
index 1da8ec7037..b510809a56 100644
--- a/remote.c
+++ b/remote.c
@@ -925,7 +925,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	return ref_map;
 }
 
-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)
+static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
 {
 	int i, matched_negative = 0;
 	int find_src = !query->src;
@@ -982,7 +982,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
 	return matched_negative;
 }
 
-static void query_refspecs_multiple(struct refspec *rs,
+static void refspec_find_all_matches(struct refspec *rs,
 				    struct refspec_item *query,
 				    struct string_list *results)
 {
@@ -990,9 +990,9 @@ static void query_refspecs_multiple(struct refspec *rs,
 	int find_src = !query->src;
 
 	if (find_src && !query->dst)
-		BUG("query_refspecs_multiple: need either src or dst");
+		BUG("refspec_find_all_matches: need either src or dst");
 
-	if (query_matches_negative_refspec(rs, query))
+	if (refspec_find_negative_match(rs, query))
 		return;
 
 	for (i = 0; i < rs->nr; i++) {
@@ -1013,7 +1013,7 @@ static void query_refspecs_multiple(struct refspec *rs,
 	}
 }
 
-int query_refspecs(struct refspec *rs, struct refspec_item *query)
+int refspec_find_match(struct refspec *rs, struct refspec_item *query)
 {
 	int i;
 	int find_src = !query->src;
@@ -1021,9 +1021,9 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)
 	char **result = find_src ? &query->src : &query->dst;
 
 	if (find_src && !query->dst)
-		BUG("query_refspecs: need either src or dst");
+		BUG("refspec_find_match: need either src or dst");
 
-	if (query_matches_negative_refspec(rs, query))
+	if (refspec_find_negative_match(rs, query))
 		return -1;
 
 	for (i = 0; i < rs->nr; i++) {
@@ -1054,7 +1054,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)
 	memset(&query, 0, sizeof(struct refspec_item));
 	query.src = (char *)name;
 
-	if (query_refspecs(rs, &query))
+	if (refspec_find_match(rs, &query))
 		return NULL;
 
 	return query.dst;
@@ -1062,7 +1062,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)
 
 int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
 {
-	return query_refspecs(&remote->fetch, refspec);
+	return refspec_find_match(&remote->fetch, refspec);
 }
 
 static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
@@ -2487,7 +2487,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED,
 	memset(&query, 0, sizeof(struct refspec_item));
 	query.dst = (char *)refname;
 
-	query_refspecs_multiple(info->rs, &query, &matches);
+	refspec_find_all_matches(info->rs, &query, &matches);
 	if (matches.nr == 0)
 		goto clean_exit; /* No matches */
 
diff --git a/remote.h b/remote.h
index 66ee53411d..516ba7f398 100644
--- a/remote.h
+++ b/remote.h
@@ -269,7 +269,7 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
  */
 struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
 
-int query_refspecs(struct refspec *rs, struct refspec_item *query);
+int refspec_find_match(struct refspec *rs, struct refspec_item *query);
 char *apply_refspecs(struct refspec *rs, const char *name);
 
 int check_push_refs(struct ref *src, struct refspec *rs);
-- 
2.34.1

[GSoC][PATCH v4 4/5] refspec: relocate matching related functions

[Meet Soni]

Move the functions `refspec_find_match()`, `refspec_find_all_matches()`
and `refspec_find_negative_match()` from `remote.c` to `refspec.c`.
These functions focus on matching refspecs, so centralizing them in
`refspec.c` improves code organization by keeping refspec-related logic
in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 refspec.h |  16 +++++++
 remote.c  | 122 -----------------------------------------------------
 3 files changed, 139 insertions(+), 122 deletions(-)

diff --git a/refspec.c b/refspec.c
index b447768304..cab0b0d127 100644
--- a/refspec.c
+++ b/refspec.c
@@ -5,6 +5,7 @@
 #include "gettext.h"
 #include "hash.h"
 #include "hex.h"
+#include "string-list.h"
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
@@ -324,3 +325,125 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
 	}
 	return 0;
 }
+
+static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
+{
+	int i, matched_negative = 0;
+	int find_src = !query->src;
+	struct string_list reversed = STRING_LIST_INIT_DUP;
+	const char *needle = find_src ? query->dst : query->src;
+
+	/*
+	 * Check whether the queried ref matches any negative refpsec. If so,
+	 * then we should ultimately treat this as not matching the query at
+	 * all.
+	 *
+	 * Note that negative refspecs always match the source, but the query
+	 * item uses the destination. To handle this, we apply pattern
+	 * refspecs in reverse to figure out if the query source matches any
+	 * of the negative refspecs.
+	 *
+	 * The first loop finds and expands all positive refspecs
+	 * matched by the queried ref.
+	 *
+	 * The second loop checks if any of the results of the first loop
+	 * match any negative refspec.
+	 */
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		char *expn_name;
+
+		if (refspec->negative)
+			continue;
+
+		/* Note the reversal of src and dst */
+		if (refspec->pattern) {
+			const char *key = refspec->dst ? refspec->dst : refspec->src;
+			const char *value = refspec->src;
+
+			if (match_name_with_pattern(key, needle, value, &expn_name))
+				string_list_append_nodup(&reversed, expn_name);
+		} else if (refspec->matching) {
+			/* For the special matching refspec, any query should match */
+			string_list_append(&reversed, needle);
+		} else if (!refspec->src) {
+			BUG("refspec->src should not be null here");
+		} else if (!strcmp(needle, refspec->src)) {
+			string_list_append(&reversed, refspec->src);
+		}
+	}
+
+	for (i = 0; !matched_negative && i < reversed.nr; i++) {
+		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
+			matched_negative = 1;
+	}
+
+	string_list_clear(&reversed, 0);
+
+	return matched_negative;
+}
+
+void refspec_find_all_matches(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results)
+{
+	int i;
+	int find_src = !query->src;
+
+	if (find_src && !query->dst)
+		BUG("refspec_find_all_matches: need either src or dst");
+
+	if (refspec_find_negative_match(rs, query))
+		return;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+		const char *needle = find_src ? query->dst : query->src;
+		char **result = find_src ? &query->src : &query->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result))
+				string_list_append_nodup(results, *result);
+		} else if (!strcmp(needle, key)) {
+			string_list_append(results, value);
+		}
+	}
+}
+
+int refspec_find_match(struct refspec *rs, struct refspec_item *query)
+{
+	int i;
+	int find_src = !query->src;
+	const char *needle = find_src ? query->dst : query->src;
+	char **result = find_src ? &query->src : &query->dst;
+
+	if (find_src && !query->dst)
+		BUG("refspec_find_match: need either src or dst");
+
+	if (refspec_find_negative_match(rs, query))
+		return -1;
+
+	for (i = 0; i < rs->nr; i++) {
+		struct refspec_item *refspec = &rs->items[i];
+		const char *key = find_src ? refspec->dst : refspec->src;
+		const char *value = find_src ? refspec->src : refspec->dst;
+
+		if (!refspec->dst || refspec->negative)
+			continue;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(key, needle, value, result)) {
+				query->force = refspec->force;
+				return 0;
+			}
+		} else if (!strcmp(needle, key)) {
+			*result = xstrdup(value);
+			query->force = refspec->force;
+			return 0;
+		}
+	}
+	return -1;
+}
diff --git a/refspec.h b/refspec.h
index 584d9c9eb5..be20ba53ab 100644
--- a/refspec.h
+++ b/refspec.h
@@ -30,6 +30,8 @@ struct refspec_item {
 	char *raw;
 };
 
+struct string_list;
+
 #define REFSPEC_FETCH 1
 #define REFSPEC_PUSH 0
 
@@ -80,4 +82,18 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r
 int match_name_with_pattern(const char *key, const char *name,
 				   const char *value, char **result);
 
+/*
+ * Queries a refspec for a match and updates the query item.
+ * Returns 0 on success, -1 if no match is found or negative refspec matches.
+ */
+int refspec_find_match(struct refspec *rs, struct refspec_item *query);
+
+/*
+ * Queries a refspec for all matches and appends results to the provided string
+ * list.
+ */
+void refspec_find_all_matches(struct refspec *rs,
+				    struct refspec_item *query,
+				    struct string_list *results);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index b510809a56..4c5940482f 100644
--- a/remote.c
+++ b/remote.c
@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
 	return ref_map;
 }
 
-static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)
-{
-	int i, matched_negative = 0;
-	int find_src = !query->src;
-	struct string_list reversed = STRING_LIST_INIT_DUP;
-	const char *needle = find_src ? query->dst : query->src;
-
-	/*
-	 * Check whether the queried ref matches any negative refpsec. If so,
-	 * then we should ultimately treat this as not matching the query at
-	 * all.
-	 *
-	 * Note that negative refspecs always match the source, but the query
-	 * item uses the destination. To handle this, we apply pattern
-	 * refspecs in reverse to figure out if the query source matches any
-	 * of the negative refspecs.
-	 *
-	 * The first loop finds and expands all positive refspecs
-	 * matched by the queried ref.
-	 *
-	 * The second loop checks if any of the results of the first loop
-	 * match any negative refspec.
-	 */
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		char *expn_name;
-
-		if (refspec->negative)
-			continue;
-
-		/* Note the reversal of src and dst */
-		if (refspec->pattern) {
-			const char *key = refspec->dst ? refspec->dst : refspec->src;
-			const char *value = refspec->src;
-
-			if (match_name_with_pattern(key, needle, value, &expn_name))
-				string_list_append_nodup(&reversed, expn_name);
-		} else if (refspec->matching) {
-			/* For the special matching refspec, any query should match */
-			string_list_append(&reversed, needle);
-		} else if (!refspec->src) {
-			BUG("refspec->src should not be null here");
-		} else if (!strcmp(needle, refspec->src)) {
-			string_list_append(&reversed, refspec->src);
-		}
-	}
-
-	for (i = 0; !matched_negative && i < reversed.nr; i++) {
-		if (refname_matches_negative_refspec_item(reversed.items[i].string, rs))
-			matched_negative = 1;
-	}
-
-	string_list_clear(&reversed, 0);
-
-	return matched_negative;
-}
-
-static void refspec_find_all_matches(struct refspec *rs,
-				    struct refspec_item *query,
-				    struct string_list *results)
-{
-	int i;
-	int find_src = !query->src;
-
-	if (find_src && !query->dst)
-		BUG("refspec_find_all_matches: need either src or dst");
-
-	if (refspec_find_negative_match(rs, query))
-		return;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-		const char *needle = find_src ? query->dst : query->src;
-		char **result = find_src ? &query->src : &query->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result))
-				string_list_append_nodup(results, *result);
-		} else if (!strcmp(needle, key)) {
-			string_list_append(results, value);
-		}
-	}
-}
-
-int refspec_find_match(struct refspec *rs, struct refspec_item *query)
-{
-	int i;
-	int find_src = !query->src;
-	const char *needle = find_src ? query->dst : query->src;
-	char **result = find_src ? &query->src : &query->dst;
-
-	if (find_src && !query->dst)
-		BUG("refspec_find_match: need either src or dst");
-
-	if (refspec_find_negative_match(rs, query))
-		return -1;
-
-	for (i = 0; i < rs->nr; i++) {
-		struct refspec_item *refspec = &rs->items[i];
-		const char *key = find_src ? refspec->dst : refspec->src;
-		const char *value = find_src ? refspec->src : refspec->dst;
-
-		if (!refspec->dst || refspec->negative)
-			continue;
-		if (refspec->pattern) {
-			if (match_name_with_pattern(key, needle, value, result)) {
-				query->force = refspec->force;
-				return 0;
-			}
-		} else if (!strcmp(needle, key)) {
-			*result = xstrdup(value);
-			query->force = refspec->force;
-			return 0;
-		}
-	}
-	return -1;
-}
-
 char *apply_refspecs(struct refspec *rs, const char *name)
 {
 	struct refspec_item query;
-- 
2.34.1

[GSoC][PATCH v4 5/5] refspec: relocate apply_refspecs and related funtions

[Meet Soni]

Move the functions `apply_refspecs()` and `apply_negative_refspecs()`
from `remote.c` to `refspec.c`. These functions focus on applying
refspecs, so centralizing them in `refspec.c` improves code organization
by keeping refspec-related logic in one place.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
 refspec.c | 32 ++++++++++++++++++++++++++++++++
 refspec.h | 12 ++++++++++++
 remote.c  | 31 -------------------------------
 remote.h  | 11 -----------
 4 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/refspec.c b/refspec.c
index cab0b0d127..0dbbd1e799 100644
--- a/refspec.c
+++ b/refspec.c
@@ -9,6 +9,7 @@
 #include "strvec.h"
 #include "refs.h"
 #include "refspec.h"
+#include "remote.h"
 #include "strbuf.h"
 
 /*
@@ -447,3 +448,34 @@ int refspec_find_match(struct refspec *rs, struct refspec_item *query)
 	}
 	return -1;
 }
+
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
+{
+	struct ref **tail;
+
+	for (tail = &ref_map; *tail; ) {
+		struct ref *ref = *tail;
+
+		if (refname_matches_negative_refspec_item(ref->name, rs)) {
+			*tail = ref->next;
+			free(ref->peer_ref);
+			free(ref);
+		} else
+			tail = &ref->next;
+	}
+
+	return ref_map;
+}
+
+char *apply_refspecs(struct refspec *rs, const char *name)
+{
+	struct refspec_item query;
+
+	memset(&query, 0, sizeof(struct refspec_item));
+	query.src = (char *)name;
+
+	if (refspec_find_match(rs, &query))
+		return NULL;
+
+	return query.dst;
+}
diff --git a/refspec.h b/refspec.h
index be20ba53ab..2a28d043be 100644
--- a/refspec.h
+++ b/refspec.h
@@ -96,4 +96,16 @@ void refspec_find_all_matches(struct refspec *rs,
 				    struct refspec_item *query,
 				    struct string_list *results);
 
+/*
+ * Remove all entries in the input list which match any negative refspec in
+ * the refspec list.
+ */
+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
+
+/*
+ * Search for a refspec that matches the given name and return the
+ * corresponding destination (dst) if a match is found, NULL otherwise.
+ */
+char *apply_refspecs(struct refspec *rs, const char *name);
+
 #endif /* REFSPEC_H */
diff --git a/remote.c b/remote.c
index 4c5940482f..7f27c59a5b 100644
--- a/remote.c
+++ b/remote.c
@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)
 	}
 }
 
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)
-{
-	struct ref **tail;
-
-	for (tail = &ref_map; *tail; ) {
-		struct ref *ref = *tail;
-
-		if (refname_matches_negative_refspec_item(ref->name, rs)) {
-			*tail = ref->next;
-			free(ref->peer_ref);
-			free(ref);
-		} else
-			tail = &ref->next;
-	}
-
-	return ref_map;
-}
-
-char *apply_refspecs(struct refspec *rs, const char *name)
-{
-	struct refspec_item query;
-
-	memset(&query, 0, sizeof(struct refspec_item));
-	query.src = (char *)name;
-
-	if (refspec_find_match(rs, &query))
-		return NULL;
-
-	return query.dst;
-}
-
 int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)
 {
 	return refspec_find_match(&remote->fetch, refspec);
diff --git a/remote.h b/remote.h
index 516ba7f398..b4bb16af0e 100644
--- a/remote.h
+++ b/remote.h
@@ -261,17 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
  */
 struct ref *ref_remove_duplicates(struct ref *ref_map);
 
-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
-
-/*
- * Remove all entries in the input list which match any negative refspec in
- * the refspec list.
- */
-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);
-
-int refspec_find_match(struct refspec *rs, struct refspec_item *query);
-char *apply_refspecs(struct refspec *rs, const char *name);
-
 int check_push_refs(struct ref *src, struct refspec *rs);
 int match_push_refs(struct ref *src, struct ref **dst,
 		    struct refspec *rs, int flags);
-- 
2.34.1

Re: [GSoC][PATCH v4 0/5] refspec: centralize refspec-related logic

[Patrick Steinhardt]

On Tue, Feb 04, 2025 at 09:35:53AM +0530, Meet Soni wrote:
> Changes since v3:
>     - updated commit message.
>     - renamed functions as per review.
>     - added GSoC mark , since the announcement has been made by google and
>       we've started the discussion regarding the same.

Thanks, this version looks good to me!

Patrick

Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

Meet Soni <meetsoni3017@gmail.com> writes:

> diff --git a/remote.h b/remote.h
> index bda10dd5c8..66ee53411d 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
>   */
>  struct ref *ref_remove_duplicates(struct ref *ref_map);
>
> -/*
> - * Check whether a name matches any negative refspec in rs. Returns 1 if the
> - * name matches at least one negative refspec, and 0 otherwise.
> - */
> -int omit_name_by_refspec(const char *name, struct refspec *rs);
> +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
>

Nit: The first sentence is now duplicated by the function name as
mentioned in the commit message. But aren't we loosing information by
removing the second sentence?

>  /*
>   * Remove all entries in the input list which match any negative refspec in
> --
> 2.34.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions

[Junio C Hamano]

Meet Soni <meetsoni3017@gmail.com> writes:

>> So:
>>
>>   - `query_refspecs()` would be renamed to `refspec_find_match()`.
>>
>>   - `query_refspecs_multiple()` would be renamed to
>>     `refspec_find_all_matches()`.
>>
>>   - `find_negative_refspec_match()` would be renamed to
>>     `refspec_find_negative_match()`.
>>
>> Patrick
>
> Thanks for the review.
> Meet

Yup, using predictable names that follow patterns based on
easy-to-follow rules is a very useful tool to help developers.

Thanks, both.

Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec

[Meet Soni]

On Tue, 4 Feb 2025 at 14:30, Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Meet Soni <meetsoni3017@gmail.com> writes:
>
> > diff --git a/remote.h b/remote.h
> > index bda10dd5c8..66ee53411d 100644
> > --- a/remote.h
> > +++ b/remote.h
> > @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
> >   */
> >  struct ref *ref_remove_duplicates(struct ref *ref_map);
> >
> > -/*
> > - * Check whether a name matches any negative refspec in rs. Returns 1 if the
> > - * name matches at least one negative refspec, and 0 otherwise.
> > - */
> > -int omit_name_by_refspec(const char *name, struct refspec *rs);
> > +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
> >
>
> Nit: The first sentence is now duplicated by the function name as
> mentioned in the commit message. But aren't we loosing information by
> removing the second sentence?
>
Correct. I considered keeping the second sentence for clarity, but that other
function signatures in the codebase don’t include comments solely describing
return values. To maintain consistency with the existing style, I
opted to remove
it. Let me know if you think an alternative approach would be better!
> >  /*
> >   * Remove all entries in the input list which match any negative refspec in
> > --
> > 2.34.1

Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

Meet Soni <meetsoni3017@gmail.com> writes:

> On Tue, 4 Feb 2025 at 14:30, Karthik Nayak <karthik.188@gmail.com> wrote:
>>
>> Meet Soni <meetsoni3017@gmail.com> writes:
>>
>> > diff --git a/remote.h b/remote.h
>> > index bda10dd5c8..66ee53411d 100644
>> > --- a/remote.h
>> > +++ b/remote.h
>> > @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);
>> >   */
>> >  struct ref *ref_remove_duplicates(struct ref *ref_map);
>> >
>> > -/*
>> > - * Check whether a name matches any negative refspec in rs. Returns 1 if the
>> > - * name matches at least one negative refspec, and 0 otherwise.
>> > - */
>> > -int omit_name_by_refspec(const char *name, struct refspec *rs);
>> > +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);
>> >
>>
>> Nit: The first sentence is now duplicated by the function name as
>> mentioned in the commit message. But aren't we loosing information by
>> removing the second sentence?
>>
> Correct. I considered keeping the second sentence for clarity, but that other
> function signatures in the codebase don’t include comments solely describing
> return values. To maintain consistency with the existing style, I
> opted to remove
> it. Let me know if you think an alternative approach would be better!

I think its okay as-is for now :)

>> >  /*
>> >   * Remove all entries in the input list which match any negative refspec in
>> > --
>> > 2.34.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]