{"thread":{"id":"62861","subject":"[PATCH v2 0/3] refspec: centralize refspec-related logic","startedAt":"2025-01-27T10:36:51Z","lastAt":"2025-02-06T10:13:53Z","messageCount":33,"participants":["Meet Soni","Junio C Hamano","Patrick Steinhardt","Karthik Nayak"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"511235","messageId":"20250127103644.36627-1-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":null,"subject":"[PATCH v2 0/3] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-27T10:36:41Z","receivedAt":"2025-01-27T10:36:51Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Thank you for reviewing :)\n\nI've added documentation comments for various function signatures to\nbetter understand what they do.\n\nMeet Soni (3):\n refspec: relocate omit_name_by_refspec and related functions\n refspec: relocate query related functions\n refspec: relocate apply_refspecs and related funtions\n\n refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 41 +++++++++++\n remote.c | 201 -----------------------------------------------------\n remote.h | 15 ----\n 4 files changed, 244 insertions(+), 216 deletions(-)\n\nRange-diff against v1:\n1: 97c98f5a38 ! 1: 8e393ea1c2 refspec: relocate omit_name_by_refspec and related functions\n @@ refspec.h: struct strvec;\n + * name matches at least one negative refspec, and 0 otherwise.\n + */\n +int omit_name_by_refspec(const char *name, struct refspec *rs);\n ++\n ++/*\n ++ * Checks whether a name matches a pattern and optionally generates a result.\n ++ * Returns 1 if the name matches the pattern, 0 otherwise.\n ++ */\n +int match_name_with_pattern(const char *key, const char *name,\n +\t\t\t\t const char *value, char **result);\n +\n2: 4f0080aad6 ! 2: ef6edbc15b refspec: relocate query related functions\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +}\n \n ## refspec.h ##\n -@@\n - #ifndef REFSPEC_H\n - #define REFSPEC_H\n +@@ refspec.h: struct refspec_item {\n + \tchar *raw;\n + };\n \n -+#include \"string-list.h\"\n ++struct string_list;\n +\n - #define TAG_REFSPEC \"refs/tags/*:refs/tags/*\"\n + #define REFSPEC_FETCH 1\n + #define REFSPEC_PUSH 0\n \n - /**\n @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t const char *value, char **result);\n \n ++/*\n ++ * Queries a refspec for a match and updates the query item.\n ++ * Returns 0 on success, -1 if no match is found or negative refspec matches.\n ++ */\n +int query_refspecs(struct refspec *rs, struct refspec_item *query);\n ++\n ++/*\n ++ * Queries a refspec for all matches and appends results to the provided string\n ++ * list.\n ++ */\n +void query_refspecs_multiple(struct refspec *rs,\n +\t\t\t\t struct refspec_item *query,\n +\t\t\t\t struct string_list *results);\n3: f89328fa66 ! 3: ea72647439 refspec: relocate apply_refspecs and related funtions\n @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,\n + */\n +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n +\n ++/*\n ++ * Applies refspecs to a name and returns the corresponding destination.\n ++ * Returns the destination string if a match is found, NULL otherwise.\n ++ */\n +char *apply_refspecs(struct refspec *rs, const char *name);\n +\n #endif /* REFSPEC_H */\n-- \n2.34.1\n\n"},{"id":"511236","messageId":"20250127103644.36627-2-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"[PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-27T10:36:42Z","receivedAt":"2025-01-27T10:36:56Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `omit_name_by_refspec()`, `refspec_match()`, and\n`match_name_with_pattern()` from `remote.c` to `refspec.c`. These\nfunctions focus on refspec matching, so placing them in `refspec.c`\naligns with the separation of concerns. Keep refspec-related logic in\n`refspec.c` and remote-specific logic in `remote.c` for better code\norganization.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 13 +++++++++++++\n remote.c | 48 ------------------------------------------------\n remote.h | 6 ------\n 4 files changed, 61 insertions(+), 54 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 6d86e04442..66989a1d75 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,\n \t\t}\n \t}\n }\n+\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result)\n+{\n+\tconst char *kstar = strchr(key, '*');\n+\tsize_t klen;\n+\tsize_t ksuffixlen;\n+\tsize_t namelen;\n+\tint ret;\n+\tif (!kstar)\n+\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n+\tklen = kstar - key;\n+\tksuffixlen = strlen(kstar + 1);\n+\tnamelen = strlen(name);\n+\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n+\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n+\tif (ret && value) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *vstar = strchr(value, '*');\n+\t\tif (!vstar)\n+\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n+\t\tstrbuf_add(&sb, value, vstar - value);\n+\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n+\t\tstrbuf_addstr(&sb, vstar + 1);\n+\t\t*result = strbuf_detach(&sb, NULL);\n+\t}\n+\treturn ret;\n+}\n+\n+static int refspec_match(const struct refspec_item *refspec,\n+\t\t\t const char *name)\n+{\n+\tif (refspec->pattern)\n+\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n+\n+\treturn !strcmp(refspec->src, name);\n+}\n+\n+int omit_name_by_refspec(const char *name, struct refspec *rs)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 69d693c87d..891d50b159 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -71,4 +71,17 @@ struct strvec;\n void refspec_ref_prefixes(const struct refspec *rs,\n \t\t\t struct strvec *ref_prefixes);\n \n+/*\n+ * Check whether a name matches any negative refspec in rs. Returns 1 if the\n+ * name matches at least one negative refspec, and 0 otherwise.\n+ */\n+int omit_name_by_refspec(const char *name, struct refspec *rs);\n+\n+/*\n+ * Checks whether a name matches a pattern and optionally generates a result.\n+ * Returns 1 if the name matches the pattern, 0 otherwise.\n+ */\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 0f6fba8562..40c2418065 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-static int match_name_with_pattern(const char *key, const char *name,\n-\t\t\t\t const char *value, char **result)\n-{\n-\tconst char *kstar = strchr(key, '*');\n-\tsize_t klen;\n-\tsize_t ksuffixlen;\n-\tsize_t namelen;\n-\tint ret;\n-\tif (!kstar)\n-\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n-\tklen = kstar - key;\n-\tksuffixlen = strlen(kstar + 1);\n-\tnamelen = strlen(name);\n-\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n-\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n-\tif (ret && value) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\tconst char *vstar = strchr(value, '*');\n-\t\tif (!vstar)\n-\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n-\t\tstrbuf_add(&sb, value, vstar - value);\n-\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n-\t\tstrbuf_addstr(&sb, vstar + 1);\n-\t\t*result = strbuf_detach(&sb, NULL);\n-\t}\n-\treturn ret;\n-}\n-\n-static int refspec_match(const struct refspec_item *refspec,\n-\t\t\t const char *name)\n-{\n-\tif (refspec->pattern)\n-\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n-\n-\treturn !strcmp(refspec->src, name);\n-}\n-\n-int omit_name_by_refspec(const char *name, struct refspec *rs)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n {\n \tstruct ref **tail;\ndiff --git a/remote.h b/remote.h\nindex bda10dd5c8..0d109fa9c9 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,12 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Check whether a name matches any negative refspec in rs. Returns 1 if the\n- * name matches at least one negative refspec, and 0 otherwise.\n- */\n-int omit_name_by_refspec(const char *name, struct refspec *rs);\n-\n /*\n * Remove all entries in the input list which match any negative refspec in\n * the refspec list.\n-- \n2.34.1\n\n"},{"id":"511237","messageId":"20250127103644.36627-3-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"[PATCH v2 2/3] refspec: relocate query related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-27T10:36:43Z","receivedAt":"2025-01-27T10:37:00Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `query_refspecs()`, `query_refspecs_multiple()` and\n`query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These\nfunctions focus on querying refspecs, so centralizing them in `refspec.c`\nimproves code organization by keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 16 +++++++\n remote.c | 122 -----------------------------------------------------\n remote.h | 1 -\n 4 files changed, 139 insertions(+), 123 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 66989a1d75..72b3911110 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -5,6 +5,7 @@\n #include \"gettext.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n@@ -324,3 +325,125 @@ int omit_name_by_refspec(const char *name, struct refspec *rs)\n \t}\n \treturn 0;\n }\n+\n+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i, matched_negative = 0;\n+\tint find_src = !query->src;\n+\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\n+\t/*\n+\t * Check whether the queried ref matches any negative refpsec. If so,\n+\t * then we should ultimately treat this as not matching the query at\n+\t * all.\n+\t *\n+\t * Note that negative refspecs always match the source, but the query\n+\t * item uses the destination. To handle this, we apply pattern\n+\t * refspecs in reverse to figure out if the query source matches any\n+\t * of the negative refspecs.\n+\t *\n+\t * The first loop finds and expands all positive refspecs\n+\t * matched by the queried ref.\n+\t *\n+\t * The second loop checks if any of the results of the first loop\n+\t * match any negative refspec.\n+\t */\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tchar *expn_name;\n+\n+\t\tif (refspec->negative)\n+\t\t\tcontinue;\n+\n+\t\t/* Note the reversal of src and dst */\n+\t\tif (refspec->pattern) {\n+\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n+\t\t\tconst char *value = refspec->src;\n+\n+\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n+\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n+\t\t} else if (refspec->matching) {\n+\t\t\t/* For the special matching refspec, any query should match */\n+\t\t\tstring_list_append(&reversed, needle);\n+\t\t} else if (!refspec->src) {\n+\t\t\tBUG(\"refspec->src should not be null here\");\n+\t\t} else if (!strcmp(needle, refspec->src)) {\n+\t\t\tstring_list_append(&reversed, refspec->src);\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n+\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n+\t\t\tmatched_negative = 1;\n+\t}\n+\n+\tstring_list_clear(&reversed, 0);\n+\n+\treturn matched_negative;\n+}\n+\n+void query_refspecs_multiple(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n+\n+\tif (query_matches_negative_refspec(rs, query))\n+\t\treturn;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\t\tconst char *needle = find_src ? query->dst : query->src;\n+\t\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result))\n+\t\t\t\tstring_list_append_nodup(results, *result);\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\tstring_list_append(results, value);\n+\t\t}\n+\t}\n+}\n+\n+int query_refspecs(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"query_refspecs: need either src or dst\");\n+\n+\tif (query_matches_negative_refspec(rs, query))\n+\t\treturn -1;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n+\t\t\t\tquery->force = refspec->force;\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\t*result = xstrdup(value);\n+\t\t\tquery->force = refspec->force;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn -1;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 891d50b159..d0788de782 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -30,6 +30,8 @@ struct refspec_item {\n \tchar *raw;\n };\n \n+struct string_list;\n+\n #define REFSPEC_FETCH 1\n #define REFSPEC_PUSH 0\n \n@@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t const char *value, char **result);\n \n+/*\n+ * Queries a refspec for a match and updates the query item.\n+ * Returns 0 on success, -1 if no match is found or negative refspec matches.\n+ */\n+int query_refspecs(struct refspec *rs, struct refspec_item *query);\n+\n+/*\n+ * Queries a refspec for all matches and appends results to the provided string\n+ * list.\n+ */\n+void query_refspecs_multiple(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 40c2418065..2c46611821 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i, matched_negative = 0;\n-\tint find_src = !query->src;\n-\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\n-\t/*\n-\t * Check whether the queried ref matches any negative refpsec. If so,\n-\t * then we should ultimately treat this as not matching the query at\n-\t * all.\n-\t *\n-\t * Note that negative refspecs always match the source, but the query\n-\t * item uses the destination. To handle this, we apply pattern\n-\t * refspecs in reverse to figure out if the query source matches any\n-\t * of the negative refspecs.\n-\t *\n-\t * The first loop finds and expands all positive refspecs\n-\t * matched by the queried ref.\n-\t *\n-\t * The second loop checks if any of the results of the first loop\n-\t * match any negative refspec.\n-\t */\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tchar *expn_name;\n-\n-\t\tif (refspec->negative)\n-\t\t\tcontinue;\n-\n-\t\t/* Note the reversal of src and dst */\n-\t\tif (refspec->pattern) {\n-\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n-\t\t\tconst char *value = refspec->src;\n-\n-\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n-\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n-\t\t} else if (refspec->matching) {\n-\t\t\t/* For the special matching refspec, any query should match */\n-\t\t\tstring_list_append(&reversed, needle);\n-\t\t} else if (!refspec->src) {\n-\t\t\tBUG(\"refspec->src should not be null here\");\n-\t\t} else if (!strcmp(needle, refspec->src)) {\n-\t\t\tstring_list_append(&reversed, refspec->src);\n-\t\t}\n-\t}\n-\n-\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n-\t\t\tmatched_negative = 1;\n-\t}\n-\n-\tstring_list_clear(&reversed, 0);\n-\n-\treturn matched_negative;\n-}\n-\n-static void query_refspecs_multiple(struct refspec *rs,\n-\t\t\t\t struct refspec_item *query,\n-\t\t\t\t struct string_list *results)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n-\n-\tif (query_matches_negative_refspec(rs, query))\n-\t\treturn;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\t\tconst char *needle = find_src ? query->dst : query->src;\n-\t\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result))\n-\t\t\t\tstring_list_append_nodup(results, *result);\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\tstring_list_append(results, value);\n-\t\t}\n-\t}\n-}\n-\n-int query_refspecs(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs: need either src or dst\");\n-\n-\tif (query_matches_negative_refspec(rs, query))\n-\t\treturn -1;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n-\t\t\t\tquery->force = refspec->force;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\t*result = xstrdup(value);\n-\t\t\tquery->force = refspec->force;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn -1;\n-}\n-\n char *apply_refspecs(struct refspec *rs, const char *name)\n {\n \tstruct refspec_item query;\ndiff --git a/remote.h b/remote.h\nindex 0d109fa9c9..f3da64dc41 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -267,7 +267,6 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);\n */\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query);\n char *apply_refspecs(struct refspec *rs, const char *name);\n \n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511238","messageId":"20250127103644.36627-4-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"[PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-27T10:36:44Z","receivedAt":"2025-01-27T10:37:04Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `apply_refspecs()` and `apply_negative_refspecs()`\nfrom `remote.c` to `refspec.c`. These functions focus on applying\nrefspecs, so centralizing them in `refspec.c` improves code organization\nby keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 32 ++++++++++++++++++++++++++++++++\n refspec.h | 12 ++++++++++++\n remote.c | 31 -------------------------------\n remote.h | 8 --------\n 4 files changed, 44 insertions(+), 39 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 72b3911110..d279d6032a 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -9,6 +9,7 @@\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n+#include \"remote.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -447,3 +448,34 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n+\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n+{\n+\tstruct ref **tail;\n+\n+\tfor (tail = &ref_map; *tail; ) {\n+\t\tstruct ref *ref = *tail;\n+\n+\t\tif (omit_name_by_refspec(ref->name, rs)) {\n+\t\t\t*tail = ref->next;\n+\t\t\tfree(ref->peer_ref);\n+\t\t\tfree(ref);\n+\t\t} else\n+\t\t\ttail = &ref->next;\n+\t}\n+\n+\treturn ref_map;\n+}\n+\n+char *apply_refspecs(struct refspec *rs, const char *name)\n+{\n+\tstruct refspec_item query;\n+\n+\tmemset(&query, 0, sizeof(struct refspec_item));\n+\tquery.src = (char *)name;\n+\n+\tif (query_refspecs(rs, &query))\n+\t\treturn NULL;\n+\n+\treturn query.dst;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex d0788de782..231bcfb33e 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -100,4 +100,16 @@ void query_refspecs_multiple(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results);\n \n+/*\n+ * Remove all entries in the input list which match any negative refspec in\n+ * the refspec list.\n+ */\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n+\n+/*\n+ * Applies refspecs to a name and returns the corresponding destination.\n+ * Returns the destination string if a match is found, NULL otherwise.\n+ */\n+char *apply_refspecs(struct refspec *rs, const char *name);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 2c46611821..641dd1125f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n-{\n-\tstruct ref **tail;\n-\n-\tfor (tail = &ref_map; *tail; ) {\n-\t\tstruct ref *ref = *tail;\n-\n-\t\tif (omit_name_by_refspec(ref->name, rs)) {\n-\t\t\t*tail = ref->next;\n-\t\t\tfree(ref->peer_ref);\n-\t\t\tfree(ref);\n-\t\t} else\n-\t\t\ttail = &ref->next;\n-\t}\n-\n-\treturn ref_map;\n-}\n-\n-char *apply_refspecs(struct refspec *rs, const char *name)\n-{\n-\tstruct refspec_item query;\n-\n-\tmemset(&query, 0, sizeof(struct refspec_item));\n-\tquery.src = (char *)name;\n-\n-\tif (query_refspecs(rs, &query))\n-\t\treturn NULL;\n-\n-\treturn query.dst;\n-}\n-\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n \treturn query_refspecs(&remote->fetch, refspec);\ndiff --git a/remote.h b/remote.h\nindex f3da64dc41..b4bb16af0e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,14 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Remove all entries in the input list which match any negative refspec in\n- * the refspec list.\n- */\n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n-\n-char *apply_refspecs(struct refspec *rs, const char *name);\n-\n int check_push_refs(struct ref *src, struct refspec *rs);\n int match_push_refs(struct ref *src, struct ref **dst,\n \t\t struct refspec *rs, int flags);\n-- \n2.34.1\n\n"},{"id":"511239","messageId":"CAPhwyn1GHkZK-ySwFqEzFMxQjmzsEtPMBKGLv311pixUeR5W=g@mail.gmail.com","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"Re: [PATCH v2 0/3] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-27T11:00:45Z","receivedAt":"2025-01-27T11:00:59Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Mon, 27 Jan 2025 at 16:06, Meet Soni wrote:\n>\n> Thank you for reviewing :)\n>\n> I've added documentation comments for various function signatures to\n> better understand what they do.\n>\n> Meet Soni (3):\n> refspec: relocate omit_name_by_refspec and related functions\n> refspec: relocate query related functions\n> refspec: relocate apply_refspecs and related funtions\n>\n> refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> refspec.h | 41 +++++++++++\n> remote.c | 201 -----------------------------------------------------\n> remote.h | 15 ----\n> 4 files changed, 244 insertions(+), 216 deletions(-)\n>\n> Range-diff against v1:\n> 1: 97c98f5a38 ! 1: 8e393ea1c2 refspec: relocate omit_name_by_refspec and related functions\n> @@ refspec.h: struct strvec;\n> + * name matches at least one negative refspec, and 0 otherwise.\n> + */\n> +int omit_name_by_refspec(const char *name, struct refspec *rs);\n> ++\n> ++/*\n> ++ * Checks whether a name matches a pattern and optionally generates a result.\n> ++ * Returns 1 if the name matches the pattern, 0 otherwise.\n> ++ */\n> +int match_name_with_pattern(const char *key, const char *name,\n> + const char *value, char **result);\n> +\n> 2: 4f0080aad6 ! 2: ef6edbc15b refspec: relocate query related functions\n> @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n> +}\n>\n> ## refspec.h ##\n> -@@\n> - #ifndef REFSPEC_H\n> - #define REFSPEC_H\n> +@@ refspec.h: struct refspec_item {\n> + char *raw;\n> + };\n>\n> -+#include \"string-list.h\"\n> ++struct string_list;\n> +\n> - #define TAG_REFSPEC \"refs/tags/*:refs/tags/*\"\n> + #define REFSPEC_FETCH 1\n> + #define REFSPEC_PUSH 0\n>\n> - /**\n> @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);\n> int match_name_with_pattern(const char *key, const char *name,\n> const char *value, char **result);\n>\n> ++/*\n> ++ * Queries a refspec for a match and updates the query item.\n> ++ * Returns 0 on success, -1 if no match is found or negative refspec matches.\n> ++ */\n> +int query_refspecs(struct refspec *rs, struct refspec_item *query);\n> ++\n> ++/*\n> ++ * Queries a refspec for all matches and appends results to the provided string\n> ++ * list.\n> ++ */\n> +void query_refspecs_multiple(struct refspec *rs,\n> + struct refspec_item *query,\n> + struct string_list *results);\n> 3: f89328fa66 ! 3: ea72647439 refspec: relocate apply_refspecs and related funtions\n> @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,\n> + */\n> +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n> +\n> ++/*\n> ++ * Applies refspecs to a name and returns the corresponding destination.\n> ++ * Returns the destination string if a match is found, NULL otherwise.\n> ++ */\n> +char *apply_refspecs(struct refspec *rs, const char *name);\n> +\n> #endif /* REFSPEC_H */\n> --\n> 2.34.1\n>\nHi everyone,\nI realized I forgot to include the In-Reply-To header in my v2 submission,\nwhich would have linked this series to v1. My apologies for the oversight!\n\nFor reference, the v1 cover letter can be found here [1]\n[1]: https://lore.kernel.org/git/20250122075154.5697-1-meetsoni3017@gmail.com/\n\nPlease consider this email as a manual link between the v1 and v2 series.\nThank you for your understanding.\n\nBest regards,\nMeet\n"},{"id":"511283","messageId":"xmqqa5bctbnx.fsf@gitster.g","threadId":"62861","inReplyTo":"20250127103644.36627-2-meetsoni3017@gmail.com","subject":"Re: [PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-27T17:21:54Z","receivedAt":"2025-01-27T17:21:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Meet Soni writes:\n\n> Move the functions `omit_name_by_refspec()`, `refspec_match()`, and\n> `match_name_with_pattern()` from `remote.c` to `refspec.c`. These\n> functions focus on refspec matching, so placing them in `refspec.c`\n> aligns with the separation of concerns. Keep refspec-related logic in\n> `refspec.c` and remote-specific logic in `remote.c` for better code\n> organization.\n>\n> Signed-off-by: Meet Soni \n> ---\n> ...\n> diff --git a/refspec.h b/refspec.h\n> index 69d693c87d..891d50b159 100644\n> --- a/refspec.h\n> +++ b/refspec.h\n> @@ -71,4 +71,17 @@ struct strvec;\n> void refspec_ref_prefixes(const struct refspec *rs,\n> \t\t\t struct strvec *ref_prefixes);\n\nBack when these functions were mere local helper functions in\nremote.c, their name being less descriptive of what they do may have\nbeen OK (because readers have more context to understand them), but\nwhen we make it a part of a public API, we should re-evaluate if\ntheir names are good enough.\n\n> +/*\n> + * Check whether a name matches any negative refspec in rs. Returns 1 if the\n> + * name matches at least one negative refspec, and 0 otherwise.\n> + */\n> +int omit_name_by_refspec(const char *name, struct refspec *rs);\n\nImagine you found this description in the header file and are trying\nto figure out if it helps you writing the feature you are adding to\nGit. Are the above description and the name of the function useful\nenough to you?\n\nThe first question that came to my mind was \"what is exactly a 'name'?\"\n\nIn the context of the original, the caller iterates over a list of\n\"struct ref\" and feeds the \"name\" member of the struct, but this\ncaller does not even have to know it is getting a part of \"struct\nref\"; it only cares about its parameter being a character string.\n\nIn that context, is \"name\" the best identifer you can give to this\nparameter? At least calling it \"refname\" might boost the signal the\nname gives to the reader a bit better (and it is in line with how\nrefs.h calls these things).\n\nAnother thing to consider is if the comment describes the purpose of\nthe function well, instead of just rephrasing what its\nimplementation does. What does it mean to return true iff there is\neven one negative refspec that matches? What is the conceivable use\na caller would want to use such a function?\n\nAs I said, calling it \"omit\" was probably OK in the context of the\noriginal file, but it was already sloppy. This function merely\nprovides one bit of information (i.e. \"does it match any nagative\nrefspec---Yes or No?\"), and it is up to its caller how to use that\npiece of information form.\n\nOne of its callers, apply_negative_refspecs(), happens to use it to\nfilter a list of \"struct ref\" it received from its caller to drop\nthe refs from the list that match any negative refspec, but the\nother existing caller does not even filter or omit anything from a\ncollection it has.\n\nMy personal preference is to do this kind of change in two separate\npatches:\n\n (1) as a preliminary clean-up, we rename functions and their\n parameters in the original place; if needed, add clarifying\n comments.\n\n (2) move the resulting functions with the comments to their new\n home.\n\nIf these two step conversions results in\n\nextern int refname_matches_negative_refspec_item\n\t(const char *refname, struct refspec *refspec);\n\nI suspect that it is clear enough that there is no need for any\nextra comment to explain what it does.\n\n> +/*\n> + * Checks whether a name matches a pattern and optionally generates a result.\n> + * Returns 1 if the name matches the pattern, 0 otherwise.\n> + */\n> +int match_name_with_pattern(const char *key, const char *name,\n> +\t\t\t\t const char *value, char **result);\n> +\n\nAs this is merely moved from an existing header, I am tempted to say\nI'll leave it as an exercise to the readers to improve this one, as\nimproving it is outside the scope of this work.\n\nSome hints for those who want to tackle the clean-up for extra\npoints, perhaps after the dust settles from this series.\n\nThe \"pattern\" in the name refers to the src side of a globbing\nrefspec and is passed in the parameter \"key\", so we are calling the\nsame thing in three different names, which is already triply bad.\n\n\"optionally generates a result\" does not convey any meaning outside\nthe context of the original, as it does not even talk about what\ncomputation is creating the result. It does not even say what\ncontrols the optionality---without reading the implementation, it is\nlikely your readers would assume passing NULL to result is all it\ntakes to skip that optional feature, but that is not the case.\n\nIf I understand correctly, here is what this one does.\n\n It takes the source side of a globbing refspec item (e.g.\n \"refs/heads/*\" in \"refs/heads/*:refs/remotes/origin/*\"), a\n refname that might match the glob pattern, the destination side\n of the refspec item (e.g. \"refs/remotes/origin/*\" in the same\n example), and a pointer that points at a variable to receive the\n result. If the source pattern matches the given refname, apply\n the source-to-destination mapping rule to compute the resulting\n destination refname and store it in the result.\n\n The destination side is optional; if you do not need to map the\n refname to another refname, but are merely interested if the\n refname matches the glob pattern, you can pass NULL and result\n location is not touched.\n\n In either case, returns true iff the source side of the globbing\n refspec item matches the given refname.\n\nSo \"name\" in the function name should probably become a bit\nnarrower, like \"refname\". Also the names of its parameters need to\nbe better thought out.\n"},{"id":"511287","messageId":"xmqqikq0ruuk.fsf@gitster.g","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"Re: [PATCH v2 0/3] refspec: centralize refspec-related logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-27T18:10:27Z","receivedAt":"2025-01-27T18:10:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Meet Soni writes:\n\n> Thank you for reviewing :)\n>\n> I've added documentation comments for various function signatures to\n> better understand what they do.\n\nBefore saying all that, please help those who haven't read the\nprevious round (which wasn't even v1 IIRC but RFC and may have been\nskipped by some potential reviewers) by summarizing what this series\nis about. For other's convenience, here is a key excerpt from the\ncover letter of the previous iteration:\n\n As Patrick pointed out in [1], the logic related to refspec is currently\n split across multiple headers. This patch series addresses that by\n relocating refspec-related logic from remote to refspec for improved\n cohesion.\n\nWhile I was working on an unrelated issue, I noticed that there is\none function, \"extern int valid_remote_name(const char *);\" declared\nin which is only about a remote and should probably be\nmoved to ; cleaning it up does not have to be part of this\nseries, but since you are doing a similar clean-up effort, I thought\nyou would want to be aware of it.\n\nThanks.\n"},{"id":"511297","messageId":"xmqq7c6grrdl.fsf@gitster.g","threadId":"62861","inReplyTo":"20250127103644.36627-3-meetsoni3017@gmail.com","subject":"Re: [PATCH v2 2/3] refspec: relocate query related functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-27T19:25:26Z","receivedAt":"2025-01-27T19:25:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Meet Soni writes:\n\n> Move the functions `query_refspecs()`, `query_refspecs_multiple()` and\n> `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These\n> functions focus on querying refspecs, so centralizing them in `refspec.c`\n> improves code organization by keeping refspec-related logic in one place.\n\nI think query_matches_negative_refspec() is appropriate named (not\nthat it matters much, as it becomes a mere private helper in the\nfile), unlike the ones in the first patch that are suboptimally\nnamed. query_refspecs() could probalby lose the plural 's' at the\nend---there is only single refspec, which is a collection of refspec\nitems, involved and it makes a single query---but otherwise it also\nhas an appropriate name (this matters a bit more, but not that much,\nas it was already public).\n\nquery_refspecs_multiple() is not a great name, though. It does not\nconvey what is multiple. Does it make multiple questions in one go?\nDoes it ask a question that can have multiple answers?\n\n> Signed-off-by: Meet Soni \n> ---\n> refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> refspec.h | 16 +++++++\n> remote.c | 122 -----------------------------------------------------\n> remote.h | 1 -\n> 4 files changed, 139 insertions(+), 123 deletions(-)\n\n> diff --git a/refspec.h b/refspec.h\n> index 891d50b159..d0788de782 100644\n> --- a/refspec.h\n> +++ b/refspec.h\n> @@ -30,6 +30,8 @@ struct refspec_item {\n> \tchar *raw;\n> };\n> \n> +struct string_list;\n> +\n> #define REFSPEC_FETCH 1\n> #define REFSPEC_PUSH 0\n> \n> @@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);\n> int match_name_with_pattern(const char *key, const char *name,\n> \t\t\t\t const char *value, char **result);\n> \n> +/*\n> + * Queries a refspec for a match and updates the query item.\n> + * Returns 0 on success, -1 if no match is found or negative refspec matches.\n> + */\n> +int query_refspecs(struct refspec *rs, struct refspec_item *query);\n\nThis one now has an excellent comment. Great job.\n\nThanks.\n"},{"id":"511300","messageId":"xmqqtt9kqak5.fsf@gitster.g","threadId":"62861","inReplyTo":"20250127103644.36627-4-meetsoni3017@gmail.com","subject":"Re: [PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-27T20:14:02Z","receivedAt":"2025-01-27T20:14:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Meet Soni writes:\n\n> +/*\n> + * Remove all entries in the input list which match any negative refspec in\n> + * the refspec list.\n> + */\n> +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n\nExcellent. \n\n ... it is merely moved from the original so it is not entirely\n your achievement, but still, this is good.\n\n> +/*\n> + * Applies refspecs to a name and returns the corresponding destination.\n> + * Returns the destination string if a match is found, NULL otherwise.\n> + */\n> +char *apply_refspecs(struct refspec *rs, const char *name);\n\nExplaining a function whose name has \"apply\" with a comment that\nuses \"apply\" as the verb does not add as much information as a\ncomment with a bit rephrased explanation. What does it mean to\n\"apply refspec to a name\" in the context of this function?\n"},{"id":"511380","messageId":"CAPhwyn34H1NgR5k67MBKEezwTJXtCLeiUhwKQkfVGcmKu7_v5A@mail.gmail.com","threadId":"62861","inReplyTo":"xmqqa5bctbnx.fsf@gitster.g","subject":"Re: [PATCH v2 1/3] refspec: relocate omit_name_by_refspec and related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-29T05:15:06Z","receivedAt":"2025-01-29T05:15:19Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Mon, 27 Jan 2025 at 22:51, Junio C Hamano wrote:\n>\n> Meet Soni writes:\n>\n> > Move the functions `omit_name_by_refspec()`, `refspec_match()`, and\n> > `match_name_with_pattern()` from `remote.c` to `refspec.c`. These\n> > functions focus on refspec matching, so placing them in `refspec.c`\n> > aligns with the separation of concerns. Keep refspec-related logic in\n> > `refspec.c` and remote-specific logic in `remote.c` for better code\n> > organization.\n> >\n> > Signed-off-by: Meet Soni \n> > ---\n> > ...\n> > diff --git a/refspec.h b/refspec.h\n> > index 69d693c87d..891d50b159 100644\n> > --- a/refspec.h\n> > +++ b/refspec.h\n> > @@ -71,4 +71,17 @@ struct strvec;\n> > void refspec_ref_prefixes(const struct refspec *rs,\n> > struct strvec *ref_prefixes);\n>\n> Back when these functions were mere local helper functions in\n> remote.c, their name being less descriptive of what they do may have\n> been OK (because readers have more context to understand them), but\n> when we make it a part of a public API, we should re-evaluate if\n> their names are good enough.\n>\n> > +/*\n> > + * Check whether a name matches any negative refspec in rs. Returns 1 if the\n> > + * name matches at least one negative refspec, and 0 otherwise.\n> > + */\n> > +int omit_name_by_refspec(const char *name, struct refspec *rs);\n>\n> Imagine you found this description in the header file and are trying\n> to figure out if it helps you writing the feature you are adding to\n> Git. Are the above description and the name of the function useful\n> enough to you?\n>\n> The first question that came to my mind was \"what is exactly a 'name'?\"\n>\n> In the context of the original, the caller iterates over a list of\n> \"struct ref\" and feeds the \"name\" member of the struct, but this\n> caller does not even have to know it is getting a part of \"struct\n> ref\"; it only cares about its parameter being a character string.\n>\n> In that context, is \"name\" the best identifer you can give to this\n> parameter? At least calling it \"refname\" might boost the signal the\n> name gives to the reader a bit better (and it is in line with how\n> refs.h calls these things).\n>\n> Another thing to consider is if the comment describes the purpose of\n> the function well, instead of just rephrasing what its\n> implementation does. What does it mean to return true iff there is\n> even one negative refspec that matches? What is the conceivable use\n> a caller would want to use such a function?\n>\n> As I said, calling it \"omit\" was probably OK in the context of the\n> original file, but it was already sloppy. This function merely\n> provides one bit of information (i.e. \"does it match any nagative\n> refspec---Yes or No?\"), and it is up to its caller how to use that\n> piece of information form.\n>\n> One of its callers, apply_negative_refspecs(), happens to use it to\n> filter a list of \"struct ref\" it received from its caller to drop\n> the refs from the list that match any negative refspec, but the\n> other existing caller does not even filter or omit anything from a\n> collection it has.\n>\n> My personal preference is to do this kind of change in two separate\n> patches:\n>\n> (1) as a preliminary clean-up, we rename functions and their\n> parameters in the original place; if needed, add clarifying\n> comments.\n>\n> (2) move the resulting functions with the comments to their new\n> home.\n>\n> If these two step conversions results in\n>\n> extern int refname_matches_negative_refspec_item\n> (const char *refname, struct refspec *refspec);\n>\n> I suspect that it is clear enough that there is no need for any\n> extra comment to explain what it does.\n>\nMakes sense. I'll implement this in the upcoming version of this patch.\n\nSince I’ve already prepared a patch for moving the function in the current\nseries, I’ll add a commit to handle the renaming and changing comments.\n\n> > +/*\n> > + * Checks whether a name matches a pattern and optionally generates a result.\n> > + * Returns 1 if the name matches the pattern, 0 otherwise.\n> > + */\n> > +int match_name_with_pattern(const char *key, const char *name,\n> > + const char *value, char **result);\n> > +\n>\n> As this is merely moved from an existing header, I am tempted to say\n> I'll leave it as an exercise to the readers to improve this one, as\n> improving it is outside the scope of this work.\n>\n> Some hints for those who want to tackle the clean-up for extra\n> points, perhaps after the dust settles from this series.\n>\n> The \"pattern\" in the name refers to the src side of a globbing\n> refspec and is passed in the parameter \"key\", so we are calling the\n> same thing in three different names, which is already triply bad.\n>\n> \"optionally generates a result\" does not convey any meaning outside\n> the context of the original, as it does not even talk about what\n> computation is creating the result. It does not even say what\n> controls the optionality---without reading the implementation, it is\n> likely your readers would assume passing NULL to result is all it\n> takes to skip that optional feature, but that is not the case.\n>\n> If I understand correctly, here is what this one does.\n>\n> It takes the source side of a globbing refspec item (e.g.\n> \"refs/heads/*\" in \"refs/heads/*:refs/remotes/origin/*\"), a\n> refname that might match the glob pattern, the destination side\n> of the refspec item (e.g. \"refs/remotes/origin/*\" in the same\n> example), and a pointer that points at a variable to receive the\n> result. If the source pattern matches the given refname, apply\n> the source-to-destination mapping rule to compute the resulting\n> destination refname and store it in the result.\n>\n> The destination side is optional; if you do not need to map the\n> refname to another refname, but are merely interested if the\n> refname matches the glob pattern, you can pass NULL and result\n> location is not touched.\n>\n> In either case, returns true iff the source side of the globbing\n> refspec item matches the given refname.\n>\n> So \"name\" in the function name should probably become a bit\n> narrower, like \"refname\". Also the names of its parameters need to\n> be better thought out.\n\nI agree that the function and its parameters could be improved for clarity.\nSince you mentioned leaving it as an exercise for readers, I’m happy to\ntake it up and write a follow-up patch to address these issues after\nfinishing the current series, if that works.\n"},{"id":"511381","messageId":"CAPhwyn3za29WwtFFJJodHXOpVRFuq8QhByE8ixjPPq9oyxfCmQ@mail.gmail.com","threadId":"62861","inReplyTo":"xmqqikq0ruuk.fsf@gitster.g","subject":"Re: [PATCH v2 0/3] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-29T05:18:50Z","receivedAt":"2025-01-29T05:19:03Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Mon, 27 Jan 2025 at 23:40, Junio C Hamano wrote:\n>\n> Meet Soni writes:\n>\n> > Thank you for reviewing :)\n> >\n> > I've added documentation comments for various function signatures to\n> > better understand what they do.\n>\n> Before saying all that, please help those who haven't read the\n> previous round (which wasn't even v1 IIRC but RFC and may have been\n> skipped by some potential reviewers) by summarizing what this series\n> is about. For other's convenience, here is a key excerpt from the\n> cover letter of the previous iteration:\n>\n> As Patrick pointed out in [1], the logic related to refspec is currently\n> split across multiple headers. This patch series addresses that by\n> relocating refspec-related logic from remote to refspec for improved\n> cohesion.\n>\nUnderstood.\n\n> While I was working on an unrelated issue, I noticed that there is\n> one function, \"extern int valid_remote_name(const char *);\" declared\n> in which is only about a remote and should probably be\n> moved to ; cleaning it up does not have to be part of this\n> series, but since you are doing a similar clean-up effort, I thought\n> you would want to be aware of it.\n>\n> Thanks.\nThank you for pointing this out. I’ll be happy to write up a patch after\nthis series is done.\n"},{"id":"511384","messageId":"CAPhwyn110E39uksCbSNYy3wRrxmG2QuuXEvPRrUT2SSTLxCKcQ@mail.gmail.com","threadId":"62861","inReplyTo":"xmqq7c6grrdl.fsf@gitster.g","subject":"Re: [PATCH v2 2/3] refspec: relocate query related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-29T06:32:20Z","receivedAt":"2025-01-29T06:32:34Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Tue, 28 Jan 2025 at 00:55, Junio C Hamano wrote:\n>\n> Meet Soni writes:\n>\n> > Move the functions `query_refspecs()`, `query_refspecs_multiple()` and\n> > `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These\n> > functions focus on querying refspecs, so centralizing them in `refspec.c`\n> > improves code organization by keeping refspec-related logic in one place.\n>\n> I think query_matches_negative_refspec() is appropriate named (not\n> that it matters much, as it becomes a mere private helper in the\n> file), unlike the ones in the first patch that are suboptimally\n> named. query_refspecs() could probalby lose the plural 's' at the\n> end---there is only single refspec, which is a collection of refspec\n> items, involved and it makes a single query---but otherwise it also\n> has an appropriate name (this matters a bit more, but not that much,\n> as it was already public).\n>\n> query_refspecs_multiple() is not a great name, though. It does not\n> convey what is multiple. Does it make multiple questions in one go?\n> Does it ask a question that can have multiple answers?\n>\nI agree that the original names are ambiguous. query_refspecs_multiple()\nis similar to query_refspecs(), but instead of returning the first match, it\ncollects all matching results.\n\nTo improve clarity and consistency, I’d like to propose the following\nrenames:\n *query_refspecs() -> find_refspec_match()\n `find` better describes its purpose than `query` and `match`\n clarifies that it’s looking for a single result.\n\n *query_refspecs_multiple() -> find_all_refspec_matches()\n Unlike the previous function, this one collects all matching results\n instead of stopping at the first match. The new name highlights that\n it returns multiple matches.\nLet me know what you think!\n\n> > Signed-off-by: Meet Soni \n> > ---\n> > refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> > refspec.h | 16 +++++++\n> > remote.c | 122 -----------------------------------------------------\n> > remote.h | 1 -\n> > 4 files changed, 139 insertions(+), 123 deletions(-)\n>\n> > diff --git a/refspec.h b/refspec.h\n> > index 891d50b159..d0788de782 100644\n> > --- a/refspec.h\n> > +++ b/refspec.h\n> > @@ -30,6 +30,8 @@ struct refspec_item {\n> > char *raw;\n> > };\n> >\n> > +struct string_list;\n> > +\n> > #define REFSPEC_FETCH 1\n> > #define REFSPEC_PUSH 0\n> >\n> > @@ -84,4 +86,18 @@ int omit_name_by_refspec(const char *name, struct refspec *rs);\n> > int match_name_with_pattern(const char *key, const char *name,\n> > const char *value, char **result);\n> >\n> > +/*\n> > + * Queries a refspec for a match and updates the query item.\n> > + * Returns 0 on success, -1 if no match is found or negative refspec matches.\n> > + */\n> > +int query_refspecs(struct refspec *rs, struct refspec_item *query);\n>\n> This one now has an excellent comment. Great job.\n>\n> Thanks.\n"},{"id":"511385","messageId":"CAPhwyn0tsU-0Tw44DeZssBJnjDpL_mYF6OBGT5sTdSYEy+k4cw@mail.gmail.com","threadId":"62861","inReplyTo":"xmqqtt9kqak5.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] refspec: relocate apply_refspecs and related funtions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-01-29T07:03:52Z","receivedAt":"2025-01-29T07:04:05Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Tue, 28 Jan 2025 at 01:44, Junio C Hamano wrote:\n>\n> Meet Soni writes:\n>\n> > +/*\n> > + * Applies refspecs to a name and returns the corresponding destination.\n> > + * Returns the destination string if a match is found, NULL otherwise.\n> > + */\n> > +char *apply_refspecs(struct refspec *rs, const char *name);\n>\n> Explaining a function whose name has \"apply\" with a comment that\n> uses \"apply\" as the verb does not add as much information as a\n> comment with a bit rephrased explanation. What does it mean to\n> \"apply refspec to a name\" in the context of this function?\n\nThe term \"apply\" was intended to convey the idea of mapping the\nrefspec with the given name, but it’s more helpful to describe the\nfunction’s behavior more explicitly.\n\nI’ll update the comment in the next version of this series.\nHere’s the revised comment I plan to use:\n\n/*\n * Search for a refspec that matches the given name and return the\n * corresponding destination (dst) if a match is found, NULL otherwise.\n */\n"},{"id":"511619","messageId":"20250201064202.76116-1-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250127103644.36627-1-meetsoni3017@gmail.com","subject":"[PATCH v3 0/5] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:41:57Z","receivedAt":"2025-02-01T06:42:25Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"As Patrick pointed out in [1], the logic related to refspec is currently\nsplit across multiple headers. This patch series addresses that by\nrenaming and relocating refspec-related logic from remote to refspec for\nimproved cohesion.\n\n[1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/\n\nSpecifically, the following changes have been made:\n\n Refactoring and renaming functions: Functions such as\n omit_name_by_refspec() have been renamed to better reflect their\n functionality. \n\n Relocation of functions: Functions that are primarily responsible\n for refspec related functionality, have been relocated from remote.c\n to refspec.c to maintain a clear separation of concerns.\n\nThank you for considering this patch.\nMeet\n\n\nMeet Soni (5):\n refactor(remote): rename function omit_name_by_refspec\n refspec: relocate refname_matches_negative_refspec_item\n refactor(remote): rename query_refspecs functions\n refspec: relocate matching related functions\n refspec: relocate apply_refspecs and related funtions\n\n builtin/push.c | 2 +-\n builtin/remote.c | 2 +-\n refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 37 +++++++++\n remote.c | 205 +----------------------------------------------\n remote.h | 15 ----\n 6 files changed, 244 insertions(+), 220 deletions(-)\n\nRange-diff against v2:\n-: ---------- > 1: 399e59ff67 refactor(remote): rename function omit_name_by_refspec\n1: 8e393ea1c2 ! 2: 4109b2bd1c refspec: relocate omit_name_by_refspec and related functions\n @@ Metadata\n Author: Meet Soni \n \n ## Commit message ##\n - refspec: relocate omit_name_by_refspec and related functions\n + refspec: relocate refname_matches_negative_refspec_item\n \n - Move the functions `omit_name_by_refspec()`, `refspec_match()`, and\n - `match_name_with_pattern()` from `remote.c` to `refspec.c`. These\n - functions focus on refspec matching, so placing them in `refspec.c`\n - aligns with the separation of concerns. Keep refspec-related logic in\n - `refspec.c` and remote-specific logic in `remote.c` for better code\n - organization.\n + Move the functions `refname_matches_negative_refspec_item()`,\n + `refspec_match()`, and `match_name_with_pattern()` from `remote.c` to\n + `refspec.c`. These functions focus on refspec matching, so placing them\n + in `refspec.c` aligns with the separation of concerns. Keep\n + refspec-related logic in `refspec.c` and remote-specific logic in\n + `remote.c` for better code organization.\n \n Signed-off-by: Meet Soni \n \n @@ refspec.c: void refspec_ref_prefixes(const struct refspec *rs,\n +\treturn !strcmp(refspec->src, name);\n +}\n +\n -+int omit_name_by_refspec(const char *name, struct refspec *rs)\n ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n +{\n +\tint i;\n +\n +\tfor (i = 0; i < rs->nr; i++) {\n -+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n ++\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n +\t\t\treturn 1;\n +\t}\n +\treturn 0;\n @@ refspec.h: struct strvec;\n void refspec_ref_prefixes(const struct refspec *rs,\n \t\t\t struct strvec *ref_prefixes);\n \n -+/*\n -+ * Check whether a name matches any negative refspec in rs. Returns 1 if the\n -+ * name matches at least one negative refspec, and 0 otherwise.\n -+ */\n -+int omit_name_by_refspec(const char *name, struct refspec *rs);\n ++int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n +\n +/*\n + * Checks whether a name matches a pattern and optionally generates a result.\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\treturn !strcmp(refspec->src, name);\n -}\n -\n --int omit_name_by_refspec(const char *name, struct refspec *rs)\n +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n -{\n -\tint i;\n -\n -\tfor (i = 0; i < rs->nr; i++) {\n --\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n +-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n -\t\t\treturn 1;\n -\t}\n -\treturn 0;\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n {\n \tstruct ref **tail;\n -\n - ## remote.h ##\n -@@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);\n - */\n - struct ref *ref_remove_duplicates(struct ref *ref_map);\n - \n --/*\n -- * Check whether a name matches any negative refspec in rs. Returns 1 if the\n -- * name matches at least one negative refspec, and 0 otherwise.\n -- */\n --int omit_name_by_refspec(const char *name, struct refspec *rs);\n --\n - /*\n - * Remove all entries in the input list which match any negative refspec in\n - * the refspec list.\n-: ---------- > 3: 559224864f refactor(remote): rename query_refspecs functions\n2: ef6edbc15b ! 4: 13e49509fc refspec: relocate query related functions\n @@ Metadata\n Author: Meet Soni \n \n ## Commit message ##\n - refspec: relocate query related functions\n + refspec: relocate matching related functions\n \n - Move the functions `query_refspecs()`, `query_refspecs_multiple()` and\n - `query_matches_negative_refspec()` from `remote.c` to `refspec.c`. These\n - functions focus on querying refspecs, so centralizing them in `refspec.c`\n - improves code organization by keeping refspec-related logic in one place.\n + Move the functions `find_refspec_match()`, `find_all_refspec_matches()`\n + and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.\n + These functions focus on matching refspecs, so centralizing them in\n + `refspec.c` improves code organization by keeping refspec-related logic\n + in one place.\n \n Signed-off-by: Meet Soni \n \n @@ refspec.c\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n -@@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +@@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n \t}\n \treturn 0;\n }\n +\n -+static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n ++static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n +{\n +\tint i, matched_negative = 0;\n +\tint find_src = !query->src;\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +\t}\n +\n +\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n -+\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n ++\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n +\t\t\tmatched_negative = 1;\n +\t}\n +\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +\treturn matched_negative;\n +}\n +\n -+void query_refspecs_multiple(struct refspec *rs,\n ++void find_all_refspec_matches(struct refspec *rs,\n +\t\t\t\t struct refspec_item *query,\n +\t\t\t\t struct string_list *results)\n +{\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +\tint find_src = !query->src;\n +\n +\tif (find_src && !query->dst)\n -+\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n ++\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n +\n -+\tif (query_matches_negative_refspec(rs, query))\n ++\tif (find_negative_refspec_match(rs, query))\n +\t\treturn;\n +\n +\tfor (i = 0; i < rs->nr; i++) {\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +\t}\n +}\n +\n -+int query_refspecs(struct refspec *rs, struct refspec_item *query)\n ++int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n +{\n +\tint i;\n +\tint find_src = !query->src;\n @@ refspec.c: int omit_name_by_refspec(const char *name, struct refspec *rs)\n +\tchar **result = find_src ? &query->src : &query->dst;\n +\n +\tif (find_src && !query->dst)\n -+\t\tBUG(\"query_refspecs: need either src or dst\");\n ++\t\tBUG(\"find_refspec_match: need either src or dst\");\n +\n -+\tif (query_matches_negative_refspec(rs, query))\n ++\tif (find_negative_refspec_match(rs, query))\n +\t\treturn -1;\n +\n +\tfor (i = 0; i < rs->nr; i++) {\n @@ refspec.h: struct refspec_item {\n #define REFSPEC_FETCH 1\n #define REFSPEC_PUSH 0\n \n -@@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);\n +@@ refspec.h: int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t const char *value, char **result);\n \n @@ refspec.h: int omit_name_by_refspec(const char *name, struct refspec *rs);\n + * Queries a refspec for a match and updates the query item.\n + * Returns 0 on success, -1 if no match is found or negative refspec matches.\n + */\n -+int query_refspecs(struct refspec *rs, struct refspec_item *query);\n ++int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n +\n +/*\n + * Queries a refspec for all matches and appends results to the provided string\n + * list.\n + */\n -+void query_refspecs_multiple(struct refspec *rs,\n ++void find_all_refspec_matches(struct refspec *rs,\n +\t\t\t\t struct refspec_item *query,\n +\t\t\t\t struct string_list *results);\n +\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n \treturn ref_map;\n }\n \n --static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n +-static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n -{\n -\tint i, matched_negative = 0;\n -\tint find_src = !query->src;\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\t}\n -\n -\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n --\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n +-\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n -\t\t\tmatched_negative = 1;\n -\t}\n -\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\treturn matched_negative;\n -}\n -\n --static void query_refspecs_multiple(struct refspec *rs,\n +-static void find_all_refspec_matches(struct refspec *rs,\n -\t\t\t\t struct refspec_item *query,\n -\t\t\t\t struct string_list *results)\n -{\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\tint find_src = !query->src;\n -\n -\tif (find_src && !query->dst)\n --\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n +-\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n -\n --\tif (query_matches_negative_refspec(rs, query))\n +-\tif (find_negative_refspec_match(rs, query))\n -\t\treturn;\n -\n -\tfor (i = 0; i < rs->nr; i++) {\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\t}\n -}\n -\n --int query_refspecs(struct refspec *rs, struct refspec_item *query)\n +-int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n -{\n -\tint i;\n -\tint find_src = !query->src;\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\tchar **result = find_src ? &query->src : &query->dst;\n -\n -\tif (find_src && !query->dst)\n --\t\tBUG(\"query_refspecs: need either src or dst\");\n +-\t\tBUG(\"find_refspec_match: need either src or dst\");\n -\n --\tif (query_matches_negative_refspec(rs, query))\n +-\tif (find_negative_refspec_match(rs, query))\n -\t\treturn -1;\n -\n -\tfor (i = 0; i < rs->nr; i++) {\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n char *apply_refspecs(struct refspec *rs, const char *name)\n {\n \tstruct refspec_item query;\n -\n - ## remote.h ##\n -@@ remote.h: struct ref *ref_remove_duplicates(struct ref *ref_map);\n - */\n - struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n - \n --int query_refspecs(struct refspec *rs, struct refspec_item *query);\n - char *apply_refspecs(struct refspec *rs, const char *name);\n - \n - int check_push_refs(struct ref *src, struct refspec *rs);\n3: ea72647439 ! 5: 891e01be93 refspec: relocate apply_refspecs and related funtions\n @@ refspec.c\n #include \"strbuf.h\"\n \n /*\n -@@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)\n +@@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)\n +\tfor (tail = &ref_map; *tail; ) {\n +\t\tstruct ref *ref = *tail;\n +\n -+\t\tif (omit_name_by_refspec(ref->name, rs)) {\n ++\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n +\t\t\t*tail = ref->next;\n +\t\t\tfree(ref->peer_ref);\n +\t\t\tfree(ref);\n @@ refspec.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)\n +\tmemset(&query, 0, sizeof(struct refspec_item));\n +\tquery.src = (char *)name;\n +\n -+\tif (query_refspecs(rs, &query))\n ++\tif (find_refspec_match(rs, &query))\n +\t\treturn NULL;\n +\n +\treturn query.dst;\n +}\n \n ## refspec.h ##\n -@@ refspec.h: void query_refspecs_multiple(struct refspec *rs,\n +@@ refspec.h: void find_all_refspec_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results);\n \n @@ refspec.h: void query_refspecs_multiple(struct refspec *rs,\n +struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n +\n +/*\n -+ * Applies refspecs to a name and returns the corresponding destination.\n -+ * Returns the destination string if a match is found, NULL otherwise.\n ++ * Search for a refspec that matches the given name and return the\n ++ * corresponding destination (dst) if a match is found, NULL otherwise.\n + */\n +char *apply_refspecs(struct refspec *rs, const char *name);\n +\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\tfor (tail = &ref_map; *tail; ) {\n -\t\tstruct ref *ref = *tail;\n -\n --\t\tif (omit_name_by_refspec(ref->name, rs)) {\n +-\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n -\t\t\t*tail = ref->next;\n -\t\t\tfree(ref->peer_ref);\n -\t\t\tfree(ref);\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\tmemset(&query, 0, sizeof(struct refspec_item));\n -\tquery.src = (char *)name;\n -\n --\tif (query_refspecs(rs, &query))\n +-\tif (find_refspec_match(rs, &query))\n -\t\treturn NULL;\n -\n -\treturn query.dst;\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n - \treturn query_refspecs(&remote->fetch, refspec);\n + \treturn find_refspec_match(&remote->fetch, refspec);\n \n ## remote.h ##\n @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n +-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n +-\n -/*\n - * Remove all entries in the input list which match any negative refspec in\n - * the refspec list.\n - */\n -struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n -\n +-int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n -char *apply_refspecs(struct refspec *rs, const char *name);\n -\n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511620","messageId":"20250201064202.76116-2-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[PATCH v3 1/5] refactor(remote): rename function omit_name_by_refspec","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:41:58Z","receivedAt":"2025-02-01T06:42:48Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Rename the function `omit_name_by_refspec()` to\n`refname_matches_negative_refspec_item()` to provide clearer intent.\nThe previous function name was vague and did not accurately describe its\npurpose. By using `refname_matches_negative_refspec_item`, make the\nfunction's purpose more intuitive, clarifying that it checks if a\nreference name matches any negative refspec.\n\nRename function parameters for consistency with existing naming\nconventions. Use `refname` instead of `name` to align with terminology\nin `refs.h`.\n\nRemove the redundant doc comment since the function name is now\nself-explanatory.\n\nSigned-off-by: Meet Soni \n---\n builtin/remote.c | 2 +-\n remote.c | 8 ++++----\n remote.h | 6 +-----\n 3 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 0435963286..258b8895cd 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -383,7 +383,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat\n \t\t\t\tstates->remote->fetch.items[i].raw);\n \n \tfor (ref = fetch_map; ref; ref = ref->next) {\n-\t\tif (omit_name_by_refspec(ref->name, &states->remote->fetch))\n+\t\tif (refname_matches_negative_refspec_item(ref->name, &states->remote->fetch))\n \t\t\tstring_list_append(&states->skipped, abbrev_branch(ref->name));\n \t\telse if (!ref->peer_ref || !refs_ref_exists(get_main_ref_store(the_repository), ref->peer_ref->name))\n \t\t\tstring_list_append(&states->new_refs, abbrev_branch(ref->name));\ndiff --git a/remote.c b/remote.c\nindex 0f6fba8562..cb70ce6f3b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -944,12 +944,12 @@ static int refspec_match(const struct refspec_item *refspec,\n \treturn !strcmp(refspec->src, name);\n }\n \n-int omit_name_by_refspec(const char *name, struct refspec *rs)\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n {\n \tint i;\n \n \tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n@@ -962,7 +962,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \tfor (tail = &ref_map; *tail; ) {\n \t\tstruct ref *ref = *tail;\n \n-\t\tif (omit_name_by_refspec(ref->name, rs)) {\n+\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n \t\t\t*tail = ref->next;\n \t\t\tfree(ref->peer_ref);\n \t\t\tfree(ref);\n@@ -1021,7 +1021,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite\n \t}\n \n \tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n+\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n \t\t\tmatched_negative = 1;\n \t}\n \ndiff --git a/remote.h b/remote.h\nindex bda10dd5c8..66ee53411d 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Check whether a name matches any negative refspec in rs. Returns 1 if the\n- * name matches at least one negative refspec, and 0 otherwise.\n- */\n-int omit_name_by_refspec(const char *name, struct refspec *rs);\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n \n /*\n * Remove all entries in the input list which match any negative refspec in\n-- \n2.34.1\n\n"},{"id":"511621","messageId":"20250201064202.76116-3-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[PATCH v3 2/5] refspec: relocate refname_matches_negative_refspec_item","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:41:59Z","receivedAt":"2025-02-01T06:43:01Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `refname_matches_negative_refspec_item()`,\n`refspec_match()`, and `match_name_with_pattern()` from `remote.c` to\n`refspec.c`. These functions focus on refspec matching, so placing them\nin `refspec.c` aligns with the separation of concerns. Keep\nrefspec-related logic in `refspec.c` and remote-specific logic in\n`remote.c` for better code organization.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 9 +++++++++\n remote.c | 48 ------------------------------------------------\n 3 files changed, 57 insertions(+), 48 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 6d86e04442..b447768304 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,\n \t\t}\n \t}\n }\n+\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result)\n+{\n+\tconst char *kstar = strchr(key, '*');\n+\tsize_t klen;\n+\tsize_t ksuffixlen;\n+\tsize_t namelen;\n+\tint ret;\n+\tif (!kstar)\n+\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n+\tklen = kstar - key;\n+\tksuffixlen = strlen(kstar + 1);\n+\tnamelen = strlen(name);\n+\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n+\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n+\tif (ret && value) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *vstar = strchr(value, '*');\n+\t\tif (!vstar)\n+\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n+\t\tstrbuf_add(&sb, value, vstar - value);\n+\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n+\t\tstrbuf_addstr(&sb, vstar + 1);\n+\t\t*result = strbuf_detach(&sb, NULL);\n+\t}\n+\treturn ret;\n+}\n+\n+static int refspec_match(const struct refspec_item *refspec,\n+\t\t\t const char *name)\n+{\n+\tif (refspec->pattern)\n+\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n+\n+\treturn !strcmp(refspec->src, name);\n+}\n+\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 69d693c87d..584d9c9eb5 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -71,4 +71,13 @@ struct strvec;\n void refspec_ref_prefixes(const struct refspec *rs,\n \t\t\t struct strvec *ref_prefixes);\n \n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n+\n+/*\n+ * Checks whether a name matches a pattern and optionally generates a result.\n+ * Returns 1 if the name matches the pattern, 0 otherwise.\n+ */\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex cb70ce6f3b..1da8ec7037 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-static int match_name_with_pattern(const char *key, const char *name,\n-\t\t\t\t const char *value, char **result)\n-{\n-\tconst char *kstar = strchr(key, '*');\n-\tsize_t klen;\n-\tsize_t ksuffixlen;\n-\tsize_t namelen;\n-\tint ret;\n-\tif (!kstar)\n-\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n-\tklen = kstar - key;\n-\tksuffixlen = strlen(kstar + 1);\n-\tnamelen = strlen(name);\n-\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n-\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n-\tif (ret && value) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\tconst char *vstar = strchr(value, '*');\n-\t\tif (!vstar)\n-\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n-\t\tstrbuf_add(&sb, value, vstar - value);\n-\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n-\t\tstrbuf_addstr(&sb, vstar + 1);\n-\t\t*result = strbuf_detach(&sb, NULL);\n-\t}\n-\treturn ret;\n-}\n-\n-static int refspec_match(const struct refspec_item *refspec,\n-\t\t\t const char *name)\n-{\n-\tif (refspec->pattern)\n-\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n-\n-\treturn !strcmp(refspec->src, name);\n-}\n-\n-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n {\n \tstruct ref **tail;\n-- \n2.34.1\n\n"},{"id":"511622","messageId":"20250201064202.76116-4-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[PATCH v3 3/5] refactor(remote): rename query_refspecs functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:42:00Z","receivedAt":"2025-02-01T06:43:08Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Rename `query_refspecs()` to `find_refspec_match` for clarity, as it\nfinds a single matching refspec.\n\nRename `query_refspecs_multiple()` to `find_all_refspec_matches` to\nbetter reflect that it collects all matching refspecs instead of\nreturning just the first match.\n\nRename `query_matches_negative_refspec()` to\n`find_negative_refspec_match` for consistency with the updated naming\nconvention.\n\nSigned-off-by: Meet Soni \n---\n builtin/push.c | 2 +-\n remote.c | 20 ++++++++++----------\n remote.h | 2 +-\n 3 files changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 90de3746b5..e6527b0b04 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -78,7 +78,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,\n \t\t\t.src = matched->name,\n \t\t};\n \n-\t\tif (!query_refspecs(&remote->push, &query) && query.dst) {\n+\t\tif (!find_refspec_match(&remote->push, &query) && query.dst) {\n \t\t\trefspec_appendf(refspec, \"%s%s:%s\",\n \t\t\t\t\tquery.force ? \"+\" : \"\",\n \t\t\t\t\tquery.src, query.dst);\ndiff --git a/remote.c b/remote.c\nindex 1da8ec7037..4654bce5d4 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,7 +925,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i, matched_negative = 0;\n \tint find_src = !query->src;\n@@ -982,7 +982,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite\n \treturn matched_negative;\n }\n \n-static void query_refspecs_multiple(struct refspec *rs,\n+static void find_all_refspec_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results)\n {\n@@ -990,9 +990,9 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n+\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n \n-\tif (query_matches_negative_refspec(rs, query))\n+\tif (find_negative_refspec_match(rs, query))\n \t\treturn;\n \n \tfor (i = 0; i < rs->nr; i++) {\n@@ -1013,7 +1013,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \t}\n }\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query)\n+int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i;\n \tint find_src = !query->src;\n@@ -1021,9 +1021,9 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs: need either src or dst\");\n+\t\tBUG(\"find_refspec_match: need either src or dst\");\n \n-\tif (query_matches_negative_refspec(rs, query))\n+\tif (find_negative_refspec_match(rs, query))\n \t\treturn -1;\n \n \tfor (i = 0; i < rs->nr; i++) {\n@@ -1054,7 +1054,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)\n \tmemset(&query, 0, sizeof(struct refspec_item));\n \tquery.src = (char *)name;\n \n-\tif (query_refspecs(rs, &query))\n+\tif (find_refspec_match(rs, &query))\n \t\treturn NULL;\n \n \treturn query.dst;\n@@ -1062,7 +1062,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)\n \n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n-\treturn query_refspecs(&remote->fetch, refspec);\n+\treturn find_refspec_match(&remote->fetch, refspec);\n }\n \n static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,\n@@ -2487,7 +2487,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED,\n \tmemset(&query, 0, sizeof(struct refspec_item));\n \tquery.dst = (char *)refname;\n \n-\tquery_refspecs_multiple(info->rs, &query, &matches);\n+\tfind_all_refspec_matches(info->rs, &query, &matches);\n \tif (matches.nr == 0)\n \t\tgoto clean_exit; /* No matches */\n \ndiff --git a/remote.h b/remote.h\nindex 66ee53411d..f109310eda 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -269,7 +269,7 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n */\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query);\n+int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n char *apply_refspecs(struct refspec *rs, const char *name);\n \n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511623","messageId":"20250201064202.76116-5-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[PATCH v3 4/5] refspec: relocate matching related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:42:01Z","receivedAt":"2025-02-01T06:43:12Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `find_refspec_match()`, `find_all_refspec_matches()`\nand `find_negative_refspec_match()` from `remote.c` to `refspec.c`.\nThese functions focus on matching refspecs, so centralizing them in\n`refspec.c` improves code organization by keeping refspec-related logic\nin one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 16 +++++++\n remote.c | 122 -----------------------------------------------------\n 3 files changed, 139 insertions(+), 122 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex b447768304..6634e7765d 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -5,6 +5,7 @@\n #include \"gettext.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n@@ -324,3 +325,125 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n \t}\n \treturn 0;\n }\n+\n+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i, matched_negative = 0;\n+\tint find_src = !query->src;\n+\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\n+\t/*\n+\t * Check whether the queried ref matches any negative refpsec. If so,\n+\t * then we should ultimately treat this as not matching the query at\n+\t * all.\n+\t *\n+\t * Note that negative refspecs always match the source, but the query\n+\t * item uses the destination. To handle this, we apply pattern\n+\t * refspecs in reverse to figure out if the query source matches any\n+\t * of the negative refspecs.\n+\t *\n+\t * The first loop finds and expands all positive refspecs\n+\t * matched by the queried ref.\n+\t *\n+\t * The second loop checks if any of the results of the first loop\n+\t * match any negative refspec.\n+\t */\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tchar *expn_name;\n+\n+\t\tif (refspec->negative)\n+\t\t\tcontinue;\n+\n+\t\t/* Note the reversal of src and dst */\n+\t\tif (refspec->pattern) {\n+\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n+\t\t\tconst char *value = refspec->src;\n+\n+\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n+\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n+\t\t} else if (refspec->matching) {\n+\t\t\t/* For the special matching refspec, any query should match */\n+\t\t\tstring_list_append(&reversed, needle);\n+\t\t} else if (!refspec->src) {\n+\t\t\tBUG(\"refspec->src should not be null here\");\n+\t\t} else if (!strcmp(needle, refspec->src)) {\n+\t\t\tstring_list_append(&reversed, refspec->src);\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n+\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n+\t\t\tmatched_negative = 1;\n+\t}\n+\n+\tstring_list_clear(&reversed, 0);\n+\n+\treturn matched_negative;\n+}\n+\n+void find_all_refspec_matches(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n+\n+\tif (find_negative_refspec_match(rs, query))\n+\t\treturn;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\t\tconst char *needle = find_src ? query->dst : query->src;\n+\t\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result))\n+\t\t\t\tstring_list_append_nodup(results, *result);\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\tstring_list_append(results, value);\n+\t\t}\n+\t}\n+}\n+\n+int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"find_refspec_match: need either src or dst\");\n+\n+\tif (find_negative_refspec_match(rs, query))\n+\t\treturn -1;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n+\t\t\t\tquery->force = refspec->force;\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\t*result = xstrdup(value);\n+\t\t\tquery->force = refspec->force;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn -1;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 584d9c9eb5..0393643bc8 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -30,6 +30,8 @@ struct refspec_item {\n \tchar *raw;\n };\n \n+struct string_list;\n+\n #define REFSPEC_FETCH 1\n #define REFSPEC_PUSH 0\n \n@@ -80,4 +82,18 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t const char *value, char **result);\n \n+/*\n+ * Queries a refspec for a match and updates the query item.\n+ * Returns 0 on success, -1 if no match is found or negative refspec matches.\n+ */\n+int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n+\n+/*\n+ * Queries a refspec for all matches and appends results to the provided string\n+ * list.\n+ */\n+void find_all_refspec_matches(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 4654bce5d4..858ab39471 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i, matched_negative = 0;\n-\tint find_src = !query->src;\n-\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\n-\t/*\n-\t * Check whether the queried ref matches any negative refpsec. If so,\n-\t * then we should ultimately treat this as not matching the query at\n-\t * all.\n-\t *\n-\t * Note that negative refspecs always match the source, but the query\n-\t * item uses the destination. To handle this, we apply pattern\n-\t * refspecs in reverse to figure out if the query source matches any\n-\t * of the negative refspecs.\n-\t *\n-\t * The first loop finds and expands all positive refspecs\n-\t * matched by the queried ref.\n-\t *\n-\t * The second loop checks if any of the results of the first loop\n-\t * match any negative refspec.\n-\t */\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tchar *expn_name;\n-\n-\t\tif (refspec->negative)\n-\t\t\tcontinue;\n-\n-\t\t/* Note the reversal of src and dst */\n-\t\tif (refspec->pattern) {\n-\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n-\t\t\tconst char *value = refspec->src;\n-\n-\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n-\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n-\t\t} else if (refspec->matching) {\n-\t\t\t/* For the special matching refspec, any query should match */\n-\t\t\tstring_list_append(&reversed, needle);\n-\t\t} else if (!refspec->src) {\n-\t\t\tBUG(\"refspec->src should not be null here\");\n-\t\t} else if (!strcmp(needle, refspec->src)) {\n-\t\t\tstring_list_append(&reversed, refspec->src);\n-\t\t}\n-\t}\n-\n-\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n-\t\t\tmatched_negative = 1;\n-\t}\n-\n-\tstring_list_clear(&reversed, 0);\n-\n-\treturn matched_negative;\n-}\n-\n-static void find_all_refspec_matches(struct refspec *rs,\n-\t\t\t\t struct refspec_item *query,\n-\t\t\t\t struct string_list *results)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n-\n-\tif (find_negative_refspec_match(rs, query))\n-\t\treturn;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\t\tconst char *needle = find_src ? query->dst : query->src;\n-\t\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result))\n-\t\t\t\tstring_list_append_nodup(results, *result);\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\tstring_list_append(results, value);\n-\t\t}\n-\t}\n-}\n-\n-int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"find_refspec_match: need either src or dst\");\n-\n-\tif (find_negative_refspec_match(rs, query))\n-\t\treturn -1;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n-\t\t\t\tquery->force = refspec->force;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\t*result = xstrdup(value);\n-\t\t\tquery->force = refspec->force;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn -1;\n-}\n-\n char *apply_refspecs(struct refspec *rs, const char *name)\n {\n \tstruct refspec_item query;\n-- \n2.34.1\n\n"},{"id":"511624","messageId":"20250201064202.76116-6-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[PATCH v3 5/5] refspec: relocate apply_refspecs and related funtions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-01T06:42:02Z","receivedAt":"2025-02-01T06:43:17Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `apply_refspecs()` and `apply_negative_refspecs()`\nfrom `remote.c` to `refspec.c`. These functions focus on applying\nrefspecs, so centralizing them in `refspec.c` improves code organization\nby keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 32 ++++++++++++++++++++++++++++++++\n refspec.h | 12 ++++++++++++\n remote.c | 31 -------------------------------\n remote.h | 11 -----------\n 4 files changed, 44 insertions(+), 42 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 6634e7765d..47974e86f0 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -9,6 +9,7 @@\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n+#include \"remote.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -447,3 +448,34 @@ int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n+\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n+{\n+\tstruct ref **tail;\n+\n+\tfor (tail = &ref_map; *tail; ) {\n+\t\tstruct ref *ref = *tail;\n+\n+\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n+\t\t\t*tail = ref->next;\n+\t\t\tfree(ref->peer_ref);\n+\t\t\tfree(ref);\n+\t\t} else\n+\t\t\ttail = &ref->next;\n+\t}\n+\n+\treturn ref_map;\n+}\n+\n+char *apply_refspecs(struct refspec *rs, const char *name)\n+{\n+\tstruct refspec_item query;\n+\n+\tmemset(&query, 0, sizeof(struct refspec_item));\n+\tquery.src = (char *)name;\n+\n+\tif (find_refspec_match(rs, &query))\n+\t\treturn NULL;\n+\n+\treturn query.dst;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 0393643bc8..5cbdc5f622 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -96,4 +96,16 @@ void find_all_refspec_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results);\n \n+/*\n+ * Remove all entries in the input list which match any negative refspec in\n+ * the refspec list.\n+ */\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n+\n+/*\n+ * Search for a refspec that matches the given name and return the\n+ * corresponding destination (dst) if a match is found, NULL otherwise.\n+ */\n+char *apply_refspecs(struct refspec *rs, const char *name);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 858ab39471..ad16d2493d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n-{\n-\tstruct ref **tail;\n-\n-\tfor (tail = &ref_map; *tail; ) {\n-\t\tstruct ref *ref = *tail;\n-\n-\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n-\t\t\t*tail = ref->next;\n-\t\t\tfree(ref->peer_ref);\n-\t\t\tfree(ref);\n-\t\t} else\n-\t\t\ttail = &ref->next;\n-\t}\n-\n-\treturn ref_map;\n-}\n-\n-char *apply_refspecs(struct refspec *rs, const char *name)\n-{\n-\tstruct refspec_item query;\n-\n-\tmemset(&query, 0, sizeof(struct refspec_item));\n-\tquery.src = (char *)name;\n-\n-\tif (find_refspec_match(rs, &query))\n-\t\treturn NULL;\n-\n-\treturn query.dst;\n-}\n-\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n \treturn find_refspec_match(&remote->fetch, refspec);\ndiff --git a/remote.h b/remote.h\nindex f109310eda..b4bb16af0e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,17 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n-\n-/*\n- * Remove all entries in the input list which match any negative refspec in\n- * the refspec list.\n- */\n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n-\n-int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n-char *apply_refspecs(struct refspec *rs, const char *name);\n-\n int check_push_refs(struct ref *src, struct refspec *rs);\n int match_push_refs(struct ref *src, struct ref **dst,\n \t\t struct refspec *rs, int flags);\n-- \n2.34.1\n\n"},{"id":"511689","messageId":"Z6BmIGIJYq5D2ZWO@pks.im","threadId":"62861","inReplyTo":"20250201064202.76116-2-meetsoni3017@gmail.com","subject":"Re: [PATCH v3 1/5] refactor(remote): rename function omit_name_by_refspec","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-03T06:45:52Z","receivedAt":"2025-02-03T06:46:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Feb 01, 2025 at 12:11:58PM +0530, Meet Soni wrote:\n\nPlease drop the `refactor()` bit from the commit subject, we don't use\nthese prefixes here. You also did the same in a later commit.\n\n> Rename the function `omit_name_by_refspec()` to\n> `refname_matches_negative_refspec_item()` to provide clearer intent.\n> The previous function name was vague and did not accurately describe its\n> purpose. By using `refname_matches_negative_refspec_item`, make the\n> function's purpose more intuitive, clarifying that it checks if a\n> reference name matches any negative refspec.\n\nThe new name certainly reads way better, and the changes themselves look\n\nPatrick\n"},{"id":"511688","messageId":"Z6BmKO-034bqOCjS@pks.im","threadId":"62861","inReplyTo":"20250201064202.76116-4-meetsoni3017@gmail.com","subject":"Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-03T06:46:00Z","receivedAt":"2025-02-03T06:46:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Feb 01, 2025 at 12:12:00PM +0530, Meet Soni wrote:\n> Rename `query_refspecs()` to `find_refspec_match` for clarity, as it\n> finds a single matching refspec.\n> \n> Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to\n> better reflect that it collects all matching refspecs instead of\n> returning just the first match.\n> \n> Rename `query_matches_negative_refspec()` to\n> `find_negative_refspec_match` for consistency with the updated naming\n> convention.\n\nOkay. The message might've read a tiny bit easier if it was a bulleted\nlist of renames. E.g.:\n\n We're about to move a couple of functions related to handling of\n refspecs from \"remote.c\" into \"refspec.c\". In preparation for this\n move, rename them to better reflect their intent:\n\n - `query_refspecs()` becomes `find_refspec_match()` for clarity,\n as it finds a single matching refspec.\n\n ...\n\nI was wondering a bit about why we rename the static functions, as we\nwouldn't have to expose them in a subsequent step anyway. Other than\nthat I think we should adhere to our coding guidelines with the renamed\npublic functions:\n\n The primary data structure that a subsystem 'S' deals with is called\n `struct S`. Functions that operate on `struct S` are named\n `S_()` and should generally receive a pointer to `struct S` as\n first parameter. E.g.\n\nSo:\n\n - `query_refspecs()` would be renamed to `refspec_find_match()`.\n\n - `query_refspecs_multiple()` would be renamed to\n `refspec_find_all_matches()`.\n\n - `find_negative_refspec_match()` would be renamed to\n `refspec_find_negative_match()`.\n\nPatrick\n"},{"id":"511782","messageId":"CAPhwyn0-Hq5WHWvGzhqwafrJqmDic5+_S7hRxShk53d++hfw8A@mail.gmail.com","threadId":"62861","inReplyTo":"Z6BmKO-034bqOCjS@pks.im","subject":"Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T03:39:03Z","receivedAt":"2025-02-04T03:39:16Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Mon, 3 Feb 2025 at 12:16, Patrick Steinhardt wrote:\n>\n> On Sat, Feb 01, 2025 at 12:12:00PM +0530, Meet Soni wrote:\n> > Rename `query_refspecs()` to `find_refspec_match` for clarity, as it\n> > finds a single matching refspec.\n> >\n> > Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to\n> > better reflect that it collects all matching refspecs instead of\n> > returning just the first match.\n> >\n> > Rename `query_matches_negative_refspec()` to\n> > `find_negative_refspec_match` for consistency with the updated naming\n> > convention.\n>\n> Okay. The message might've read a tiny bit easier if it was a bulleted\n> list of renames. E.g.:\n>\n> We're about to move a couple of functions related to handling of\n> refspecs from \"remote.c\" into \"refspec.c\". In preparation for this\n> move, rename them to better reflect their intent:\n>\n> - `query_refspecs()` becomes `find_refspec_match()` for clarity,\n> as it finds a single matching refspec.\n>\n> ...\nMakes sense.\n>\n> I was wondering a bit about why we rename the static functions, as we\n> wouldn't have to expose them in a subsequent step anyway. Other than\n> that I think we should adhere to our coding guidelines with the renamed\n> public functions:\nSince we were renaming the query_* functions that are exposed, I updated\nthe static ones as well to maintain naming consistency across related functions.\n>\n> The primary data structure that a subsystem 'S' deals with is called\n> `struct S`. Functions that operate on `struct S` are named\n> `S_()` and should generally receive a pointer to `struct S` as\n> first parameter. E.g.\n>\n> So:\n>\n> - `query_refspecs()` would be renamed to `refspec_find_match()`.\n>\n> - `query_refspecs_multiple()` would be renamed to\n> `refspec_find_all_matches()`.\n>\n> - `find_negative_refspec_match()` would be renamed to\n> `refspec_find_negative_match()`.\n>\n> Patrick\n\nThanks for the review.\nMeet\n"},{"id":"511784","messageId":"20250204040558.34766-1-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250201064202.76116-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 0/5] refspec: centralize refspec-related logic","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:53Z","receivedAt":"2025-02-04T04:06:06Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Changes since v3:\n - updated commit message.\n - renamed functions as per review.\n - added GSoC mark , since the announcement has been made by google and\n we've started the discussion regarding the same.\n\nAdditional context of the series:\n Patrick pointed out in [1], the logic related to refspec is currently\n split across multiple headers. This patch series addresses that by\n renaming and relocating refspec-related logic from remote to refspec for\n improved cohesion.\n\n [1]: https://lore.kernel.org/git/ZysQvUyxgdRqjvj2@pks.im/\n\n Specifically, the following changes have been made:\n\n Refactoring and renaming functions: Functions such as\n omit_name_by_refspec() have been renamed to better reflect their\n functionality. \n\n Relocation of functions: Functions that are primarily responsible\n for refspec related functionality, have been relocated from remote.c\n to refspec.c to maintain a clear separation of concerns.\n\nMeet Soni (5):\n remote: rename function omit_name_by_refspec\n refspec: relocate refname_matches_negative_refspec_item\n remote: rename query_refspecs functions\n refspec: relocate matching related functions\n refspec: relocate apply_refspecs and related funtions\n\n builtin/push.c | 2 +-\n builtin/remote.c | 2 +-\n refspec.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 37 +++++++++\n remote.c | 205 +----------------------------------------------\n remote.h | 15 ----\n 6 files changed, 244 insertions(+), 220 deletions(-)\n\nRange-diff against v3:\n1: 399e59ff67 ! 1: 1b8606ffcb refactor(remote): rename function omit_name_by_refspec\n @@ Metadata\n Author: Meet Soni \n \n ## Commit message ##\n - refactor(remote): rename function omit_name_by_refspec\n + remote: rename function omit_name_by_refspec\n \n Rename the function `omit_name_by_refspec()` to\n `refname_matches_negative_refspec_item()` to provide clearer intent.\n2: 4109b2bd1c = 2: 3da817839c refspec: relocate refname_matches_negative_refspec_item\n3: 559224864f ! 3: bad0c43c96 refactor(remote): rename query_refspecs functions\n @@ Metadata\n Author: Meet Soni \n \n ## Commit message ##\n - refactor(remote): rename query_refspecs functions\n + remote: rename query_refspecs functions\n \n - Rename `query_refspecs()` to `find_refspec_match` for clarity, as it\n - finds a single matching refspec.\n + Rename functions related to handling refspecs in preparation for their\n + move from `remote.c` to `refspec.c`. Update their names to better\n + reflect their intent:\n \n - Rename `query_refspecs_multiple()` to `find_all_refspec_matches` to\n - better reflect that it collects all matching refspecs instead of\n - returning just the first match.\n + - `query_refspecs()` -> `refspec_find_match()` for clarity, as it\n + finds a single matching refspec.\n \n - Rename `query_matches_negative_refspec()` to\n - `find_negative_refspec_match` for consistency with the updated naming\n - convention.\n + - `query_refspecs_multiple()` -> `refspec_find_all_matches()` to\n + better reflect that it collects all matching refspecs instead of\n + returning just the first match.\n +\n + - `query_matches_negative_refspec()` ->\n + `refspec_find_negative_match()` for consistency with the\n + updated naming convention, even though this static function\n + didn't strictly require renaming.\n \n Signed-off-by: Meet Soni \n \n @@ builtin/push.c: static void refspec_append_mapped(struct refspec *refspec, const\n \t\t};\n \n -\t\tif (!query_refspecs(&remote->push, &query) && query.dst) {\n -+\t\tif (!find_refspec_match(&remote->push, &query) && query.dst) {\n ++\t\tif (!refspec_find_match(&remote->push, &query) && query.dst) {\n \t\t\trefspec_appendf(refspec, \"%s%s:%s\",\n \t\t\t\t\tquery.force ? \"+\" : \"\",\n \t\t\t\t\tquery.src, query.dst);\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n }\n \n -static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n -+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n ++static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i, matched_negative = 0;\n \tint find_src = !query->src;\n @@ remote.c: static int query_matches_negative_refspec(struct refspec *rs, struct r\n }\n \n -static void query_refspecs_multiple(struct refspec *rs,\n -+static void find_all_refspec_matches(struct refspec *rs,\n ++static void refspec_find_all_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results)\n {\n @@ remote.c: static void query_refspecs_multiple(struct refspec *rs,\n \n \tif (find_src && !query->dst)\n -\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n -+\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n ++\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n \n -\tif (query_matches_negative_refspec(rs, query))\n -+\tif (find_negative_refspec_match(rs, query))\n ++\tif (refspec_find_negative_match(rs, query))\n \t\treturn;\n \n \tfor (i = 0; i < rs->nr; i++) {\n @@ remote.c: static void query_refspecs_multiple(struct refspec *rs,\n }\n \n -int query_refspecs(struct refspec *rs, struct refspec_item *query)\n -+int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n ++int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i;\n \tint find_src = !query->src;\n @@ remote.c: int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \n \tif (find_src && !query->dst)\n -\t\tBUG(\"query_refspecs: need either src or dst\");\n -+\t\tBUG(\"find_refspec_match: need either src or dst\");\n ++\t\tBUG(\"refspec_find_match: need either src or dst\");\n \n -\tif (query_matches_negative_refspec(rs, query))\n -+\tif (find_negative_refspec_match(rs, query))\n ++\tif (refspec_find_negative_match(rs, query))\n \t\treturn -1;\n \n \tfor (i = 0; i < rs->nr; i++) {\n @@ remote.c: char *apply_refspecs(struct refspec *rs, const char *name)\n \tquery.src = (char *)name;\n \n -\tif (query_refspecs(rs, &query))\n -+\tif (find_refspec_match(rs, &query))\n ++\tif (refspec_find_match(rs, &query))\n \t\treturn NULL;\n \n \treturn query.dst;\n @@ remote.c: char *apply_refspecs(struct refspec *rs, const char *name)\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n -\treturn query_refspecs(&remote->fetch, refspec);\n -+\treturn find_refspec_match(&remote->fetch, refspec);\n ++\treturn refspec_find_match(&remote->fetch, refspec);\n }\n \n static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,\n @@ remote.c: static int get_stale_heads_cb(const char *refname, const char *referen\n \tquery.dst = (char *)refname;\n \n -\tquery_refspecs_multiple(info->rs, &query, &matches);\n -+\tfind_all_refspec_matches(info->rs, &query, &matches);\n ++\trefspec_find_all_matches(info->rs, &query, &matches);\n \tif (matches.nr == 0)\n \t\tgoto clean_exit; /* No matches */\n \n @@ remote.h: int refname_matches_negative_refspec_item(const char *refname, struct\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n \n -int query_refspecs(struct refspec *rs, struct refspec_item *query);\n -+int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n ++int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n char *apply_refspecs(struct refspec *rs, const char *name);\n \n int check_push_refs(struct ref *src, struct refspec *rs);\n4: 13e49509fc ! 4: 9a5dc26731 refspec: relocate matching related functions\n @@ Metadata\n ## Commit message ##\n refspec: relocate matching related functions\n \n - Move the functions `find_refspec_match()`, `find_all_refspec_matches()`\n - and `find_negative_refspec_match()` from `remote.c` to `refspec.c`.\n + Move the functions `refspec_find_match()`, `refspec_find_all_matches()`\n + and `refspec_find_negative_match()` from `remote.c` to `refspec.c`.\n These functions focus on matching refspecs, so centralizing them in\n `refspec.c` improves code organization by keeping refspec-related logic\n in one place.\n @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct\n \treturn 0;\n }\n +\n -+static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n ++static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n +{\n +\tint i, matched_negative = 0;\n +\tint find_src = !query->src;\n @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct\n +\treturn matched_negative;\n +}\n +\n -+void find_all_refspec_matches(struct refspec *rs,\n ++void refspec_find_all_matches(struct refspec *rs,\n +\t\t\t\t struct refspec_item *query,\n +\t\t\t\t struct string_list *results)\n +{\n @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct\n +\tint find_src = !query->src;\n +\n +\tif (find_src && !query->dst)\n -+\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n ++\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n +\n -+\tif (find_negative_refspec_match(rs, query))\n ++\tif (refspec_find_negative_match(rs, query))\n +\t\treturn;\n +\n +\tfor (i = 0; i < rs->nr; i++) {\n @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct\n +\t}\n +}\n +\n -+int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n ++int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n +{\n +\tint i;\n +\tint find_src = !query->src;\n @@ refspec.c: int refname_matches_negative_refspec_item(const char *refname, struct\n +\tchar **result = find_src ? &query->src : &query->dst;\n +\n +\tif (find_src && !query->dst)\n -+\t\tBUG(\"find_refspec_match: need either src or dst\");\n ++\t\tBUG(\"refspec_find_match: need either src or dst\");\n +\n -+\tif (find_negative_refspec_match(rs, query))\n ++\tif (refspec_find_negative_match(rs, query))\n +\t\treturn -1;\n +\n +\tfor (i = 0; i < rs->nr; i++) {\n @@ refspec.h: int refname_matches_negative_refspec_item(const char *refname, struct\n + * Queries a refspec for a match and updates the query item.\n + * Returns 0 on success, -1 if no match is found or negative refspec matches.\n + */\n -+int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n ++int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n +\n +/*\n + * Queries a refspec for all matches and appends results to the provided string\n + * list.\n + */\n -+void find_all_refspec_matches(struct refspec *rs,\n ++void refspec_find_all_matches(struct refspec *rs,\n +\t\t\t\t struct refspec_item *query,\n +\t\t\t\t struct string_list *results);\n +\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n \treturn ref_map;\n }\n \n --static int find_negative_refspec_match(struct refspec *rs, struct refspec_item *query)\n +-static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n -{\n -\tint i, matched_negative = 0;\n -\tint find_src = !query->src;\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\treturn matched_negative;\n -}\n -\n --static void find_all_refspec_matches(struct refspec *rs,\n +-static void refspec_find_all_matches(struct refspec *rs,\n -\t\t\t\t struct refspec_item *query,\n -\t\t\t\t struct string_list *results)\n -{\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\tint find_src = !query->src;\n -\n -\tif (find_src && !query->dst)\n --\t\tBUG(\"find_all_refspec_matches: need either src or dst\");\n +-\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n -\n --\tif (find_negative_refspec_match(rs, query))\n +-\tif (refspec_find_negative_match(rs, query))\n -\t\treturn;\n -\n -\tfor (i = 0; i < rs->nr; i++) {\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\t}\n -}\n -\n --int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n +-int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n -{\n -\tint i;\n -\tint find_src = !query->src;\n @@ remote.c: struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspe\n -\tchar **result = find_src ? &query->src : &query->dst;\n -\n -\tif (find_src && !query->dst)\n --\t\tBUG(\"find_refspec_match: need either src or dst\");\n +-\t\tBUG(\"refspec_find_match: need either src or dst\");\n -\n --\tif (find_negative_refspec_match(rs, query))\n +-\tif (refspec_find_negative_match(rs, query))\n -\t\treturn -1;\n -\n -\tfor (i = 0; i < rs->nr; i++) {\n5: 891e01be93 ! 5: f13ac6f11f refspec: relocate apply_refspecs and related funtions\n @@ refspec.c\n #include \"strbuf.h\"\n \n /*\n -@@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query)\n +@@ refspec.c: int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n @@ refspec.c: int find_refspec_match(struct refspec *rs, struct refspec_item *query\n +\tmemset(&query, 0, sizeof(struct refspec_item));\n +\tquery.src = (char *)name;\n +\n -+\tif (find_refspec_match(rs, &query))\n ++\tif (refspec_find_match(rs, &query))\n +\t\treturn NULL;\n +\n +\treturn query.dst;\n +}\n \n ## refspec.h ##\n -@@ refspec.h: void find_all_refspec_matches(struct refspec *rs,\n +@@ refspec.h: void refspec_find_all_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results);\n \n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\tmemset(&query, 0, sizeof(struct refspec_item));\n -\tquery.src = (char *)name;\n -\n --\tif (find_refspec_match(rs, &query))\n +-\tif (refspec_find_match(rs, &query))\n -\t\treturn NULL;\n -\n -\treturn query.dst;\n @@ remote.c: void ref_push_report_free(struct ref_push_report *report)\n -\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n - \treturn find_refspec_match(&remote->fetch, refspec);\n + \treturn refspec_find_match(&remote->fetch, refspec);\n \n ## remote.h ##\n @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);\n @@ remote.h: int resolve_remote_symref(struct ref *ref, struct ref *list);\n - */\n -struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n -\n --int find_refspec_match(struct refspec *rs, struct refspec_item *query);\n +-int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n -char *apply_refspecs(struct refspec *rs, const char *name);\n -\n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511785","messageId":"20250204040558.34766-2-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:54Z","receivedAt":"2025-02-04T04:06:09Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Rename the function `omit_name_by_refspec()` to\n`refname_matches_negative_refspec_item()` to provide clearer intent.\nThe previous function name was vague and did not accurately describe its\npurpose. By using `refname_matches_negative_refspec_item`, make the\nfunction's purpose more intuitive, clarifying that it checks if a\nreference name matches any negative refspec.\n\nRename function parameters for consistency with existing naming\nconventions. Use `refname` instead of `name` to align with terminology\nin `refs.h`.\n\nRemove the redundant doc comment since the function name is now\nself-explanatory.\n\nSigned-off-by: Meet Soni \n---\n builtin/remote.c | 2 +-\n remote.c | 8 ++++----\n remote.h | 6 +-----\n 3 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 0435963286..258b8895cd 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -383,7 +383,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat\n \t\t\t\tstates->remote->fetch.items[i].raw);\n \n \tfor (ref = fetch_map; ref; ref = ref->next) {\n-\t\tif (omit_name_by_refspec(ref->name, &states->remote->fetch))\n+\t\tif (refname_matches_negative_refspec_item(ref->name, &states->remote->fetch))\n \t\t\tstring_list_append(&states->skipped, abbrev_branch(ref->name));\n \t\telse if (!ref->peer_ref || !refs_ref_exists(get_main_ref_store(the_repository), ref->peer_ref->name))\n \t\t\tstring_list_append(&states->new_refs, abbrev_branch(ref->name));\ndiff --git a/remote.c b/remote.c\nindex 0f6fba8562..cb70ce6f3b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -944,12 +944,12 @@ static int refspec_match(const struct refspec_item *refspec,\n \treturn !strcmp(refspec->src, name);\n }\n \n-int omit_name_by_refspec(const char *name, struct refspec *rs)\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n {\n \tint i;\n \n \tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], name))\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n@@ -962,7 +962,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \tfor (tail = &ref_map; *tail; ) {\n \t\tstruct ref *ref = *tail;\n \n-\t\tif (omit_name_by_refspec(ref->name, rs)) {\n+\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n \t\t\t*tail = ref->next;\n \t\t\tfree(ref->peer_ref);\n \t\t\tfree(ref);\n@@ -1021,7 +1021,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite\n \t}\n \n \tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (omit_name_by_refspec(reversed.items[i].string, rs))\n+\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n \t\t\tmatched_negative = 1;\n \t}\n \ndiff --git a/remote.h b/remote.h\nindex bda10dd5c8..66ee53411d 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-/*\n- * Check whether a name matches any negative refspec in rs. Returns 1 if the\n- * name matches at least one negative refspec, and 0 otherwise.\n- */\n-int omit_name_by_refspec(const char *name, struct refspec *rs);\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n \n /*\n * Remove all entries in the input list which match any negative refspec in\n-- \n2.34.1\n\n"},{"id":"511786","messageId":"20250204040558.34766-3-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 2/5] refspec: relocate refname_matches_negative_refspec_item","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:55Z","receivedAt":"2025-02-04T04:06:13Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `refname_matches_negative_refspec_item()`,\n`refspec_match()`, and `match_name_with_pattern()` from `remote.c` to\n`refspec.c`. These functions focus on refspec matching, so placing them\nin `refspec.c` aligns with the separation of concerns. Keep\nrefspec-related logic in `refspec.c` and remote-specific logic in\n`remote.c` for better code organization.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 9 +++++++++\n remote.c | 48 ------------------------------------------------\n 3 files changed, 57 insertions(+), 48 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex 6d86e04442..b447768304 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -276,3 +276,51 @@ void refspec_ref_prefixes(const struct refspec *rs,\n \t\t}\n \t}\n }\n+\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result)\n+{\n+\tconst char *kstar = strchr(key, '*');\n+\tsize_t klen;\n+\tsize_t ksuffixlen;\n+\tsize_t namelen;\n+\tint ret;\n+\tif (!kstar)\n+\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n+\tklen = kstar - key;\n+\tksuffixlen = strlen(kstar + 1);\n+\tnamelen = strlen(name);\n+\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n+\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n+\tif (ret && value) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *vstar = strchr(value, '*');\n+\t\tif (!vstar)\n+\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n+\t\tstrbuf_add(&sb, value, vstar - value);\n+\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n+\t\tstrbuf_addstr(&sb, vstar + 1);\n+\t\t*result = strbuf_detach(&sb, NULL);\n+\t}\n+\treturn ret;\n+}\n+\n+static int refspec_match(const struct refspec_item *refspec,\n+\t\t\t const char *name)\n+{\n+\tif (refspec->pattern)\n+\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n+\n+\treturn !strcmp(refspec->src, name);\n+}\n+\n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 69d693c87d..584d9c9eb5 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -71,4 +71,13 @@ struct strvec;\n void refspec_ref_prefixes(const struct refspec *rs,\n \t\t\t struct strvec *ref_prefixes);\n \n+int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n+\n+/*\n+ * Checks whether a name matches a pattern and optionally generates a result.\n+ * Returns 1 if the name matches the pattern, 0 otherwise.\n+ */\n+int match_name_with_pattern(const char *key, const char *name,\n+\t\t\t\t const char *value, char **result);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex cb70ce6f3b..1da8ec7037 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,54 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-static int match_name_with_pattern(const char *key, const char *name,\n-\t\t\t\t const char *value, char **result)\n-{\n-\tconst char *kstar = strchr(key, '*');\n-\tsize_t klen;\n-\tsize_t ksuffixlen;\n-\tsize_t namelen;\n-\tint ret;\n-\tif (!kstar)\n-\t\tdie(_(\"key '%s' of pattern had no '*'\"), key);\n-\tklen = kstar - key;\n-\tksuffixlen = strlen(kstar + 1);\n-\tnamelen = strlen(name);\n-\tret = !strncmp(name, key, klen) && namelen >= klen + ksuffixlen &&\n-\t\t!memcmp(name + namelen - ksuffixlen, kstar + 1, ksuffixlen);\n-\tif (ret && value) {\n-\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\tconst char *vstar = strchr(value, '*');\n-\t\tif (!vstar)\n-\t\t\tdie(_(\"value '%s' of pattern has no '*'\"), value);\n-\t\tstrbuf_add(&sb, value, vstar - value);\n-\t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n-\t\tstrbuf_addstr(&sb, vstar + 1);\n-\t\t*result = strbuf_detach(&sb, NULL);\n-\t}\n-\treturn ret;\n-}\n-\n-static int refspec_match(const struct refspec_item *refspec,\n-\t\t\t const char *name)\n-{\n-\tif (refspec->pattern)\n-\t\treturn match_name_with_pattern(refspec->src, name, NULL, NULL);\n-\n-\treturn !strcmp(refspec->src, name);\n-}\n-\n-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tif (rs->items[i].negative && refspec_match(&rs->items[i], refname))\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n {\n \tstruct ref **tail;\n-- \n2.34.1\n\n"},{"id":"511787","messageId":"20250204040558.34766-4-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 3/5] remote: rename query_refspecs functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:56Z","receivedAt":"2025-02-04T04:06:17Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Rename functions related to handling refspecs in preparation for their\nmove from `remote.c` to `refspec.c`. Update their names to better\nreflect their intent:\n\n - `query_refspecs()` -> `refspec_find_match()` for clarity, as it\n finds a single matching refspec.\n\n - `query_refspecs_multiple()` -> `refspec_find_all_matches()` to\n better reflect that it collects all matching refspecs instead of\n returning just the first match.\n\n - `query_matches_negative_refspec()` ->\n `refspec_find_negative_match()` for consistency with the\n updated naming convention, even though this static function\n didn't strictly require renaming.\n\nSigned-off-by: Meet Soni \n---\n builtin/push.c | 2 +-\n remote.c | 20 ++++++++++----------\n remote.h | 2 +-\n 3 files changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 90de3746b5..92d530e5c4 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -78,7 +78,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,\n \t\t\t.src = matched->name,\n \t\t};\n \n-\t\tif (!query_refspecs(&remote->push, &query) && query.dst) {\n+\t\tif (!refspec_find_match(&remote->push, &query) && query.dst) {\n \t\t\trefspec_appendf(refspec, \"%s%s:%s\",\n \t\t\t\t\tquery.force ? \"+\" : \"\",\n \t\t\t\t\tquery.src, query.dst);\ndiff --git a/remote.c b/remote.c\nindex 1da8ec7037..b510809a56 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,7 +925,7 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int query_matches_negative_refspec(struct refspec *rs, struct refspec_item *query)\n+static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i, matched_negative = 0;\n \tint find_src = !query->src;\n@@ -982,7 +982,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite\n \treturn matched_negative;\n }\n \n-static void query_refspecs_multiple(struct refspec *rs,\n+static void refspec_find_all_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results)\n {\n@@ -990,9 +990,9 @@ static void query_refspecs_multiple(struct refspec *rs,\n \tint find_src = !query->src;\n \n \tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs_multiple: need either src or dst\");\n+\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n \n-\tif (query_matches_negative_refspec(rs, query))\n+\tif (refspec_find_negative_match(rs, query))\n \t\treturn;\n \n \tfor (i = 0; i < rs->nr; i++) {\n@@ -1013,7 +1013,7 @@ static void query_refspecs_multiple(struct refspec *rs,\n \t}\n }\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query)\n+int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n {\n \tint i;\n \tint find_src = !query->src;\n@@ -1021,9 +1021,9 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)\n \tchar **result = find_src ? &query->src : &query->dst;\n \n \tif (find_src && !query->dst)\n-\t\tBUG(\"query_refspecs: need either src or dst\");\n+\t\tBUG(\"refspec_find_match: need either src or dst\");\n \n-\tif (query_matches_negative_refspec(rs, query))\n+\tif (refspec_find_negative_match(rs, query))\n \t\treturn -1;\n \n \tfor (i = 0; i < rs->nr; i++) {\n@@ -1054,7 +1054,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)\n \tmemset(&query, 0, sizeof(struct refspec_item));\n \tquery.src = (char *)name;\n \n-\tif (query_refspecs(rs, &query))\n+\tif (refspec_find_match(rs, &query))\n \t\treturn NULL;\n \n \treturn query.dst;\n@@ -1062,7 +1062,7 @@ char *apply_refspecs(struct refspec *rs, const char *name)\n \n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n-\treturn query_refspecs(&remote->fetch, refspec);\n+\treturn refspec_find_match(&remote->fetch, refspec);\n }\n \n static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,\n@@ -2487,7 +2487,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED,\n \tmemset(&query, 0, sizeof(struct refspec_item));\n \tquery.dst = (char *)refname;\n \n-\tquery_refspecs_multiple(info->rs, &query, &matches);\n+\trefspec_find_all_matches(info->rs, &query, &matches);\n \tif (matches.nr == 0)\n \t\tgoto clean_exit; /* No matches */\n \ndiff --git a/remote.h b/remote.h\nindex 66ee53411d..516ba7f398 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -269,7 +269,7 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n */\n struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n \n-int query_refspecs(struct refspec *rs, struct refspec_item *query);\n+int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n char *apply_refspecs(struct refspec *rs, const char *name);\n \n int check_push_refs(struct ref *src, struct refspec *rs);\n-- \n2.34.1\n\n"},{"id":"511788","messageId":"20250204040558.34766-5-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 4/5] refspec: relocate matching related functions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:57Z","receivedAt":"2025-02-04T04:06:22Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `refspec_find_match()`, `refspec_find_all_matches()`\nand `refspec_find_negative_match()` from `remote.c` to `refspec.c`.\nThese functions focus on matching refspecs, so centralizing them in\n`refspec.c` improves code organization by keeping refspec-related logic\nin one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refspec.h | 16 +++++++\n remote.c | 122 -----------------------------------------------------\n 3 files changed, 139 insertions(+), 122 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex b447768304..cab0b0d127 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -5,6 +5,7 @@\n #include \"gettext.h\"\n #include \"hash.h\"\n #include \"hex.h\"\n+#include \"string-list.h\"\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n@@ -324,3 +325,125 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n \t}\n \treturn 0;\n }\n+\n+static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i, matched_negative = 0;\n+\tint find_src = !query->src;\n+\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\n+\t/*\n+\t * Check whether the queried ref matches any negative refpsec. If so,\n+\t * then we should ultimately treat this as not matching the query at\n+\t * all.\n+\t *\n+\t * Note that negative refspecs always match the source, but the query\n+\t * item uses the destination. To handle this, we apply pattern\n+\t * refspecs in reverse to figure out if the query source matches any\n+\t * of the negative refspecs.\n+\t *\n+\t * The first loop finds and expands all positive refspecs\n+\t * matched by the queried ref.\n+\t *\n+\t * The second loop checks if any of the results of the first loop\n+\t * match any negative refspec.\n+\t */\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tchar *expn_name;\n+\n+\t\tif (refspec->negative)\n+\t\t\tcontinue;\n+\n+\t\t/* Note the reversal of src and dst */\n+\t\tif (refspec->pattern) {\n+\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n+\t\t\tconst char *value = refspec->src;\n+\n+\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n+\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n+\t\t} else if (refspec->matching) {\n+\t\t\t/* For the special matching refspec, any query should match */\n+\t\t\tstring_list_append(&reversed, needle);\n+\t\t} else if (!refspec->src) {\n+\t\t\tBUG(\"refspec->src should not be null here\");\n+\t\t} else if (!strcmp(needle, refspec->src)) {\n+\t\t\tstring_list_append(&reversed, refspec->src);\n+\t\t}\n+\t}\n+\n+\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n+\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n+\t\t\tmatched_negative = 1;\n+\t}\n+\n+\tstring_list_clear(&reversed, 0);\n+\n+\treturn matched_negative;\n+}\n+\n+void refspec_find_all_matches(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n+\n+\tif (refspec_find_negative_match(rs, query))\n+\t\treturn;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\t\tconst char *needle = find_src ? query->dst : query->src;\n+\t\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result))\n+\t\t\t\tstring_list_append_nodup(results, *result);\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\tstring_list_append(results, value);\n+\t\t}\n+\t}\n+}\n+\n+int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n+{\n+\tint i;\n+\tint find_src = !query->src;\n+\tconst char *needle = find_src ? query->dst : query->src;\n+\tchar **result = find_src ? &query->src : &query->dst;\n+\n+\tif (find_src && !query->dst)\n+\t\tBUG(\"refspec_find_match: need either src or dst\");\n+\n+\tif (refspec_find_negative_match(rs, query))\n+\t\treturn -1;\n+\n+\tfor (i = 0; i < rs->nr; i++) {\n+\t\tstruct refspec_item *refspec = &rs->items[i];\n+\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n+\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n+\n+\t\tif (!refspec->dst || refspec->negative)\n+\t\t\tcontinue;\n+\t\tif (refspec->pattern) {\n+\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n+\t\t\t\tquery->force = refspec->force;\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t} else if (!strcmp(needle, key)) {\n+\t\t\t*result = xstrdup(value);\n+\t\t\tquery->force = refspec->force;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn -1;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex 584d9c9eb5..be20ba53ab 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -30,6 +30,8 @@ struct refspec_item {\n \tchar *raw;\n };\n \n+struct string_list;\n+\n #define REFSPEC_FETCH 1\n #define REFSPEC_PUSH 0\n \n@@ -80,4 +82,18 @@ int refname_matches_negative_refspec_item(const char *refname, struct refspec *r\n int match_name_with_pattern(const char *key, const char *name,\n \t\t\t\t const char *value, char **result);\n \n+/*\n+ * Queries a refspec for a match and updates the query item.\n+ * Returns 0 on success, -1 if no match is found or negative refspec matches.\n+ */\n+int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n+\n+/*\n+ * Queries a refspec for all matches and appends results to the provided string\n+ * list.\n+ */\n+void refspec_find_all_matches(struct refspec *rs,\n+\t\t\t\t struct refspec_item *query,\n+\t\t\t\t struct string_list *results);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex b510809a56..4c5940482f 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -925,128 +925,6 @@ struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n \treturn ref_map;\n }\n \n-static int refspec_find_negative_match(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i, matched_negative = 0;\n-\tint find_src = !query->src;\n-\tstruct string_list reversed = STRING_LIST_INIT_DUP;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\n-\t/*\n-\t * Check whether the queried ref matches any negative refpsec. If so,\n-\t * then we should ultimately treat this as not matching the query at\n-\t * all.\n-\t *\n-\t * Note that negative refspecs always match the source, but the query\n-\t * item uses the destination. To handle this, we apply pattern\n-\t * refspecs in reverse to figure out if the query source matches any\n-\t * of the negative refspecs.\n-\t *\n-\t * The first loop finds and expands all positive refspecs\n-\t * matched by the queried ref.\n-\t *\n-\t * The second loop checks if any of the results of the first loop\n-\t * match any negative refspec.\n-\t */\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tchar *expn_name;\n-\n-\t\tif (refspec->negative)\n-\t\t\tcontinue;\n-\n-\t\t/* Note the reversal of src and dst */\n-\t\tif (refspec->pattern) {\n-\t\t\tconst char *key = refspec->dst ? refspec->dst : refspec->src;\n-\t\t\tconst char *value = refspec->src;\n-\n-\t\t\tif (match_name_with_pattern(key, needle, value, &expn_name))\n-\t\t\t\tstring_list_append_nodup(&reversed, expn_name);\n-\t\t} else if (refspec->matching) {\n-\t\t\t/* For the special matching refspec, any query should match */\n-\t\t\tstring_list_append(&reversed, needle);\n-\t\t} else if (!refspec->src) {\n-\t\t\tBUG(\"refspec->src should not be null here\");\n-\t\t} else if (!strcmp(needle, refspec->src)) {\n-\t\t\tstring_list_append(&reversed, refspec->src);\n-\t\t}\n-\t}\n-\n-\tfor (i = 0; !matched_negative && i < reversed.nr; i++) {\n-\t\tif (refname_matches_negative_refspec_item(reversed.items[i].string, rs))\n-\t\t\tmatched_negative = 1;\n-\t}\n-\n-\tstring_list_clear(&reversed, 0);\n-\n-\treturn matched_negative;\n-}\n-\n-static void refspec_find_all_matches(struct refspec *rs,\n-\t\t\t\t struct refspec_item *query,\n-\t\t\t\t struct string_list *results)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"refspec_find_all_matches: need either src or dst\");\n-\n-\tif (refspec_find_negative_match(rs, query))\n-\t\treturn;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\t\tconst char *needle = find_src ? query->dst : query->src;\n-\t\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result))\n-\t\t\t\tstring_list_append_nodup(results, *result);\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\tstring_list_append(results, value);\n-\t\t}\n-\t}\n-}\n-\n-int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n-{\n-\tint i;\n-\tint find_src = !query->src;\n-\tconst char *needle = find_src ? query->dst : query->src;\n-\tchar **result = find_src ? &query->src : &query->dst;\n-\n-\tif (find_src && !query->dst)\n-\t\tBUG(\"refspec_find_match: need either src or dst\");\n-\n-\tif (refspec_find_negative_match(rs, query))\n-\t\treturn -1;\n-\n-\tfor (i = 0; i < rs->nr; i++) {\n-\t\tstruct refspec_item *refspec = &rs->items[i];\n-\t\tconst char *key = find_src ? refspec->dst : refspec->src;\n-\t\tconst char *value = find_src ? refspec->src : refspec->dst;\n-\n-\t\tif (!refspec->dst || refspec->negative)\n-\t\t\tcontinue;\n-\t\tif (refspec->pattern) {\n-\t\t\tif (match_name_with_pattern(key, needle, value, result)) {\n-\t\t\t\tquery->force = refspec->force;\n-\t\t\t\treturn 0;\n-\t\t\t}\n-\t\t} else if (!strcmp(needle, key)) {\n-\t\t\t*result = xstrdup(value);\n-\t\t\tquery->force = refspec->force;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn -1;\n-}\n-\n char *apply_refspecs(struct refspec *rs, const char *name)\n {\n \tstruct refspec_item query;\n-- \n2.34.1\n\n"},{"id":"511789","messageId":"20250204040558.34766-6-meetsoni3017@gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"[GSoC][PATCH v4 5/5] refspec: relocate apply_refspecs and related funtions","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T04:05:58Z","receivedAt":"2025-02-04T04:06:28Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"Move the functions `apply_refspecs()` and `apply_negative_refspecs()`\nfrom `remote.c` to `refspec.c`. These functions focus on applying\nrefspecs, so centralizing them in `refspec.c` improves code organization\nby keeping refspec-related logic in one place.\n\nSigned-off-by: Meet Soni \n---\n refspec.c | 32 ++++++++++++++++++++++++++++++++\n refspec.h | 12 ++++++++++++\n remote.c | 31 -------------------------------\n remote.h | 11 -----------\n 4 files changed, 44 insertions(+), 42 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex cab0b0d127..0dbbd1e799 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -9,6 +9,7 @@\n #include \"strvec.h\"\n #include \"refs.h\"\n #include \"refspec.h\"\n+#include \"remote.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -447,3 +448,34 @@ int refspec_find_match(struct refspec *rs, struct refspec_item *query)\n \t}\n \treturn -1;\n }\n+\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n+{\n+\tstruct ref **tail;\n+\n+\tfor (tail = &ref_map; *tail; ) {\n+\t\tstruct ref *ref = *tail;\n+\n+\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n+\t\t\t*tail = ref->next;\n+\t\t\tfree(ref->peer_ref);\n+\t\t\tfree(ref);\n+\t\t} else\n+\t\t\ttail = &ref->next;\n+\t}\n+\n+\treturn ref_map;\n+}\n+\n+char *apply_refspecs(struct refspec *rs, const char *name)\n+{\n+\tstruct refspec_item query;\n+\n+\tmemset(&query, 0, sizeof(struct refspec_item));\n+\tquery.src = (char *)name;\n+\n+\tif (refspec_find_match(rs, &query))\n+\t\treturn NULL;\n+\n+\treturn query.dst;\n+}\ndiff --git a/refspec.h b/refspec.h\nindex be20ba53ab..2a28d043be 100644\n--- a/refspec.h\n+++ b/refspec.h\n@@ -96,4 +96,16 @@ void refspec_find_all_matches(struct refspec *rs,\n \t\t\t\t struct refspec_item *query,\n \t\t\t\t struct string_list *results);\n \n+/*\n+ * Remove all entries in the input list which match any negative refspec in\n+ * the refspec list.\n+ */\n+struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n+\n+/*\n+ * Search for a refspec that matches the given name and return the\n+ * corresponding destination (dst) if a match is found, NULL otherwise.\n+ */\n+char *apply_refspecs(struct refspec *rs, const char *name);\n+\n #endif /* REFSPEC_H */\ndiff --git a/remote.c b/remote.c\nindex 4c5940482f..7f27c59a5b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -907,37 +907,6 @@ void ref_push_report_free(struct ref_push_report *report)\n \t}\n }\n \n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs)\n-{\n-\tstruct ref **tail;\n-\n-\tfor (tail = &ref_map; *tail; ) {\n-\t\tstruct ref *ref = *tail;\n-\n-\t\tif (refname_matches_negative_refspec_item(ref->name, rs)) {\n-\t\t\t*tail = ref->next;\n-\t\t\tfree(ref->peer_ref);\n-\t\t\tfree(ref);\n-\t\t} else\n-\t\t\ttail = &ref->next;\n-\t}\n-\n-\treturn ref_map;\n-}\n-\n-char *apply_refspecs(struct refspec *rs, const char *name)\n-{\n-\tstruct refspec_item query;\n-\n-\tmemset(&query, 0, sizeof(struct refspec_item));\n-\tquery.src = (char *)name;\n-\n-\tif (refspec_find_match(rs, &query))\n-\t\treturn NULL;\n-\n-\treturn query.dst;\n-}\n-\n int remote_find_tracking(struct remote *remote, struct refspec_item *refspec)\n {\n \treturn refspec_find_match(&remote->fetch, refspec);\ndiff --git a/remote.h b/remote.h\nindex 516ba7f398..b4bb16af0e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -261,17 +261,6 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n */\n struct ref *ref_remove_duplicates(struct ref *ref_map);\n \n-int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n-\n-/*\n- * Remove all entries in the input list which match any negative refspec in\n- * the refspec list.\n- */\n-struct ref *apply_negative_refspecs(struct ref *ref_map, struct refspec *rs);\n-\n-int refspec_find_match(struct refspec *rs, struct refspec_item *query);\n-char *apply_refspecs(struct refspec *rs, const char *name);\n-\n int check_push_refs(struct ref *src, struct refspec *rs);\n int match_push_refs(struct ref *src, struct ref **dst,\n \t\t struct refspec *rs, int flags);\n-- \n2.34.1\n\n"},{"id":"511796","messageId":"Z6G-toOJjMmK8iJG@pks.im","threadId":"62861","inReplyTo":"20250204040558.34766-1-meetsoni3017@gmail.com","subject":"Re: [GSoC][PATCH v4 0/5] refspec: centralize refspec-related logic","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-04T07:16:06Z","receivedAt":"2025-02-04T07:16:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 04, 2025 at 09:35:53AM +0530, Meet Soni wrote:\n> Changes since v3:\n> - updated commit message.\n> - renamed functions as per review.\n> - added GSoC mark , since the announcement has been made by google and\n> we've started the discussion regarding the same.\n\nThanks, this version looks good to me!\n\nPatrick\n"},{"id":"511798","messageId":"CAOLa=ZShqCkyabVK2PU-XXpx9QS3_W=9QMH6ioJB=t8Ec2NYqg@mail.gmail.com","threadId":"62861","inReplyTo":"20250204040558.34766-2-meetsoni3017@gmail.com","subject":"Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-02-04T09:00:22Z","receivedAt":"2025-02-04T09:00:25Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Meet Soni writes:\n\n> diff --git a/remote.h b/remote.h\n> index bda10dd5c8..66ee53411d 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n> */\n> struct ref *ref_remove_duplicates(struct ref *ref_map);\n>\n> -/*\n> - * Check whether a name matches any negative refspec in rs. Returns 1 if the\n> - * name matches at least one negative refspec, and 0 otherwise.\n> - */\n> -int omit_name_by_refspec(const char *name, struct refspec *rs);\n> +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n>\n\nNit: The first sentence is now duplicated by the function name as\nmentioned in the commit message. But aren't we loosing information by\nremoving the second sentence?\n\n> /*\n> * Remove all entries in the input list which match any negative refspec in\n> --\n> 2.34.1\n"},{"id":"511807","messageId":"xmqqwme5vmkw.fsf@gitster.g","threadId":"62861","inReplyTo":"CAPhwyn0-Hq5WHWvGzhqwafrJqmDic5+_S7hRxShk53d++hfw8A@mail.gmail.com","subject":"Re: [PATCH v3 3/5] refactor(remote): rename query_refspecs functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-04T13:58:07Z","receivedAt":"2025-02-04T13:58:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Meet Soni writes:\n\n>> So:\n>>\n>> - `query_refspecs()` would be renamed to `refspec_find_match()`.\n>>\n>> - `query_refspecs_multiple()` would be renamed to\n>> `refspec_find_all_matches()`.\n>>\n>> - `find_negative_refspec_match()` would be renamed to\n>> `refspec_find_negative_match()`.\n>>\n>> Patrick\n>\n> Thanks for the review.\n> Meet\n\nYup, using predictable names that follow patterns based on\neasy-to-follow rules is a very useful tool to help developers.\n\nThanks, both.\n"},{"id":"511808","messageId":"CAPhwyn32CmjtKu5ivxS9=AJ-h+5GskDUp=rUGvofv-aWLhH8Ng@mail.gmail.com","threadId":"62861","inReplyTo":"CAOLa=ZShqCkyabVK2PU-XXpx9QS3_W=9QMH6ioJB=t8Ec2NYqg@mail.gmail.com","subject":"Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-02-04T13:58:55Z","receivedAt":"2025-02-04T13:59:08Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Tue, 4 Feb 2025 at 14:30, Karthik Nayak wrote:\n>\n> Meet Soni writes:\n>\n> > diff --git a/remote.h b/remote.h\n> > index bda10dd5c8..66ee53411d 100644\n> > --- a/remote.h\n> > +++ b/remote.h\n> > @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n> > */\n> > struct ref *ref_remove_duplicates(struct ref *ref_map);\n> >\n> > -/*\n> > - * Check whether a name matches any negative refspec in rs. Returns 1 if the\n> > - * name matches at least one negative refspec, and 0 otherwise.\n> > - */\n> > -int omit_name_by_refspec(const char *name, struct refspec *rs);\n> > +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n> >\n>\n> Nit: The first sentence is now duplicated by the function name as\n> mentioned in the commit message. But aren't we loosing information by\n> removing the second sentence?\n>\nCorrect. I considered keeping the second sentence for clarity, but that other\nfunction signatures in the codebase don’t include comments solely describing\nreturn values. To maintain consistency with the existing style, I\nopted to remove\nit. Let me know if you think an alternative approach would be better!\n> > /*\n> > * Remove all entries in the input list which match any negative refspec in\n> > --\n> > 2.34.1\n"},{"id":"511991","messageId":"CAOLa=ZQmALUCY1CiJZG-S3fgRvD_wj8ZwSj5dV-9X=f5NpLVfw@mail.gmail.com","threadId":"62861","inReplyTo":"CAPhwyn32CmjtKu5ivxS9=AJ-h+5GskDUp=rUGvofv-aWLhH8Ng@mail.gmail.com","subject":"Re: [GSoC][PATCH v4 1/5] remote: rename function omit_name_by_refspec","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-02-06T10:13:50Z","receivedAt":"2025-02-06T10:13:53Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Meet Soni writes:\n\n> On Tue, 4 Feb 2025 at 14:30, Karthik Nayak wrote:\n>>\n>> Meet Soni writes:\n>>\n>> > diff --git a/remote.h b/remote.h\n>> > index bda10dd5c8..66ee53411d 100644\n>> > --- a/remote.h\n>> > +++ b/remote.h\n>> > @@ -261,11 +261,7 @@ int resolve_remote_symref(struct ref *ref, struct ref *list);\n>> > */\n>> > struct ref *ref_remove_duplicates(struct ref *ref_map);\n>> >\n>> > -/*\n>> > - * Check whether a name matches any negative refspec in rs. Returns 1 if the\n>> > - * name matches at least one negative refspec, and 0 otherwise.\n>> > - */\n>> > -int omit_name_by_refspec(const char *name, struct refspec *rs);\n>> > +int refname_matches_negative_refspec_item(const char *refname, struct refspec *rs);\n>> >\n>>\n>> Nit: The first sentence is now duplicated by the function name as\n>> mentioned in the commit message. But aren't we loosing information by\n>> removing the second sentence?\n>>\n> Correct. I considered keeping the second sentence for clarity, but that other\n> function signatures in the codebase don’t include comments solely describing\n> return values. To maintain consistency with the existing style, I\n> opted to remove\n> it. Let me know if you think an alternative approach would be better!\n\nI think its okay as-is for now :)\n\n>> > /*\n>> > * Remove all entries in the input list which match any negative refspec in\n>> > --\n>> > 2.34.1\n"}]}