From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id DDDD86DE0A6C for ; Mon, 25 Dec 2017 10:42:43 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[AWL=0.011, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i_7PKxn2_3vf for ; Mon, 25 Dec 2017 10:42:40 -0800 (PST) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id DAFF96DE0946 for ; Mon, 25 Dec 2017 10:42:39 -0800 (PST) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1eTXhs-0006My-2c; Mon, 25 Dec 2017 13:42:36 -0500 Received: (nullmailer pid 6164 invoked by uid 1000); Mon, 25 Dec 2017 18:42:34 -0000 From: David Bremner To: David Bremner , Daniel Kahn Gillmor , Notmuch Mail Subject: [PATCH] cli: some keyword options can be supplied with no argument Date: Mon, 25 Dec 2017 14:42:26 -0400 Message-Id: <20171225184226.6119-1-david@tethera.net> X-Mailer: git-send-email 2.15.1 In-Reply-To: <87a7y9a56d.fsf@tethera.net> References: <87a7y9a56d.fsf@tethera.net> X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Dec 2017 18:42:44 -0000 From: Daniel Kahn Gillmor 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 | 61 +++++++++++++++++++++++++++++++------------ command-line-arguments.h | 4 +++ test/T410-argument-parsing.sh | 32 +++++++++++++++++++++++ test/arg-test.c | 12 ++++++++- 4 files changed, 91 insertions(+), 18 deletions(-) diff --git a/command-line-arguments.c b/command-line-arguments.c index a4a8bb4c..d64aa85b 100644 --- a/command-line-arguments.c +++ b/command-line-arguments.c @@ -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; diff --git a/command-line-arguments.h b/command-line-arguments.h index c0228f7c..f722f97d 100644 --- a/command-line-arguments.h +++ b/command-line-arguments.h @@ -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; diff --git a/test/T410-argument-parsing.sh b/test/T410-argument-parsing.sh index 192133c5..a384ce86 100755 --- a/test/T410-argument-parsing.sh +++ b/test/T410-argument-parsing.sh @@ -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 < 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 < 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 < 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 < EXPECTED +Unknown keyword argument "" for option "boolkeyword". +Unrecognized option: --boolkeyword= +EOF +test_expect_equal_file EXPECTED OUTPUT + test_done diff --git a/test/arg-test.c b/test/arg-test.c index 7aff8255..a218f967 100644 --- a/test/arg-test.c +++ b/test/arg-test.c @@ -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); -- 2.15.1