Karthik Nayak <karthik....@gmail.com> writes:
> From: Karthik Nayak <karthik....@gmail.com>
>
> Add an option in 'filter_refs()' to use 'for_each_branch_ref()'
> and filter refs. This type checking is done by adding a
> 'FILTER_REFS_BRANCHES' in 'ref-filter.h'.
>
> Add an option in 'ref_filter_handler()' to filter different
> types of branches by calling 'filter_branch_kind()' which
> checks for the type of branch needed.
>
> Mentored-by: Christian Couder <christian.cou...@gmail.com>
> Mentored-by: Matthieu Moy <matthieu....@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik....@gmail.com>
> ---
> ref-filter.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> ref-filter.h | 10 ++++++++--
> 2 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index de84dd4..c573109 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1044,6 +1044,46 @@ static const unsigned char *match_points_at(struct
> sha1_array *points_at,
> return NULL;
> }
>
> +/*
> + * Checks if a given refname is a branch and returns the kind of
> + * branch it is. If not a branch, 0 is returned.
> + */
> +static int filter_branch_kind(struct ref_filter *filter, const char *refname)
> +{
> + int kind, i;
> +
> + static struct {
> + const char *prefix;
> + int kind;
Make a mental note that this is signed int.
> + } ref_kind[] = {
> + { "refs/heads/" , REF_LOCAL_BRANCH },
> + { "refs/remotes/" , REF_REMOTE_BRANCH },
> + };
> +
> + /* If no kind is specified, no need to filter */
> + if (!filter->branch_kind)
> + return REF_NO_BRANCH_FILTERING;
> +
> + for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
> + if (starts_with(refname, ref_kind[i].prefix)) {
> + kind = ref_kind[i].kind;
> + break;
> + }
> + }
> +
> + if (ARRAY_SIZE(ref_kind) <= i) {
> + if (!strcmp(refname, "HEAD"))
> + kind = REF_DETACHED_HEAD;
> + else
> + return 0;
> + }
> +
> + if ((filter->branch_kind & kind) == 0)
> + return 0;
> +
> + return kind;
> +}
While this looks fine, I am not sure if this is a good interface for
filtering, though.
If you start from already having everything and want to limit things
down to "refs/heads/", this might make sense but it would be far
more efficient, if you know that you want to limit to "refs/heads/"
upfront, not to collect everything but just limit the collection to
those under "refs/heads/" without wasting cycles in the first place.
> diff --git a/ref-filter.h b/ref-filter.h
> index 5be3e35..b5a13e8 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -16,6 +16,12 @@
> #define FILTER_REFS_INCLUDE_BROKEN 0x1
> #define FILTER_REFS_ALL 0x2
> #define FILTER_REFS_TAGS 0x4
> +#define FILTER_REFS_BRANCHES 0x8
Is this a sensible set of bits? Does it make sense to have ALL and
TAGS at the same time (and what does that mean?)?
> +#define REF_DETACHED_HEAD 0x01
> +#define REF_LOCAL_BRANCH 0x02
> +#define REF_REMOTE_BRANCH 0x04
> +#define REF_NO_BRANCH_FILTERING 0x08
Where do these values go? It is a returned by filter-branch-kind
for each ref, i.e. given "refs/heads/bar", it answers "Yeah, that is
a local branch". Why are these values pretending to be a set of
bits that can be combined together, as if a branch can be both LOCAL
and REMOTE? This does not make _any_ sense.
> #define ALIGN_LEFT 0x01
> #define ALIGN_RIGHT 0x02
> @@ -50,7 +56,7 @@ struct ref_sorting {
>
> struct ref_array_item {
> unsigned char objectname[20];
> - int flag;
> + int flag, kind;
For readability, do not define multiple structure fields on a single
line.
If you are storing a set of bits in an integer, use unsigned. If it
is an enumeration, int is fine.
> const char *symref;
> struct commit *commit;
> struct atom_value *value;
> @@ -76,7 +82,7 @@ struct ref_filter {
>
> unsigned int with_commit_tag_algo : 1,
> match_as_path : 1;
> - unsigned int lines;
> + unsigned int lines, branch_kind;
For readability, do not define multiple structure fields on a single
line.
> };
>
> struct ref_filter_cbdata {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
