git tag --contains <id> prints the whole help text if <id> is
invalid. It should only show the error message instead.

This bug was a side effect of looking up the commit in option
parser callback. When a error occurs in the option parser, the
full usage is shown. To fix this bug, the part related to
looking up the commit was moved outside of the option parser
to the ref-filter module.

Basically, the option parser only parses strings that represent
commits and the ref-filter performs the commit look-up. If an
error occurs during the option parsing, then it must be an invalid
argument and the user should be informed of usage, but if a error
occurs during ref-filtering, then it is a problem with the
argument.

Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebast...@gmail.com>
---
 builtin/tag.c           | 11 +++++++++--
 parse-options.h         | 11 +++++++++++
 ref-filter.c            | 19 +++++++++++++++++++
 ref-filter.h            |  3 +++
 t/t7013-tag-contains.sh | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100755 t/t7013-tag-contains.sh

diff --git a/builtin/tag.c b/builtin/tag.c
index a7e6a5b0f..3fce14713 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -396,6 +396,12 @@ 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_STRS(&filter.with_commit_strs, N_("print only tags 
that contain the commit")),
+               OPT_NO_CONTAINS_STRS(&filter.no_commit_strs, N_("print only 
tags that don't contain the commit")),
+               OPT_WITH_STRS(&filter.with_commit_strs, N_("print only tags 
that contain the commit")),
+               OPT_WITHOUT_STRS(&filter.no_commit_strs, N_("print only tags 
that don't contain the commit")),
+
                OPT_CONTAINS(&filter.with_commit, N_("print only tags that 
contain the commit")),
                OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that 
don't contain the commit")),
                OPT_WITH(&filter.with_commit, N_("print only tags that contain 
the commit")),
@@ -437,6 +443,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
                        cmdmode = 'l';
                else if (filter.with_commit || filter.no_commit ||
                         filter.points_at.nr || filter.merge_commit ||
+                        filter.with_commit_strs.nr || filter.no_commit_strs.nr 
||
                         filter.lines != -1)
                        cmdmode = 'l';
        }
@@ -473,9 +480,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
        }
        if (filter.lines != -1)
                die(_("-n option is only allowed in list mode"));
-       if (filter.with_commit)
+       if (filter.with_commit || filter.with_commit_strs.nr)
                die(_("--contains option is only allowed in list mode"));
-       if (filter.no_commit)
+       if (filter.no_commit || filter.no_commit_strs.nr)
                die(_("--no-contains option is only allowed in list mode"));
        if (filter.points_at.nr)
                die(_("--points-at option is only allowed in list mode"));
diff --git a/parse-options.h b/parse-options.h
index af711227a..3aa8ddd46 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -258,9 +258,20 @@ extern int parse_opt_passthru_argv(const struct option *, 
const char *, int);
          PARSE_OPT_LASTARG_DEFAULT | flag, \
          parse_opt_commits, (intptr_t) "HEAD" \
        }
+#define _OPT_CONTAINS_OR_WITH_STRS(name, variable, help, flag) \
+       { OPTION_CALLBACK, 0, name, (variable), N_("commit"), (help), \
+         PARSE_OPT_LASTARG_DEFAULT | flag, \
+         parse_opt_string_list, (intptr_t) "HEAD" \
+       }
+
 #define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 
PARSE_OPT_NONEG)
 #define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, 
PARSE_OPT_NONEG)
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | 
PARSE_OPT_NONEG)
 #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, 
PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 
+#define OPT_CONTAINS_STRS(v, h) _OPT_CONTAINS_OR_WITH_STRS("contains", v, h, 
PARSE_OPT_NONEG)
+#define OPT_NO_CONTAINS_STRS(v, h) _OPT_CONTAINS_OR_WITH_STRS("no-contains", 
v, h, PARSE_OPT_NONEG)
+#define OPT_WITH_STRS(v, h) _OPT_CONTAINS_OR_WITH_STRS("with", v, h, 
PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
+#define OPT_WITHOUT_STRS(v, h) _OPT_CONTAINS_OR_WITH_STRS("without", v, h, 
PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
+
 #endif
diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7..e7bdaa27f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2000,6 +2000,21 @@ static void do_merge_filter(struct ref_filter_cbdata 
*ref_cbdata)
        free(to_clear);
 }
 
+int add_str_to_commit_list(struct string_list_item *item, void *commit_list)
+{
+       struct object_id oid;
+       struct commit *commit;
+
+       if (get_oid(item->string, &oid))
+               die(_("malformed object name %s"), item->string);
+       commit = lookup_commit_reference(&oid);
+       if (!commit)
+               die(_("no such commit %s"), item->string);
+       commit_list_insert(commit, commit_list);
+
+       return 0;
+}
+
 /*
  * API for filtering a set of refs. Based on the type of refs the user
  * has requested, we iterate through those refs and apply filters
@@ -2012,6 +2027,10 @@ int filter_refs(struct ref_array *array, struct 
ref_filter *filter, unsigned int
        int ret = 0;
        unsigned int broken = 0;
 
+       /* Convert string representation and add to commit list. */
+       for_each_string_list(&filter->with_commit_strs, add_str_to_commit_list, 
&filter->with_commit);
+       for_each_string_list(&filter->no_commit_strs, add_str_to_commit_list, 
&filter->no_commit);
+
        ref_cbdata.array = array;
        ref_cbdata.filter = filter;
 
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b3..62f37760f 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -55,6 +55,9 @@ struct ref_filter {
        struct commit_list *with_commit;
        struct commit_list *no_commit;
 
+       struct string_list with_commit_strs;
+       struct string_list no_commit_strs;
+
        enum {
                REF_FILTER_MERGED_NONE = 0,
                REF_FILTER_MERGED_INCLUDE,
diff --git a/t/t7013-tag-contains.sh b/t/t7013-tag-contains.sh
new file mode 100755
index 000000000..65119dada
--- /dev/null
+++ b/t/t7013-tag-contains.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='Test if git tag --contains prints only the error
+when used correctly but with an inexistent tag as argument'
+
+. ./test-lib.sh
+
+test_expect_success 'setup ' '
+       git init . &&
+       echo "this is a test" >file &&
+       git add -A &&
+       git commit -am "tag test" &&
+       git tag "v1.0" &&
+       git tag "v1.1"
+'
+
+test_expect_success 'tag --contains <existent_tag>' '
+       ! (git tag --contains "v1.0" 2>&1 | grep -o "usage")
+'
+
+test_expect_success 'tag --contains <existent_tag>' '
+       ! (git tag --contains "v1.1" 2>&1 | grep -o "usage")
+'
+
+test_expect_success 'tag --contains <inexistent_tag>' '
+       ! (git tag --contains "notag" 2>&1 | grep -o "usage")
+'
+
+test_expect_success 'tag --contains <inexistent_tag>' '
+       ! (git tag --contains "v1.2" 2>&1 | grep -o "usage")
+'
+
+test_expect_success 'tag usage error' '
+       git tag --sort 2>&1 | grep -o "usage"
+'
+
+test_done
-- 
2.16.1.194.gb2e45c695.dirty

Reply via email to