cli: some keyword options can be supplied with no argument
authorDaniel Kahn Gillmor <dkg@fifthhorseman.net>
Mon, 25 Dec 2017 18:42:26 +0000 (14:42 -0400)
committerDavid Bremner <david@tethera.net>
Fri, 29 Dec 2017 20:45:35 +0000 (16:45 -0400)
We might change some notmuch command line tools that used to be
booleans into keyword arguments.

In that case, there are some legacy tools that will expect to be able
to do "notmuch foo --bar" instead of "notmuch foo --bar=baz".

This patch makes it possible to support that older API, while
providing a warning and an encouragement to upgrade.

command-line-arguments.c
command-line-arguments.h
test/T410-argument-parsing.sh
test/arg-test.c

index a4a8bb4c87cd7119a80f5136b4fb75904bf1f956..d64aa85bbb3f2a390b072f6eeccbce3177630ccb 100644 (file)
@@ -4,13 +4,19 @@
 #include "error_util.h"
 #include "command-line-arguments.h"
 
+typedef enum {
+    OPT_FAILED, /* false */
+    OPT_OK, /* good */
+    OPT_GIVEBACK, /* pop one of the arguments you thought you were getting off the stack */
+} opt_handled;
+
 /*
   Search the array of keywords for a given argument, assigning the
   output variable to the corresponding value.  Return false if nothing
   matches.
 */
 
-static bool
+static opt_handled
 _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next,
                      const char *arg_str, bool negate)
 {
@@ -32,16 +38,32 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next,
        else
            *arg_desc->opt_keyword = keywords->value;
 
-       return true;
+       return OPT_OK;
+    }
+
+    if (arg_desc->opt_keyword && arg_desc->keyword_no_arg_value && next != ':' && next != '=') {
+       for (keywords = arg_desc->keywords; keywords->name; keywords++) {
+           if (strcmp (arg_desc->keyword_no_arg_value, keywords->name) != 0)
+               continue;
+
+           *arg_desc->opt_keyword = keywords->value;
+           fprintf (stderr, "Warning: No known keyword option given for \"%s\", choosing value \"%s\"."
+                    "  Please specify the argument explicitly!\n", arg_desc->name, arg_desc->keyword_no_arg_value);
+
+           return OPT_GIVEBACK;
+       }
+       fprintf (stderr, "No matching keyword for option \"%s\" and default value \"%s\" is invalid.\n", arg_str, arg_desc->name);
+       return OPT_FAILED;
     }
+
     if (next != '\0')
        fprintf (stderr, "Unknown keyword argument \"%s\" for option \"%s\".\n", arg_str, arg_desc->name);
     else
        fprintf (stderr, "Option \"%s\" needs a keyword argument.\n", arg_desc->name);
-    return false;
+    return OPT_FAILED;
 }
 
-static bool
+static opt_handled
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next,
                      const char *arg_str, bool negate)
 {
@@ -53,45 +75,45 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next,
        value = false;
     } else {
        fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name);
-       return false;
+       return OPT_FAILED;
     }
 
     *arg_desc->opt_bool = negate ? !value : value;
 
-    return true;
+    return OPT_OK;
 }
 
-static bool
+static opt_handled
 _process_int_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
     char *endptr;
     if (next == '\0' || arg_str[0] == '\0') {
        fprintf (stderr, "Option \"%s\" needs an integer argument.\n", arg_desc->name);
-       return false;
+       return OPT_FAILED;
     }
 
     *arg_desc->opt_int = strtol (arg_str, &endptr, 10);
     if (*endptr == '\0')
-       return true;
+       return OPT_OK;
 
     fprintf (stderr, "Unable to parse argument \"%s\" for option \"%s\" as an integer.\n",
             arg_str, arg_desc->name);
-    return false;
+    return OPT_FAILED;
 }
 
-static bool
+static opt_handled
 _process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {
 
     if (next == '\0') {
        fprintf (stderr, "Option \"%s\" needs a string argument.\n", arg_desc->name);
-       return false;
+       return OPT_FAILED;
     }
     if (arg_str[0] == '\0' && ! arg_desc->allow_empty) {
        fprintf (stderr, "String argument for option \"%s\" must be non-empty.\n", arg_desc->name);
-       return false;
+       return OPT_FAILED;
     }
     *arg_desc->opt_string = arg_str;
-    return true;
+    return OPT_OK;
 }
 
 /* Return number of non-NULL opt_* fields in opt_desc. */
@@ -212,13 +234,15 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
        if (next != '=' && next != ':' && next != '\0')
            continue;
 
-       if (next == '\0' && next_arg != NULL && ! try->opt_bool) {
+       bool lookahead = (next == '\0' && next_arg != NULL && ! try->opt_bool);
+
+       if (lookahead) {
            next = ' ';
            value = next_arg;
            opt_index ++;
        }
 
-       bool opt_status = false;
+       opt_handled opt_status = OPT_FAILED;
        if (try->opt_keyword || try->opt_flags)
            opt_status = _process_keyword_arg (try, next, value, negate);
        else if (try->opt_bool)
@@ -230,9 +254,12 @@ parse_option (int argc, char **argv, const notmuch_opt_desc_t *options, int opt_
        else
            INTERNAL_ERROR ("unknown or unhandled option \"%s\"", try->name);
 
-       if (! opt_status)
+       if (opt_status == OPT_FAILED)
            return -1;
 
+       if (lookahead && opt_status == OPT_GIVEBACK)
+           opt_index --;
+
        if (try->present)
            *try->present = true;
 
index c0228f7cb634dca5f34182a53e40d0803dc644c5..f722f97dde6e57c691437914b91cf42128384fa9 100644 (file)
@@ -26,6 +26,10 @@ typedef struct notmuch_opt_desc {
     const char **opt_string;
     const char **opt_position;
 
+    /* for opt_keyword only: if no matching arguments were found, and
+     * keyword_no_arg_value is set, then use keyword_no_arg_value instead. */
+    const char *keyword_no_arg_value;
+
     /* Must be set except for opt_inherit and opt_position. */
     const char *name;
 
index 192133c512261ce6b86b7d2ee4ad9a55b384de26..a384ce8603338f28e8acaf5086abef11179e8421 100755 (executable)
@@ -65,4 +65,36 @@ flags 1
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "test keyword arguments without value"
+$TEST_DIRECTORY/arg-test --boolkeyword bananas > OUTPUT
+cat <<EOF > EXPECTED
+boolkeyword 1
+positional arg 1 bananas
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "test keyword arguments with non-default value separted by a space"
+$TEST_DIRECTORY/arg-test --boolkeyword false bananas > OUTPUT
+cat <<EOF > EXPECTED
+boolkeyword 0
+positional arg 1 bananas
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "test keyword arguments without value at the end"
+$TEST_DIRECTORY/arg-test bananas --boolkeyword > OUTPUT
+cat <<EOF > EXPECTED
+boolkeyword 1
+positional arg 1 bananas
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "test keyword arguments without value but with = (should be an error)"
+$TEST_DIRECTORY/arg-test bananas --boolkeyword= > OUTPUT 2>&1
+cat <<EOF > EXPECTED
+Unknown keyword argument "" for option "boolkeyword".
+Unrecognized option: --boolkeyword=
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
index 7aff825507a53dca4f39fe4ffe99df344133fb88..a218f967e7ff01665b1575a63e44b2b1dae3ac41 100644 (file)
@@ -7,13 +7,14 @@ int main(int argc, char **argv){
     int opt_index=1;
 
     int kw_val=0;
+    int kwb_val=0;
     int fl_val=0;
     int int_val=0;
     const char *pos_arg1=NULL;
     const char *pos_arg2=NULL;
     const char *string_val=NULL;
     bool bool_val = false;
-    bool fl_set = false, int_set = false, bool_set = false,
+    bool fl_set = false, int_set = false, bool_set = false, kwb_set = false,
        kw_set = false, string_set = false, pos1_set = false, pos2_set = false;
 
     notmuch_opt_desc_t parent_options[] = {
@@ -33,6 +34,12 @@ int main(int argc, char **argv){
                                  { "one", 1 },
                                  { "two", 2 },
                                  { 0, 0 } } },
+       { .opt_keyword = &kwb_val, .name = "boolkeyword", .present = &kwb_set,
+         .keyword_no_arg_value = "true", .keywords =
+         (notmuch_keyword_t []){ { "false", 0 },
+                                 { "true", 1 },
+                                 { "auto", 2 },
+                                 { 0, 0 } } },
        { .opt_inherit = parent_options },
        { .opt_string = &string_val, .name = "string", .present = &string_set },
        { .opt_position = &pos_arg1, .present = &pos1_set },
@@ -51,6 +58,9 @@ int main(int argc, char **argv){
     if (kw_set)
        printf("keyword %d\n", kw_val);
 
+    if (kwb_set)
+       printf("boolkeyword %d\n", kwb_val);
+
     if (fl_set)
        printf("flags %d\n", fl_val);