From: Junio C Hamano <gitster@pobox.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org, christian.couder@gmail.com,
Matthieu.Moy@grenoble-inp.fr
Subject: Re: [PATCH 01/10] ref-filter: add option to filter only branches
Date: Tue, 11 Aug 2015 10:33:17 -0700 [thread overview]
Message-ID: <xmqqpp2tspb6.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1438693282-15516-1-git-send-email-Karthik.188@gmail.com> (Karthik Nayak's message of "Tue, 4 Aug 2015 18:31:14 +0530")
Karthik Nayak <karthik.188@gmail.com> writes:
> From: Karthik Nayak <karthik.188@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.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@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 {
next prev parent reply other threads:[~2015-08-11 17:33 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-04 12:59 [PATCH 0/10] Port branch.c to ref-filter Karthik Nayak
2015-08-04 13:01 ` [PATCH 01/10] ref-filter: add option to filter only branches Karthik Nayak
2015-08-11 17:33 ` Junio C Hamano [this message]
2015-08-13 10:51 ` Karthik Nayak
2015-08-13 11:35 ` Karthik Nayak
2015-08-13 15:13 ` Karthik Nayak
2015-08-14 15:56 ` Junio C Hamano
2015-08-14 18:45 ` Karthik Nayak
2015-08-13 16:52 ` Matthieu Moy
2015-08-13 20:13 ` Karthik Nayak
2015-08-04 13:01 ` [PATCH 02/10] branch: refactor width computation Karthik Nayak
2015-08-11 1:58 ` Eric Sunshine
2015-08-11 13:10 ` Karthik Nayak
2015-08-04 13:01 ` [PATCH 03/10] branch: bump get_head_description() to the top Karthik Nayak
2015-08-11 1:59 ` Eric Sunshine
2015-08-11 13:12 ` Karthik Nayak
2015-08-04 13:01 ` [PATCH 04/10] branch: roll show_detached HEAD into regular ref_list Karthik Nayak
2015-08-11 2:41 ` Eric Sunshine
2015-08-11 13:17 ` Karthik Nayak
2015-08-04 13:01 ` [PATCH 05/10] branch: move 'current' check down to the presentation layer Karthik Nayak
2015-08-04 13:01 ` [PATCH 06/10] branch: drop non-commit error reporting Karthik Nayak
2015-08-04 13:01 ` [PATCH 07/10] branch.c: use 'ref-filter' data structures Karthik Nayak
2015-08-04 13:01 ` [PATCH 08/10] branch.c: use 'ref-filter' APIs Karthik Nayak
2015-08-04 13:01 ` [PATCH 09/10] branch: add '--points-at' option Karthik Nayak
2015-08-04 13:01 ` [PATCH 0/10] Port branch.c to ref-filter Karthik Nayak
2015-08-05 21:35 ` Junio C Hamano
2015-08-07 15:22 ` Karthik Nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpp2tspb6.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=karthik.188@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).