Merge branch 'jc/parseopt-command-modes'
authorJunio C Hamano <gitster@pobox.com>
Wed, 4 Sep 2013 19:37:52 +0000 (12:37 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 4 Sep 2013 19:37:52 +0000 (12:37 -0700)
Many commands use --dashed-option as a operation mode selector
(e.g. "git tag --delete") that the user can use at most one
(e.g. "git tag --delete --verify" is a nonsense) and you cannot
negate (e.g. "git tag --no-delete" is a nonsense). Make it easier
for users of parse_options() to enforce these restrictions.

* jc/parseopt-command-modes:
tag: use OPT_CMDMODE
parse-options: add OPT_CMDMODE()

builtin/tag.c
parse-options.c
parse-options.h
index af3af3f64935c3f92ed3f85acd44ed2ca2a35b3f..d8ae5aae2828c1e36a79f6182caaf6666ffc881b 100644 (file)
@@ -436,18 +436,18 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
        struct ref_lock *lock;
        struct create_tag_options opt;
        char *cleanup_arg = NULL;
-       int annotate = 0, force = 0, lines = -1, list = 0,
-               delete = 0, verify = 0;
+       int annotate = 0, force = 0, lines = -1;
+       int cmdmode = 0;
        const char *msgfile = NULL, *keyid = NULL;
        struct msg_arg msg = { 0, STRBUF_INIT };
        struct commit_list *with_commit = NULL;
        struct option options[] = {
-               OPT_BOOLEAN('l', "list", &list, N_("list tag names")),
+               OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
                { OPTION_INTEGER, 'n', NULL, &lines, N_("n"),
                                N_("print <n> lines of each tag message"),
                                PARSE_OPT_OPTARG, NULL, 1 },
-               OPT_BOOLEAN('d', "delete", &delete, N_("delete tags")),
-               OPT_BOOLEAN('v', "verify", &verify, N_("verify tags")),
+               OPT_CMDMODE('d', "delete", &cmdmode, N_("delete tags"), 'd'),
+               OPT_CMDMODE('v', "verify", &cmdmode, N_("verify tags"), 'v'),
 
                OPT_GROUP(N_("Tag creation options")),
                OPT_BOOLEAN('a', "annotate", &annotate,
@@ -489,22 +489,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
        }
        if (opt.sign)
                annotate = 1;
-       if (argc == 0 && !(delete || verify))
-               list = 1;
+       if (argc == 0 && !cmdmode)
+               cmdmode = 'l';
 
-       if ((annotate || msg.given || msgfile || force) &&
-           (list || delete || verify))
+       if ((annotate || msg.given || msgfile || force) && (cmdmode != 0))
                usage_with_options(git_tag_usage, options);
 
-       if (list + delete + verify > 1)
-               usage_with_options(git_tag_usage, options);
        finalize_colopts(&colopts, -1);
-       if (list && lines != -1) {
+       if (cmdmode == 'l' && lines != -1) {
                if (explicitly_enable_column(colopts))
                        die(_("--column and -n are incompatible"));
                colopts = 0;
        }
-       if (list) {
+       if (cmdmode == 'l') {
                int ret;
                if (column_active(colopts)) {
                        struct column_options copts;
@@ -523,9 +520,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
                die(_("--contains option is only allowed with -l."));
        if (points_at.nr)
                die(_("--points-at option is only allowed with -l."));
-       if (delete)
+       if (cmdmode == 'd')
                return for_each_tag_name(argv, delete_tag);
-       if (verify)
+       if (cmdmode == 'v')
                return for_each_tag_name(argv, verify_tag);
 
        if (msg.given || msgfile) {
index c2cbca25cc6bb010996063c88c9af3e7b38aad5a..62e9b1cc6831768315dd74eac30348668de22da3 100644 (file)
@@ -43,8 +43,42 @@ static void fix_filename(const char *prefix, const char **file)
        *file = xstrdup(prefix_filename(prefix, strlen(prefix), *file));
 }
 
+static int opt_command_mode_error(const struct option *opt,
+                                 const struct option *all_opts,
+                                 int flags)
+{
+       const struct option *that;
+       struct strbuf message = STRBUF_INIT;
+       struct strbuf that_name = STRBUF_INIT;
+
+       /*
+        * Find the other option that was used to set the variable
+        * already, and report that this is not compatible with it.
+        */
+       for (that = all_opts; that->type != OPTION_END; that++) {
+               if (that == opt ||
+                   that->type != OPTION_CMDMODE ||
+                   that->value != opt->value ||
+                   that->defval != *(int *)opt->value)
+                       continue;
+
+               if (that->long_name)
+                       strbuf_addf(&that_name, "--%s", that->long_name);
+               else
+                       strbuf_addf(&that_name, "-%c", that->short_name);
+               strbuf_addf(&message, ": incompatible with %s", that_name.buf);
+               strbuf_release(&that_name);
+               opterror(opt, message.buf, flags);
+               strbuf_release(&message);
+               return -1;
+       }
+       return opterror(opt, ": incompatible with something else", flags);
+}
+
 static int get_value(struct parse_opt_ctx_t *p,
-                    const struct option *opt, int flags)
+                    const struct option *opt,
+                    const struct option *all_opts,
+                    int flags)
 {
        const char *s, *arg;
        const int unset = flags & OPT_UNSET;
@@ -83,6 +117,16 @@ static int get_value(struct parse_opt_ctx_t *p,
                *(int *)opt->value = unset ? 0 : opt->defval;
                return 0;
 
+       case OPTION_CMDMODE:
+               /*
+                * Giving the same mode option twice, although is unnecessary,
+                * is not a grave error, so let it pass.
+                */
+               if (*(int *)opt->value && *(int *)opt->value != opt->defval)
+                       return opt_command_mode_error(opt, all_opts, flags);
+               *(int *)opt->value = opt->defval;
+               return 0;
+
        case OPTION_SET_PTR:
                *(void **)opt->value = unset ? NULL : (void *)opt->defval;
                return 0;
@@ -143,12 +187,13 @@ static int get_value(struct parse_opt_ctx_t *p,
 
 static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
 {
+       const struct option *all_opts = options;
        const struct option *numopt = NULL;
 
        for (; options->type != OPTION_END; options++) {
                if (options->short_name == *p->opt) {
                        p->opt = p->opt[1] ? p->opt + 1 : NULL;
-                       return get_value(p, options, OPT_SHORT);
+                       return get_value(p, options, all_opts, OPT_SHORT);
                }
 
                /*
@@ -177,6 +222,7 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
 static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                           const struct option *options)
 {
+       const struct option *all_opts = options;
        const char *arg_end = strchr(arg, '=');
        const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
        int abbrev_flags = 0, ambiguous_flags = 0;
@@ -253,7 +299,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                                continue;
                        p->opt = rest + 1;
                }
-               return get_value(p, options, flags ^ opt_flags);
+               return get_value(p, options, all_opts, flags ^ opt_flags);
        }
 
        if (ambiguous_option)
@@ -265,18 +311,20 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                        (abbrev_flags & OPT_UNSET) ?  "no-" : "",
                        abbrev_option->long_name);
        if (abbrev_option)
-               return get_value(p, abbrev_option, abbrev_flags);
+               return get_value(p, abbrev_option, all_opts, abbrev_flags);
        return -2;
 }
 
 static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
                            const struct option *options)
 {
+       const struct option *all_opts = options;
+
        for (; options->type != OPTION_END; options++) {
                if (!(options->flags & PARSE_OPT_NODASH))
                        continue;
                if (options->short_name == arg[0] && arg[1] == '\0')
-                       return get_value(p, options, OPT_SHORT);
+                       return get_value(p, options, all_opts, OPT_SHORT);
        }
        return -2;
 }
index 9b94596e4aa113518d502502a0ca1bf804a60cbc..f37fc88143f0cc0277b33d55ec92cc43034774cf 100644 (file)
@@ -13,6 +13,7 @@ enum parse_opt_type {
        OPTION_COUNTUP,
        OPTION_SET_INT,
        OPTION_SET_PTR,
+       OPTION_CMDMODE,
        /* options with arguments (usually) */
        OPTION_STRING,
        OPTION_INTEGER,
@@ -130,6 +131,8 @@ struct option {
 #define OPT_BOOL(s, l, v, h)        OPT_SET_INT(s, l, v, h, 1)
 #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, \
                                      (h), PARSE_OPT_NOARG, NULL, (p) }
+#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
+                                     (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) }
 #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) }
 #define OPT_STRING(s, l, v, a, h)   { OPTION_STRING,  (s), (l), (v), (a), (h) }
 #define OPT_STRING_LIST(s, l, v, a, h) \