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

[Junio C Hamano <gitster@pobox.com>]

Meet Soni <meetsoni3017@gmail.com> writes:

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

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

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

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

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

This one now has an excellent comment.  Great job.

Thanks.