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

2015-06-09 Thread Karthik Nayak 

On 06/09/2015 12:30 AM, Junio C Hamano wrote:


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.



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
--
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

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

2015-06-08 Thread Junio C Hamano 
Karthik Nayak karthik@gmail.com writes:

 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.

  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);
--
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

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

2015-06-06 Thread Karthik Nayak 
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 p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
 builtin/tag.c  | 19 +--
 parse-options-cb.c | 17 +
 parse-options.h|  1 +
 3 files changed, 19 insertions(+), 18 deletions(-)

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

--
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