mbox series

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

Message ID 20250127103644.36627-1-meetsoni3017@gmail.com (mailing list archive)
Headers show
Series refspec: centralize refspec-related logic | expand

Message

Meet Soni Jan. 27, 2025, 10:36 a.m. UTC
Thank you for reviewing :)

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

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

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

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

Comments

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

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

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

Best regards,
Meet
Junio C Hamano Jan. 27, 2025, 6:10 p.m. UTC | #2
Meet Soni <meetsoni3017@gmail.com> writes:

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

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

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

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

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

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