threads / rfc / 39547

RFC patch, 9 partsadd options to ref-filter

Subject: [RFC/PATCH 0/9] add options to ref-filter

## tl;dr

37 messages between Jun 6, 2015 and Jun 10, 2015. Diffs are folded; open one to read it.

replies: 36people: 4as markdown or json

Karthik Nayak· Jun 6, 2015, 20:03 UTC · lore

This is a follow up series to the one posted here http://thread.gmane.org/gmane.comp.version-control.git/270922

This patch series adds '--ponints-at', '--merged', '--no-merged' and '--contain' options to 'ref-filter' and uses these options in 'for-each-ref'.

-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 1/9] tag: libify parse_opt_points_at()

Moving 'parse_opt_points_at()' from 'tag.c' to a common location, in preparation for using it in 'ref-filter'.

Based-on-patch-by: Jeff King <peff@peff.net>
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>
---
 builtin/tag.c      | 19 +------------------
 parse-options-cb.c | 17 +++++++++++++++++
 parse-options.h    |  1 +
 3 files changed, 19 insertions(+), 18 deletions(-)
Show changes to 3 files +19 −18

builtin/tag.c, parse-options-cb.c, parse-options.h

diff --git a/builtin/tag.c b/builtin/tag.c
index 5f6cdc5..eda76ba 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -546,23 +546,6 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
 	return check_refname_format(sb->buf, 0);
 }
 
-static int parse_opt_points_at(const struct option *opt __attribute__((unused)),
-			const char *arg, int unset)
-{
-	unsigned char sha1[20];
-
-	if (unset) {
-		sha1_array_clear(&points_at);
-		return 0;
-	}
-	if (!arg)
-		return error(_("switch 'points-at' requires an object"));
-	if (get_sha1(arg, sha1))
-		return error(_("malformed object name '%s'"), arg);
-	sha1_array_append(&points_at, sha1);
-	return 0;
-}
-
 static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
 {
 	int *sort = opt->value;
@@ -625,7 +608,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			parse_opt_with_commit, (intptr_t)"HEAD",
 		},
 		{
-			OPTION_CALLBACK, 0, "points-at", NULL, N_("object"),
+			OPTION_CALLBACK, 0, "points-at", &points_at, N_("object"),
 			N_("print only tags of the object"), 0, parse_opt_points_at
 		},
 		OPT_END()
diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..a4d294d 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
 #include "commit.h"
 #include "color.h"
 #include "string-list.h"
+#include "sha1-array.h"
 
 /*----- some often used options -----*/
 
@@ -92,6 +93,22 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
+{
+	unsigned char sha1[20];
+
+	if (unset) {
+		sha1_array_clear(opt->value);
+		return 0;
+	}
+	if (!arg)
+		return -1;
+	if (get_sha1(arg, sha1))
+		return error(_("malformed object name '%s'"), arg);
+	sha1_array_append(opt->value, sha1);
+	return 0;
+}
+
 int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 {
 	int *target = opt->value;
diff --git a/parse-options.h b/parse-options.h
index c71e9da..3ae16a1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -220,6 +220,7 @@ extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
 extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
 extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
 extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
+extern int parse_opt_points_at(const struct option *, const char *, int);
 extern int parse_opt_with_commit(const struct option *, const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
-- 
2.4.2
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 2/9] ref-filter: implement '--points-at' option

In 'tag -l' we have '--points-at' option which lets users list only tags which point to a particular commit, Implement this option in 'ref-filter.{c,h}' so that the other commands can benefit from this.

Based-on-patch-by: Jeff King <peff@peff.net>
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 | 26 ++++++++++++++++++++++++++
 ref-filter.h |  1 +
 2 files changed, 27 insertions(+)
Show changes to 2 files +27 −0

ref-filter.c, ref-filter.h

diff --git a/ref-filter.c b/ref-filter.c
index 745d3b3..456b0fa 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -841,6 +841,29 @@ static int match_name_as_path(const char **pattern, const char *refname)
 	return 0;
 }
 
+/*
+ * Given a ref (sha1, refname) see if it points to a one of the sha1s
+ * in a sha1_array.
+ */
+static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1,
+			   const char *refname)
+{
+	struct object *obj;
+
+	if (!points_at || !points_at->nr)
+		return 1;
+
+	if (sha1_array_lookup(points_at, sha1) >= 0)
+		return 1;
+
+	obj = parse_object_or_die(sha1, refname);
+	if (obj->type == OBJ_TAG &&
+	    sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= 0)
+		return 1;
+
+	return 0;
+}
+
 /* Allocate space for a new ref_array_item and copy the objectname and flag to it */
 static struct ref_array_item *new_ref_array_item(const char *refname,
 						 const unsigned char *objectname,
@@ -874,6 +897,9 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
 		return 0;
 
+	if (!match_points_at(&filter->points_at, oid->hash, refname))
+		return 0;
+
 	/*
 	 * We do not open the object yet; sort may only need refname
 	 * to do its job and the resulting list may yet to be pruned
diff --git a/ref-filter.h b/ref-filter.h
index 041a39a..a8980e7 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -39,6 +39,7 @@ struct ref_array {
 
 struct ref_filter {
 	const char **name_patterns;
+	struct sha1_array points_at;
 };
 
 struct ref_filter_cbdata {
-- 
2.4.2
Matthieu Moy· Jun 8, 2015, 17:31 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option

Karthik Nayak <karthik.188@gmail.com> writes:
Show 22 quoted lines
> +/*
> + * Given a ref (sha1, refname) see if it points to a one of the sha1s
> + * in a sha1_array.
> + */
> +static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1,
> +			   const char *refname)
> +{
> +	struct object *obj;
> +
> +	if (!points_at || !points_at->nr)
> +		return 1;
> +
> +	if (sha1_array_lookup(points_at, sha1) >= 0)
> +		return 1;
> +
> +	obj = parse_object_or_die(sha1, refname);
> +	if (obj->type == OBJ_TAG &&
> +	    sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= 0)
> +		return 1;
> +
> +	return 0;
> +}

There's a similar function in builtin/tag.c that you are not removing. You should justify why you are doing this code duplication in the commit message (or not do code duplication). It might make sense to add a comment next to match_points_at in tag.c saying stg like "this is duplicated from ..., will be removed later".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:50 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option

On 06/08/2015 11:01 PM, Matthieu Moy wrote:
Show 31 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +/*
>> + * Given a ref (sha1, refname) see if it points to a one of the sha1s
>> + * in a sha1_array.
>> + */
>> +static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1,
>> +			   const char *refname)
>> +{
>> +	struct object *obj;
>> +
>> +	if (!points_at || !points_at->nr)
>> +		return 1;
>> +
>> +	if (sha1_array_lookup(points_at, sha1) >= 0)
>> +		return 1;
>> +
>> +	obj = parse_object_or_die(sha1, refname);
>> +	if (obj->type == OBJ_TAG &&
>> +	    sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= 0)
>> +		return 1;
>> +
>> +	return 0;
>> +}
>
> There's a similar function in builtin/tag.c that you are not removing.
> You should justify why you are doing this code duplication in the commit
> message (or not do code duplication). It might make sense to add a
> comment next to match_points_at in tag.c saying stg like "this is
> duplicated from ..., will be removed later".
>
Ok will add this :)
-- 
Regards,
Karthik
Matthieu Moy· Jun 8, 2015, 18:00 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option

Karthik Nayak <karthik.188@gmail.com> writes:
> In 'tag -l' we have '--points-at' option which lets users
> list only tags which point to a particular commit, Implement
s/,/./ ?
> this option in 'ref-filter.{c,h}' so that the other commands
s/the//
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:54 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option

On 06/08/2015 11:30 PM, Matthieu Moy wrote:
Show 11 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> In 'tag -l' we have '--points-at' option which lets users
>> list only tags which point to a particular commit, Implement
>
> s/,/./ ?
>
>> this option in 'ref-filter.{c,h}' so that the other commands
>
> s/the//
>
Noted! thanks
-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 3/9] for-each-ref: add '--points-at' option

Add the '--points-at' option provided by 'ref-filter'. The option lets the user to pick only refs which point to a particular commit.

Add Documentation for the same.
Based-on-patch-by: Jeff King <peff@peff.net>
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>
---
 Documentation/git-for-each-ref.txt | 3 +++
 builtin/for-each-ref.c             | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)
Show changes to 2 files +8 −1

Documentation/git-for-each-ref.txt, builtin/for-each-ref.c

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 7f8d9a5..e9f6a8a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
+		   [--points-at <object>]
 
 DESCRIPTION
 -----------
@@ -62,6 +63,8 @@ OPTIONS
 	the specified host language.  This is meant to produce
 	a scriptlet that can directly be `eval`ed.
 
+--points-at <object>::
+	Only list tags of the given object.
 
 FIELD NAMES
 -----------
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4d2d024..b9d180a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -7,6 +7,7 @@
 
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
+	N_("git for-each-ref [--points-at <object>]"),
 	NULL
 };
 
@@ -17,6 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
 	struct ref_filter_cbdata ref_cbdata;
+	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -33,6 +35,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
 		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
 			    N_("field name to sort on"), &parse_opt_ref_sorting),
+		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
+			     N_("object"), N_("print only tags of the object"),
+			     parse_opt_points_at),
 		OPT_END(),
 	};
 
@@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
 	ref_cbdata.filter.name_patterns = argv;
 	filter_refs(for_each_rawref, &ref_cbdata);
 
-- 
2.4.2
Matthieu Moy· Jun 8, 2015, 17:35 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

Karthik Nayak <karthik.188@gmail.com> writes:
Show 5 quoted lines
> Add the '--points-at' option provided by 'ref-filter'. The
> option lets the user to pick only refs which point to a particular
> commit.
>
> Add Documentation for the same.
... but no test?
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:51 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

On 06/08/2015 11:05 PM, Matthieu Moy wrote:
Show 10 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Add the '--points-at' option provided by 'ref-filter'. The
>> option lets the user to pick only refs which point to a particular
>> commit.
>>
>> Add Documentation for the same.
>
> ... but no test?
>

No haven't written tests, this was just to ensure if the way this is moving forth is correct.

-- 
Regards,
Karthik
Junio C Hamano· Jun 8, 2015, 19:12 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

Karthik Nayak <karthik.188@gmail.com> writes:
Show 33 quoted lines
> Add the '--points-at' option provided by 'ref-filter'. The
> option lets the user to pick only refs which point to a particular
> commit.
>
> Add Documentation for the same.
>
> Based-on-patch-by: Jeff King <peff@peff.net>
> 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>
> ---
>  Documentation/git-for-each-ref.txt | 3 +++
>  builtin/for-each-ref.c             | 6 +++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index 7f8d9a5..e9f6a8a 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -10,6 +10,7 @@ SYNOPSIS
>  [verse]
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
> +		   [--points-at <object>]
>  
>  DESCRIPTION
>  -----------
> @@ -62,6 +63,8 @@ OPTIONS
>  	the specified host language.  This is meant to produce
>  	a scriptlet that can directly be `eval`ed.
>  
> +--points-at <object>::
> +	Only list tags of the given object.
Is this intended?  I would have expected if I did
	git for-each-ref --points-at master

I would get refs/heads/master and any other refs that exactly points at that commit.

Show 38 quoted lines
>  
>  FIELD NAMES
>  -----------
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 4d2d024..b9d180a 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -7,6 +7,7 @@
>  
>  static char const * const for_each_ref_usage[] = {
>  	N_("git for-each-ref [<options>] [<pattern>]"),
> +	N_("git for-each-ref [--points-at <object>]"),
>  	NULL
>  };
>  
> @@ -17,6 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
>  	int maxcount = 0, quote_style = 0;
>  	struct ref_filter_cbdata ref_cbdata;
> +	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>  
>  	struct option opts[] = {
>  		OPT_BIT('s', "shell", &quote_style,
> @@ -33,6 +35,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
>  		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
>  			    N_("field name to sort on"), &parse_opt_ref_sorting),
> +		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
> +			     N_("object"), N_("print only tags of the object"),
> +			     parse_opt_points_at),
>  		OPT_END(),
>  	};
>  
> @@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  	/* for warn_ambiguous_refs */
>  	git_config(git_default_config, NULL);
>  
> -	memset(&ref_cbdata, 0, sizeof(ref_cbdata));

I cannot quite see how this change relates to the addition of the new option.

>  	ref_cbdata.filter.name_patterns = argv;
>  	filter_refs(for_each_rawref, &ref_cbdata);
Karthik Nayak· Jun 9, 2015, 12:01 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

On 06/09/2015 12:42 AM, Junio C Hamano wrote:
Show 8 quoted lines
>
> Is this intended?  I would have expected if I did
>
> 	git for-each-ref --points-at master
>
> I would get refs/heads/master and any other refs that exactly points
> at that commit.
>
Thats to be changed, thanks!
Show 42 quoted lines
>>
>>   FIELD NAMES
>>   -----------
>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index 4d2d024..b9d180a 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -7,6 +7,7 @@
>>
>>   static char const * const for_each_ref_usage[] = {
>>   	N_("git for-each-ref [<options>] [<pattern>]"),
>> +	N_("git for-each-ref [--points-at <object>]"),
>>   	NULL
>>   };
>>
>> @@ -17,6 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>   	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
>>   	int maxcount = 0, quote_style = 0;
>>   	struct ref_filter_cbdata ref_cbdata;
>> +	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>>
>>   	struct option opts[] = {
>>   		OPT_BIT('s', "shell", &quote_style,
>> @@ -33,6 +35,9 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>   		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
>>   		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
>>   			    N_("field name to sort on"), &parse_opt_ref_sorting),
>> +		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
>> +			     N_("object"), N_("print only tags of the object"),
>> +			     parse_opt_points_at),
>>   		OPT_END(),
>>   	};
>>
>> @@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>   	/* for warn_ambiguous_refs */
>>   	git_config(git_default_config, NULL);
>>
>> -	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>
> I cannot quite see how this change relates to the addition of the
> new option.
>

Well if we memset() after calling parse_opt_points_at(), we loose all the information we would have obtained. So the memset() is moved to an earlier location.

-- 
Regards,
Karthik
Junio C Hamano· Jun 9, 2015, 19:07 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

Karthik Nayak <karthik.188@gmail.com> writes:
Show 13 quoted lines
>>> @@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>>   	/* for warn_ambiguous_refs */
>>>   	git_config(git_default_config, NULL);
>>>
>>> -	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>>
>> I cannot quite see how this change relates to the addition of the
>> new option.
>>
>
> Well if we memset() after calling parse_opt_points_at(), we loose all
> the information we would have obtained.
> So the memset() is moved to an earlier location.

which I did not see, because I expected the code to follow the usual "no decl-after-statement" pattern. IOW

Show 6 quoted lines
>>   	int maxcount = 0, quote_style = 0;
>>   	struct ref_filter_cbdata ref_cbdata;
>> +	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
>>
>>   	struct option opts[] = {
>>   		OPT_BIT('s', "shell", &quote_style,
Don't do that.  Always start your function like so:
	type funcname(args)
        {
        	declarations;
                first statement;
		...

with no blank line within declarations block and a blank line after the declarations block.

Karthik Nayak· Jun 10, 2015, 06:55 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

On 06/10/2015 12:37 AM, Junio C Hamano wrote:
Show 38 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> >>> @@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
> >>>        /* for warn_ambiguous_refs */
> >>>        git_config(git_default_config, NULL);
> >>>
> >>> -    memset(&ref_cbdata, 0, sizeof(ref_cbdata));
> >>
> >> I cannot quite see how this change relates to the addition of the
> >> new option.
> >>
> >
> > Well if we memset() after calling parse_opt_points_at(), we loose all
> > the information we would have obtained.
> > So the memset() is moved to an earlier location.
>
> which I did not see, because I expected the code to follow the usual
> "no decl-after-statement" pattern.  IOW
>
> >>        int maxcount = 0, quote_style = 0;
> >>        struct ref_filter_cbdata ref_cbdata;
> >> +    memset(&ref_cbdata, 0, sizeof(ref_cbdata));
> >>
> >>        struct option opts[] = {
> >>            OPT_BIT('s', "shell", &quote_style,
>
> Don't do that.  Always start your function like so:
>
>     type funcname(args)
>          {
>              declarations;
>
>                  first statement;
>         ...
>
> with no blank line within declarations block and a blank line after
> the declarations block.
>
Will do, thanks!
-- 
Regards,
Karthik
Matthieu Moy· Jun 10, 2015, 07:39 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Don't do that.  Always start your function like so:
>
> 	type funcname(args)
>         {
>         	declarations;
>
>                 first statement;
> 		...
Hint: create a file config.mak with this content:

$ cat config.mak CFLAGS += -Wdeclaration-after-statement -Wall -Werror

and gcc will prevent you from doing this mistake again.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 10, 2015, 11:31 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option

On 06/10/2015 01:09 PM, Matthieu Moy wrote:
Show 18 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Don't do that.  Always start your function like so:
> >
> >     type funcname(args)
> >          {
> >              declarations;
> >
> >                  first statement;
> >         ...
>
> Hint: create a file config.mak with this content:
>
> $ cat config.mak
> CFLAGS += -Wdeclaration-after-statement -Wall -Werror
>
> and gcc will prevent you from doing this mistake again.
>

Thanks a lot! Your tips are brilliant.

-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()

Add 'parse_opt_merge_filter()' to parse '--merged' and '--no-merged' options and write MACROS for the same.

Based-on-patch-by: Jeff King <peff@peff.net>
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>
---
 parse-options-cb.c | 22 ++++++++++++++++++++++
 parse-options.h    | 11 +++++++++++
 2 files changed, 33 insertions(+)
Show changes to 2 files +33 −0

parse-options-cb.c, parse-options.h

diff --git a/parse-options-cb.c b/parse-options-cb.c
index a4d294d..1969803 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
 #include "commit.h"
 #include "color.h"
 #include "string-list.h"
+#include "ref-filter.h"
 #include "sha1-array.h"
 
 /*----- some often used options -----*/
@@ -109,6 +110,27 @@ int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
+{
+	struct ref_filter *rf = opt->value;
+	unsigned char sha1[20];
+
+	rf->merge = opt->long_name[0] == 'n'
+		? REF_FILTER_MERGED_OMIT
+		: REF_FILTER_MERGED_INCLUDE;
+
+	if (!arg)
+		arg = "HEAD";
+	if (get_sha1(arg, sha1))
+		die(_("malformed object name %s"), arg);
+
+	rf->merge_commit = lookup_commit_reference_gently(sha1, 0);
+	if (!rf->merge_commit)
+		return opterror(opt, "must point to a commit", 0);
+
+	return 0;
+}
+
 int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 {
 	int *target = opt->value;
diff --git a/parse-options.h b/parse-options.h
index 3ae16a1..7bcf0f3 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -221,6 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
 extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
 extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 extern int parse_opt_points_at(const struct option *, const char *, int);
+extern int parse_opt_merge_filter(const struct option *, const char *, int);
 extern int parse_opt_with_commit(const struct option *, const char *, int);
 extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
@@ -243,5 +244,15 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
 	OPT_COLOR_FLAG(0, "color", (var), (h))
 #define OPT_COLUMN(s, l, v, h) \
 	{ OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, parseopt_column_callback }
+#define OPT_NO_MERGED(filter, h) \
+	{ OPTION_CALLBACK, 0, "no-merged", (filter), N_("commit"), (h), \
+	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+	  parse_opt_merge_filter, (intptr_t) "HEAD" \
+	}
+#define OPT_MERGED(filter, h) \
+	{ OPTION_CALLBACK, 0, "merged", (filter), N_("commit"), (h), \
+	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+	  parse_opt_merge_filter, (intptr_t) "HEAD" \
+	}
 
 #endif
-- 
2.4.2
Matthieu Moy· Jun 8, 2015, 17:58 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()

Karthik Nayak <karthik.188@gmail.com> writes:
Show 8 quoted lines
> +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
> +{
> +	struct ref_filter *rf = opt->value;
> +	unsigned char sha1[20];
> +
> +	rf->merge = opt->long_name[0] == 'n'
> +		? REF_FILTER_MERGED_OMIT
> +		: REF_FILTER_MERGED_INCLUDE;

I would use starts_with("no-", opt->long_name) instead. I had a hard time understanding why the letter 'n' was special while the starts_with() version is self-explanatory.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:54 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()

On 06/08/2015 11:28 PM, Matthieu Moy wrote:
Show 15 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
>> +{
>> +	struct ref_filter *rf = opt->value;
>> +	unsigned char sha1[20];
>> +
>> +	rf->merge = opt->long_name[0] == 'n'
>> +		? REF_FILTER_MERGED_OMIT
>> +		: REF_FILTER_MERGED_INCLUDE;
>
> I would use starts_with("no-", opt->long_name) instead. I had a hard
> time understanding why the letter 'n' was special while the
> starts_with() version is self-explanatory.
>
Can do that also :)
-- 
Regards,
Karthik
Junio C Hamano· Jun 8, 2015, 19:20 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()

Karthik Nayak <karthik.188@gmail.com> writes:
Show 20 quoted lines
> +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
> +{
> +	struct ref_filter *rf = opt->value;
> +	unsigned char sha1[20];
> +
> +	rf->merge = opt->long_name[0] == 'n'
> +		? REF_FILTER_MERGED_OMIT
> +		: REF_FILTER_MERGED_INCLUDE;
> +
> +	if (!arg)
> +		arg = "HEAD";
> +	if (get_sha1(arg, sha1))
> +		die(_("malformed object name %s"), arg);
> +
> +	rf->merge_commit = lookup_commit_reference_gently(sha1, 0);
> +	if (!rf->merge_commit)
> +		return opterror(opt, "must point to a commit", 0);
> +
> +	return 0;
> +}

Again, this smells too specific to live as a part of parse-options infrastructure. If we want to have two helper callbacks, one that gives the results in an sha1-array (because there is no guarantee that you want only commits) and in a commit-list, I am fine with having parse_opt_object_name() and parse_opt_with_commit(). Perhaps rename the latter which was named too specifically to something more sensible (e.g. parse_opt_commit_object_name()) and use it from the caller you wanted to use parse_opt_merge_filter()? The caller, if it is not prepared to see more than one commits specified, may have to check if (!list || !list->next) { die("I want one and only one") } or something, though.

Having it in ref-filter.h as parse_opt_merge_filter() is fine, though. After all, you would be sharing it with for-each-ref, branch and tag and nobody else anyway.

Show 26 quoted lines
> diff --git a/parse-options.h b/parse-options.h
> index 3ae16a1..7bcf0f3 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -221,6 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
>  extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
>  extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
>  extern int parse_opt_points_at(const struct option *, const char *, int);
> +extern int parse_opt_merge_filter(const struct option *, const char *, int);
>  extern int parse_opt_with_commit(const struct option *, const char *, int);
>  extern int parse_opt_tertiary(const struct option *, const char *, int);
>  extern int parse_opt_string_list(const struct option *, const char *, int);
> @@ -243,5 +244,15 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
>  	OPT_COLOR_FLAG(0, "color", (var), (h))
>  #define OPT_COLUMN(s, l, v, h) \
>  	{ OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, parseopt_column_callback }
> +#define OPT_NO_MERGED(filter, h) \
> +	{ OPTION_CALLBACK, 0, "no-merged", (filter), N_("commit"), (h), \
> +	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
> +	  parse_opt_merge_filter, (intptr_t) "HEAD" \
> +	}
> +#define OPT_MERGED(filter, h) \
> +	{ OPTION_CALLBACK, 0, "merged", (filter), N_("commit"), (h), \
> +	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
> +	  parse_opt_merge_filter, (intptr_t) "HEAD" \
> +	}
Likewise.
Karthik Nayak· Jun 9, 2015, 12:36 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()

On 06/09/2015 12:50 AM, Junio C Hamano wrote:
Show 38 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset)
>> +{
>> +	struct ref_filter *rf = opt->value;
>> +	unsigned char sha1[20];
>> +
>> +	rf->merge = opt->long_name[0] == 'n'
>> +		? REF_FILTER_MERGED_OMIT
>> +		: REF_FILTER_MERGED_INCLUDE;
>> +
>> +	if (!arg)
>> +		arg = "HEAD";
>> +	if (get_sha1(arg, sha1))
>> +		die(_("malformed object name %s"), arg);
>> +
>> +	rf->merge_commit = lookup_commit_reference_gently(sha1, 0);
>> +	if (!rf->merge_commit)
>> +		return opterror(opt, "must point to a commit", 0);
>> +
>> +	return 0;
>> +}
>
> Again, this smells too specific to live as a part of parse-options
> infrastructure.  If we want to have two helper callbacks, one that
> gives the results in an sha1-array (because there is no guarantee
> that you want only commits) and in a commit-list, I am fine with
> having parse_opt_object_name() and parse_opt_with_commit().  Perhaps
> rename the latter which was named too specifically to something more
> sensible (e.g. parse_opt_commit_object_name()) and use it from the
> caller you wanted to use parse_opt_merge_filter()?  The caller, if
> it is not prepared to see more than one commits specified, may have
> to check if (!list || !list->next) { die("I want one and only one") }
> or something, though.
>
> Having it in ref-filter.h as parse_opt_merge_filter() is fine,
> though.  After all, you would be sharing it with for-each-ref,
> branch and tag and nobody else anyway.
 >

I guess it's better left in ref-filter.h. We could like you said make it depend on parse_opt_with_commit() but that again means we need to check if more than one commits are specified. So I think it would be better to have it in ref-filter.h

Show 30 quoted lines
>
>> diff --git a/parse-options.h b/parse-options.h
>> index 3ae16a1..7bcf0f3 100644
>> --- a/parse-options.h
>> +++ b/parse-options.h
>> @@ -221,6 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
>>   extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
>>   extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
>>   extern int parse_opt_points_at(const struct option *, const char *, int);
>> +extern int parse_opt_merge_filter(const struct option *, const char *, int);
>>   extern int parse_opt_with_commit(const struct option *, const char *, int);
>>   extern int parse_opt_tertiary(const struct option *, const char *, int);
>>   extern int parse_opt_string_list(const struct option *, const char *, int);
>> @@ -243,5 +244,15 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
>>   	OPT_COLOR_FLAG(0, "color", (var), (h))
>>   #define OPT_COLUMN(s, l, v, h) \
>>   	{ OPTION_CALLBACK, (s), (l), (v), N_("style"), (h), PARSE_OPT_OPTARG, parseopt_column_callback }
>> +#define OPT_NO_MERGED(filter, h) \
>> +	{ OPTION_CALLBACK, 0, "no-merged", (filter), N_("commit"), (h), \
>> +	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
>> +	  parse_opt_merge_filter, (intptr_t) "HEAD" \
>> +	}
>> +#define OPT_MERGED(filter, h) \
>> +	{ OPTION_CALLBACK, 0, "merged", (filter), N_("commit"), (h), \
>> +	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
>> +	  parse_opt_merge_filter, (intptr_t) "HEAD" \
>> +	}
>
> Likewise.
>
This too would be better off in ref-filter.h
-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 5/9] ref-filter: implement '--merged' and '--no-merged' options

In 'branch -l' we have '--merged' option which only lists refs (branches) merged into the named commit and '--no-merged' option which only lists refs (branches) not merged into the named commit will be listed. Implement these two options in ref-filter.{c,h} so that other commands can benefit from this.

Based-on-patch-by: Jeff King <peff@peff.net>
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 | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 ref-filter.h |  8 ++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
Show changes to 2 files +68 −1

ref-filter.c, ref-filter.h

diff --git a/ref-filter.c b/ref-filter.c
index 456b0fa..2be9df2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -9,6 +9,7 @@
 #include "tag.h"
 #include "quote.h"
 #include "ref-filter.h"
+#include "revision.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -888,6 +889,7 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
 	struct ref_filter *filter = &ref_cbdata->filter;
 	struct ref_array_item *ref;
+	struct commit *commit = NULL;
 
 	if (flag & REF_BAD_NAME) {
 		  warning("ignoring ref with broken name %s", refname);
@@ -900,12 +902,19 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	if (!match_points_at(&filter->points_at, oid->hash, refname))
 		return 0;
 
+	if(filter->merge_commit) {
+		commit = lookup_commit_reference_gently(sha1, 1);
+		if (!commit)
+			return 0;
+	}
+
 	/*
 	 * We do not open the object yet; sort may only need refname
 	 * to do its job and the resulting list may yet to be pruned
 	 * by maxcount logic.
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
+	ref->commit = commit;
 
 	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
 	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
@@ -931,6 +940,50 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
+{
+	struct rev_info revs;
+	int i, old_nr;
+	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_array *array = &ref_cbdata->array;
+	struct commit_list *p, *to_clear = NULL;
+
+	init_revisions(&revs, NULL);
+
+	for (i = 0; i < array->nr; i++) {
+		struct ref_array_item *item = array->items[i];
+		add_pending_object(&revs, &item->commit->object, item->refname);
+		commit_list_insert(item->commit, &to_clear);
+	}
+
+	filter->merge_commit->object.flags |= UNINTERESTING;
+	add_pending_object(&revs, &filter->merge_commit->object, "");
+
+	revs.limited = 1;
+	if (prepare_revision_walk(&revs))
+		die(_("revision walk setup failed"));
+
+	old_nr = array->nr;
+	array->nr = 0;
+
+	for (i = 0; i < old_nr; i++) {
+		struct ref_array_item *item = array->items[i];
+		struct commit *commit = item->commit;
+
+		int is_merged = !!(commit->object.flags & UNINTERESTING);
+
+		if (is_merged == (filter->merge == REF_FILTER_MERGED_INCLUDE))
+			array->items[array->nr++] = array->items[i];
+		else
+			free_array_item(item);
+	}
+
+	for (p = to_clear; p; p = p->next)
+		clear_commit_marks(p->item, ALL_REV_FLAGS);
+	clear_commit_marks(filter->merge_commit, ALL_REV_FLAGS);
+	free_commit_list(to_clear);
+}
+
 /*
  * API for filtering a set of refs. The refs are provided and iterated
  * over using the for_each_ref_fn(). The refs are stored into and filtered
@@ -938,7 +991,13 @@ void ref_array_clear(struct ref_array *array)
  */
 int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
 {
-	return for_each_ref_fn(ref_filter_handler, data);
+	int ret;
+
+	ret = for_each_ref_fn(ref_filter_handler, data);
+	if (data->filter.merge_commit)
+		do_merge_filter(data);
+
+	return ret;
 }
 
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
diff --git a/ref-filter.h b/ref-filter.h
index a8980e7..622b942 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -28,6 +28,7 @@ struct ref_array_item {
 	unsigned char objectname[20];
 	int flag;
 	char *symref;
+	struct commit *commit;
 	struct atom_value *value;
 	char refname[FLEX_ARRAY];
 };
@@ -40,6 +41,13 @@ struct ref_array {
 struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
+
+	enum {
+		REF_FILTER_MERGED_NONE = 0,
+		REF_FILTER_MERGED_INCLUDE,
+		REF_FILTER_MERGED_OMIT
+	} merge;
+	struct commit *merge_commit;
 };
 
 struct ref_filter_cbdata {
-- 
2.4.2
Matthieu Moy· Jun 8, 2015, 17:51 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 5/9] ref-filter: implement '--merged' and '--no-merged' options

Karthik Nayak <karthik.188@gmail.com> writes:
> +	if(filter->merge_commit) {
space after if.
Show 10 quoted lines
> @@ -938,7 +991,13 @@ void ref_array_clear(struct ref_array *array)
>   */
>  int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
>  {
> -	return for_each_ref_fn(ref_filter_handler, data);
> +	int ret;
> +
> +	ret = for_each_ref_fn(ref_filter_handler, data);
> +	if (data->filter.merge_commit)
> +		do_merge_filter(data);

Reading this, I first wondered why you did not do the merge_filter as part of ref_filter_handler. It seems weird to fill-in an array and then re-filter it. I think it would make sense to add a few comments, like

/* Simple per-ref filtering */ ret = for_each_ref_fn(ref_filter_handler, data);

/* Filters that need revision walking */ if (data->filter.merge_commit) ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:53 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 5/9] ref-filter: implement '--merged' and '--no-merged' options

On 06/08/2015 11:21 PM, Matthieu Moy wrote:
Show 28 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +	if(filter->merge_commit) {
>
> space after if.
>
>> @@ -938,7 +991,13 @@ void ref_array_clear(struct ref_array *array)
>>    */
>>   int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
>>   {
>> -	return for_each_ref_fn(ref_filter_handler, data);
>> +	int ret;
>> +
>> +	ret = for_each_ref_fn(ref_filter_handler, data);
>> +	if (data->filter.merge_commit)
>> +		do_merge_filter(data);
>
> Reading this, I first wondered why you did not do the merge_filter as
> part of ref_filter_handler. It seems weird to fill-in an array and then
> re-filter it. I think it would make sense to add a few comments, like
>
> /* Simple per-ref filtering */
> ret = for_each_ref_fn(ref_filter_handler, data);
>
> /* Filters that need revision walking */
> if (data->filter.merge_commit)
> ...
>
Thanks! will add the space and comments as suggested.
-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 6/9] for-each-ref: add '--merged' and '--no-merged' options

Add the '--merged' and '--no-merged' options provided by 'ref-filter'. The '--merged' option lets the user to only list refs merged into the named commit. The '--no-merged' option lets the user to only list refs not merged into the named commit.

Add documentation for the same.
Based-on-patch-by: Jeff King <peff@peff.net>
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>
---
 Documentation/git-for-each-ref.txt | 10 +++++++++-
 builtin/for-each-ref.c             |  3 +++
 2 files changed, 12 insertions(+), 1 deletion(-)
Show changes to 2 files +12 −1

Documentation/git-for-each-ref.txt, builtin/for-each-ref.c

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index e9f6a8a..74f24f4 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
-		   [--points-at <object>]
+		   [--points-at <object>] [(--merged | --no-merged) <object>]
 
 DESCRIPTION
 -----------
@@ -66,6 +66,14 @@ OPTIONS
 --points-at <object>::
 	Only list tags of the given object.
 
+--merged [<commit>]::
+	Only list refs whose tips are reachable from the
+	specified commit (HEAD if not specified).
+
+--no-merged [<commit>]::
+	Only list refs whose tips are not reachable from the
+	specified commit (HEAD if not specified).
+
 FIELD NAMES
 -----------
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b9d180a..82605ed 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -8,6 +8,7 @@
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
+	N_("git for-each-ref [(--merged | --no-merged) <object>]"),
 	NULL
 };
 
@@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
 			     N_("object"), N_("print only tags of the object"),
 			     parse_opt_points_at),
+		OPT_MERGED(&ref_cbdata.filter, N_("print only merged refs")),
+		OPT_NO_MERGED(&ref_cbdata.filter, N_("print only not merged refs")),
 		OPT_END(),
 	};
 
-- 
2.4.2
Matthieu Moy· Jun 8, 2015, 17:53 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 6/9] for-each-ref: add '--merged' and '--no-merged' options

Karthik Nayak <karthik.188@gmail.com> writes:
Show 6 quoted lines
> Add the '--merged' and '--no-merged' options provided by 'ref-filter'.
> The '--merged' option lets the user to only list refs merged into the
> named commit. The '--no-merged' option lets the user to only list refs
> not merged into the named commit.
>
> Add documentation for the same.

This also requires some tests. The algorithmic part will be validated by 'git tag --merged' using this library, but you need to check the codepath from the command-line to the lib in for-each-ref.

Show 6 quoted lines
> @@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
>  			     N_("object"), N_("print only tags of the object"),
>  			     parse_opt_points_at),
> +		OPT_MERGED(&ref_cbdata.filter, N_("print only merged refs")),
> +		OPT_NO_MERGED(&ref_cbdata.filter, N_("print only not merged refs")),
I'd spell that "only refs that are not merged".
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Karthik Nayak· Jun 8, 2015, 18:54 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH 6/9] for-each-ref: add '--merged' and '--no-merged' options

On 06/08/2015 11:23 PM, Matthieu Moy wrote:
Show 22 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Add the '--merged' and '--no-merged' options provided by 'ref-filter'.
>> The '--merged' option lets the user to only list refs merged into the
>> named commit. The '--no-merged' option lets the user to only list refs
>> not merged into the named commit.
>>
>> Add documentation for the same.
>
> This also requires some tests. The algorithmic part will be validated by
> 'git tag --merged' using this library, but you need to check the
> codepath from the command-line to the lib in for-each-ref.
>
>> @@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>>   		OPT_CALLBACK(0, "points-at", &ref_cbdata.filter.points_at,
>>   			     N_("object"), N_("print only tags of the object"),
>>   			     parse_opt_points_at),
>> +		OPT_MERGED(&ref_cbdata.filter, N_("print only merged refs")),
>> +		OPT_NO_MERGED(&ref_cbdata.filter, N_("print only not merged refs")),
>
> I'd spell that "only refs that are not merged".
>

Like I mentioned, no tests were written. Will change that.

-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 7/9] parse-options.h: add macros for '--contains' option

Add a macro for using the '--contains' option in parse-options.h also include an optional '--with' option macro which performs the same action as '--contains'.

Make tag.c use this new macro
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>
---
 builtin/tag.c   | 14 ++------------
 parse-options.h | 10 ++++++++++
 2 files changed, 12 insertions(+), 12 deletions(-)
Show changes to 2 files +12 −12

builtin/tag.c, parse-options.h

diff --git a/builtin/tag.c b/builtin/tag.c
index eda76ba..e16668b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -591,23 +591,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 		OPT_GROUP(N_("Tag listing options")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
+		OPT_CONTAINS(&with_commit, N_("print only tags that contain the commit")),
+		OPT_WITH(&with_commit, N_("print only tags that contain the commit")),
 		{
 			OPTION_CALLBACK, 0, "sort", &tag_sort, N_("type"), N_("sort tags"),
 			PARSE_OPT_NONEG, parse_opt_sort
 		},
 		{
-			OPTION_CALLBACK, 0, "contains", &with_commit, N_("commit"),
-			N_("print only tags that contain the commit"),
-			PARSE_OPT_LASTARG_DEFAULT,
-			parse_opt_with_commit, (intptr_t)"HEAD",
-		},
-		{
-			OPTION_CALLBACK, 0, "with", &with_commit, N_("commit"),
-			N_("print only tags that contain the commit"),
-			PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
-			parse_opt_with_commit, (intptr_t)"HEAD",
-		},
-		{
 			OPTION_CALLBACK, 0, "points-at", &points_at, N_("object"),
 			N_("print only tags of the object"), 0, parse_opt_points_at
 		},
diff --git a/parse-options.h b/parse-options.h
index 7bcf0f3..d8389ad 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -254,5 +254,15 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
 	  PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
 	  parse_opt_merge_filter, (intptr_t) "HEAD" \
 	}
+#define OPT_CONTAINS(filter, h) \
+	{ OPTION_CALLBACK, 0, "contains", (filter), N_("commit"), (h), \
+	  PARSE_OPT_LASTARG_DEFAULT, \
+	  parse_opt_with_commit, (intptr_t) "HEAD" \
+	}
+#define OPT_WITH(filter, h) \
+	{ OPTION_CALLBACK, 0, "with", (filter), N_("commit"), (h), \
+	  PARSE_OPT_LASTARG_DEFAULT, \
+	  parse_opt_with_commit, (intptr_t) "HEAD" \
+	}
 
 #endif
-- 
2.4.2
Junio C Hamano· Jun 8, 2015, 19:32 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 7/9] parse-options.h: add macros for '--contains' option

Karthik Nayak <karthik.188@gmail.com> writes:
Show 10 quoted lines
> +#define OPT_CONTAINS(filter, h) \
> +	{ OPTION_CALLBACK, 0, "contains", (filter), N_("commit"), (h), \
> +	  PARSE_OPT_LASTARG_DEFAULT, \
> +	  parse_opt_with_commit, (intptr_t) "HEAD" \
> +	}
> +#define OPT_WITH(filter, h) \
> +	{ OPTION_CALLBACK, 0, "with", (filter), N_("commit"), (h), \
> +	  PARSE_OPT_LASTARG_DEFAULT, \
> +	  parse_opt_with_commit, (intptr_t) "HEAD" \
> +	}

The redundancy bothers me. Can't we do a bit better than that, perhaps like this?

#define _OPT_CONTAINS_OR_WITH(name, variable, help) \
	{ OPTION_CALLBACK, 0, name, (variable), N_("commit"), (help), \
	  PARSE_OPT_LASTARG_DEFAULT, \
	  parse_opt_with_commit, (intptr_t) "HEAD" \
	}
#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h)
Karthik Nayak· Jun 9, 2015, 12:49 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 7/9] parse-options.h: add macros for '--contains' option

On 06/09/2015 01:02 AM, Junio C Hamano wrote:
Show 23 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +#define OPT_CONTAINS(filter, h) \
>> +	{ OPTION_CALLBACK, 0, "contains", (filter), N_("commit"), (h), \
>> +	  PARSE_OPT_LASTARG_DEFAULT, \
>> +	  parse_opt_with_commit, (intptr_t) "HEAD" \
>> +	}
>> +#define OPT_WITH(filter, h) \
>> +	{ OPTION_CALLBACK, 0, "with", (filter), N_("commit"), (h), \
>> +	  PARSE_OPT_LASTARG_DEFAULT, \
>> +	  parse_opt_with_commit, (intptr_t) "HEAD" \
>> +	}
>
> The redundancy bothers me.  Can't we do a bit better than that,
> perhaps like this?
>
> #define _OPT_CONTAINS_OR_WITH(name, variable, help) \
> 	{ OPTION_CALLBACK, 0, name, (variable), N_("commit"), (help), \
> 	  PARSE_OPT_LASTARG_DEFAULT, \
> 	  parse_opt_with_commit, (intptr_t) "HEAD" \
> 	}
> #define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h)
>
This seems better, thanks!
-- 
Regards,
Karthik
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 8/9] ref-filter: add '--contains' option

'tag -l' and 'branch -l' have two different ways of finding out if a certain ref contains a commit. Implement both these methods in ref-filter and give the caller of ref-filter API the option to pick which implementation to be used.

'branch -l' uses 'is_descendant_of()' from commit.c which is left as the default implementation to be used.

'tag -l' uses a more specific algorithm since ffc4b80. This implementation is used whenever the 'with_commit_tag_algo' bit is set in 'struct ref_filter'.

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 | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 ref-filter.h |   3 ++
 2 files changed, 115 insertions(+), 2 deletions(-)
Show changes to 2 files +115 −2

ref-filter.c, ref-filter.h

diff --git a/ref-filter.c b/ref-filter.c
index 2be9df2..6c5f5c2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -818,6 +818,113 @@ static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom
 	*v = &ref->value[atom];
 }
 
+enum contains_result {
+	CONTAINS_UNKNOWN = -1,
+	CONTAINS_NO = 0,
+	CONTAINS_YES = 1
+};
+
+/*
+ * Mimicking the real stack, this stack lives on the heap, avoiding stack
+ * overflows.
+ *
+ * At each recursion step, the stack items points to the commits whose
+ * ancestors are to be inspected.
+ */
+struct stack {
+	int nr, alloc;
+	struct stack_entry {
+		struct commit *commit;
+		struct commit_list *parents;
+	} *stack;
+};
+
+static int in_commit_list(const struct commit_list *want, struct commit *c)
+{
+	for (; want; want = want->next)
+		if (!hashcmp(want->item->object.sha1, c->object.sha1))
+			return 1;
+	return 0;
+}
+
+/*
+ * Test whether the candidate or one of its parents is contained in the list.
+ * Do not recurse to find out, though, but return -1 if inconclusive.
+ */
+static enum contains_result contains_test(struct commit *candidate,
+			    const struct commit_list *want)
+{
+	/* was it previously marked as containing a want commit? */
+	if (candidate->object.flags & TMP_MARK)
+		return 1;
+	/* or marked as not possibly containing a want commit? */
+	if (candidate->object.flags & UNINTERESTING)
+		return 0;
+	/* or are we it? */
+	if (in_commit_list(want, candidate)) {
+		candidate->object.flags |= TMP_MARK;
+		return 1;
+	}
+
+	if (parse_commit(candidate) < 0)
+		return 0;
+
+	return -1;
+}
+
+static void push_to_stack(struct commit *candidate, struct stack *stack)
+{
+	ALLOC_GROW(stack->stack, stack->nr + 1, stack->alloc);
+	stack->stack[stack->nr].commit = candidate;
+	stack->stack[stack->nr++].parents = candidate->parents;
+}
+
+static enum contains_result contains_tag_algo(struct commit *candidate,
+		const struct commit_list *want)
+{
+	struct stack stack = { 0, 0, NULL };
+	int result = contains_test(candidate, want);
+
+	if (result != CONTAINS_UNKNOWN)
+		return result;
+
+	push_to_stack(candidate, &stack);
+	while (stack.nr) {
+		struct stack_entry *entry = &stack.stack[stack.nr - 1];
+		struct commit *commit = entry->commit;
+		struct commit_list *parents = entry->parents;
+
+		if (!parents) {
+			commit->object.flags |= UNINTERESTING;
+			stack.nr--;
+		}
+		/*
+		 * If we just popped the stack, parents->item has been marked,
+		 * therefore contains_test will return a meaningful 0 or 1.
+		 */
+		else switch (contains_test(parents->item, want)) {
+		case CONTAINS_YES:
+			commit->object.flags |= TMP_MARK;
+			stack.nr--;
+			break;
+		case CONTAINS_NO:
+			entry->parents = parents->next;
+			break;
+		case CONTAINS_UNKNOWN:
+			push_to_stack(parents->item, &stack);
+			break;
+		}
+	}
+	free(stack.stack);
+	return contains_test(candidate, want);
+}
+
+static int commit_contains(struct ref_filter *filter, struct commit *commit)
+{
+	if (filter->with_commit_tag_algo)
+		return contains_tag_algo(commit, filter->with_commit);
+	return is_descendant_of(commit, filter->with_commit);
+}
 /*
  * Given a refname, return 1 if the refname matches with one of the patterns
  * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
@@ -902,10 +1009,13 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	if (!match_points_at(&filter->points_at, oid->hash, refname))
 		return 0;
 
-	if(filter->merge_commit) {
-		commit = lookup_commit_reference_gently(sha1, 1);
+	if(filter->merge_commit || filter->with_commit) {
+		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit)
 			return 0;
+		if(filter->with_commit &&
+		   !commit_contains(filter, commit))
+			return 0;
 	}
 
 	/*
diff --git a/ref-filter.h b/ref-filter.h
index 622b942..c71704e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -41,6 +41,7 @@ struct ref_array {
 struct ref_filter {
 	const char **name_patterns;
 	struct sha1_array points_at;
+	struct commit_list *with_commit;
 
 	enum {
 		REF_FILTER_MERGED_NONE = 0,
@@ -48,6 +49,8 @@ struct ref_filter {
 		REF_FILTER_MERGED_OMIT
 	} merge;
 	struct commit *merge_commit;
+
+	unsigned int with_commit_tag_algo: 1;
 };
 
 struct ref_filter_cbdata {
-- 
2.4.2
Karthik Nayak· Jun 6, 2015, 20:04 UTC · re: Karthik Nayak · lore

[RFC/PATCH 9/9] for-each-ref: add '--contains' option

Add the '--contains' option provided by 'ref-filter'. The '--contains' lists only refs which are contain the specific commit (HEAD of the branch if no commit give).

Add documentation for the same.
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>
---
 Documentation/git-for-each-ref.txt | 5 +++++
 builtin/for-each-ref.c             | 2 ++
 2 files changed, 7 insertions(+)
Show changes to 2 files +7 −0

Documentation/git-for-each-ref.txt, builtin/for-each-ref.c

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 74f24f4..7ac64ea 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
 		   [--points-at <object>] [(--merged | --no-merged) <object>]
+		   [--contains <object>]
 
 DESCRIPTION
 -----------
@@ -74,6 +75,10 @@ OPTIONS
 	Only list refs whose tips are not reachable from the
 	specified commit (HEAD if not specified).
 
+--contains [<commit>]::
+	Only list tags which contain the specified commit (HEAD if not
+	specified).
+
 FIELD NAMES
 -----------
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 82605ed..44e1467 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@ static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
 	N_("git for-each-ref [--points-at <object>]"),
 	N_("git for-each-ref [(--merged | --no-merged) <object>]"),
+	N_("git for-each-ref [--contains <object>]"),
 	NULL
 };
 
@@ -41,6 +42,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 			     parse_opt_points_at),
 		OPT_MERGED(&ref_cbdata.filter, N_("print only merged refs")),
 		OPT_NO_MERGED(&ref_cbdata.filter, N_("print only not merged refs")),
+		OPT_CONTAINS(&ref_cbdata.filter.with_commit, N_("print only refs which contain the commit")),
 		OPT_END(),
 	};
 
-- 
2.4.2
Junio C Hamano· Jun 8, 2015, 19:00 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 1/9] tag: libify parse_opt_points_at()

Karthik Nayak <karthik.188@gmail.com> writes:
Show 26 quoted lines
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index be8c413..a4d294d 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -4,6 +4,7 @@
>  #include "commit.h"
>  #include "color.h"
>  #include "string-list.h"
> +#include "sha1-array.h"
>  ...
> +int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
> +{
> +	unsigned char sha1[20];
> +
> +	if (unset) {
> +		sha1_array_clear(opt->value);
> +		return 0;
> +	}
> +	if (!arg)
> +		return -1;
> +	if (get_sha1(arg, sha1))
> +		return error(_("malformed object name '%s'"), arg);
> +	sha1_array_append(opt->value, sha1);
> +	return 0;
> +}
> +

This feels way too specialized to live as part of parse_options infrastructure.

The existing caller(s) may want to use this callback for parsing "points-at" option they have, but is that the only plausible use of this callback? It looks to be usable by any future caller that wants to take and accumulate any object names into an sha1-array, so perhaps rename it to be a bit more generic to represent its nature better?

	parse_opt_object_name()
or something?

I also wonder if we can (and want to) refactor the users of with-commit callback. Have them use this to obtain an sha1-array and then convert what they received into a commit-list themselves.

Show 15 quoted lines
>  int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
>  {
>  	int *target = opt->value;
> diff --git a/parse-options.h b/parse-options.h
> index c71e9da..3ae16a1 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -220,6 +220,7 @@ extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
>  extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
>  extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
>  extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
> +extern int parse_opt_points_at(const struct option *, const char *, int);
>  extern int parse_opt_with_commit(const struct option *, const char *, int);
>  extern int parse_opt_tertiary(const struct option *, const char *, int);
>  extern int parse_opt_string_list(const struct option *, const char *, int);
Karthik Nayak· Jun 9, 2015, 11:50 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH 1/9] tag: libify parse_opt_points_at()

On 06/09/2015 12:30 AM, Junio C Hamano wrote:
Show 14 quoted lines
>
> This feels way too specialized to live as part of parse_options
> infrastructure.
>
> The existing caller(s) may want to use this callback for parsing
> "points-at" option they have, but is that the only plausible use of
> this callback?  It looks to be usable by any future caller that
> wants to take and accumulate any object names into an sha1-array, so
> perhaps rename it to be a bit more generic to represent its nature
> better?
>
> 	parse_opt_object_name()
>
> or something?
This makes sense! Will change.
Show 5 quoted lines
>
> I also wonder if we can (and want to) refactor the users of
> with-commit callback.  Have them use this to obtain an sha1-array
> and then convert what they received into a commit-list themselves.
>

But wouldn't that be too much of an overhead to iterate through the sha1-array and convert it to a commit-list?

-- 
Regards,
Karthik
Christian Couder· Jun 7, 2015, 13:10 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 0/9] add options to ref-filter

On Sat, Jun 6, 2015 at 10:03 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> This is a follow up series to the one posted here
> http://thread.gmane.org/gmane.comp.version-control.git/270922
>
> This patch series adds '--ponints-at', '--merged', '--no-merged' and
s/--ponints-at/--points-at/
> '--contain' options to 'ref-filter' and uses these options in
> 'for-each-ref'.
Karthik Nayak· Jun 8, 2015, 15:00 UTC · re: Christian Couder · lore

Re: [RFC/PATCH 0/9] add options to ref-filter

On 06/07/2015 06:40 PM, Christian Couder wrote:
Show 10 quoted lines
> On Sat, Jun 6, 2015 at 10:03 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
> > This is a follow up series to the one posted here
> > http://thread.gmane.org/gmane.comp.version-control.git/270922
> >
> > This patch series adds '--ponints-at', '--merged', '--no-merged' and
>
> s/--ponints-at/--points-at/
>
> > '--contain' options to 'ref-filter' and uses these options in
> > 'for-each-ref'.
Thanks!
-- 
Regards,
Karthik
Junio C Hamano· Jun 8, 2015, 19:34 UTC · re: Karthik Nayak · lore

Re: [RFC/PATCH 0/9] add options to ref-filter

Karthik Nayak <karthik.188@gmail.com> writes:
Show 6 quoted lines
> This is a follow up series to the one posted here
> http://thread.gmane.org/gmane.comp.version-control.git/270922
>
> This patch series adds '--ponints-at', '--merged', '--no-merged' and
> '--contain' options to 'ref-filter' and uses these options in
> for-each-ref'.

The structure of the series looked sensible (I stopped at around 7/9, though, for now); even though a few style violations were somewhat irritating, overall it was a pleasant read.

Thanks.

← back to recent threads