From cd497ee979ccc8a0496b51ff35d62d8c719c73d2 Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Tue, 5 Jun 2012 15:36:36 +0100 Subject: [PATCH] [PATCH v2] cli: make the command line parser's errors more informative. --- 4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96 | 216 ++++++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96 diff --git a/4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96 b/4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96 new file mode 100644 index 000000000..75eb63c2f --- /dev/null +++ b/4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96 @@ -0,0 +1,216 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id B1028431FB6 + for ; Tue, 5 Jun 2012 07:36:44 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0.201 +X-Spam-Level: +X-Spam-Status: No, score=0.201 tagged_above=-999 required=5 + tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, + FREEMAIL_ENVFROM_END_DIGIT=1, FREEMAIL_FROM=0.001, + RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id uV8m17Ibeigq for ; + Tue, 5 Jun 2012 07:36:42 -0700 (PDT) +Received: from mail-wg0-f45.google.com (mail-wg0-f45.google.com + [74.125.82.45]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client + certificate requested) by olra.theworths.org (Postfix) with ESMTPS id + 8C2CD431FAE for ; Tue, 5 Jun 2012 07:36:42 -0700 + (PDT) +Received: by wgbdt14 with SMTP id dt14so4367607wgb.2 + for ; Tue, 05 Jun 2012 07:36:39 -0700 (PDT) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; + h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; + bh=HWxzz8H+vobZV9ooepheMwydwZagFl56U1Ylacl7IMo=; + b=NZw9goUOlmoSQD9T4S57D54wU5K5A4c2dtBiNfX9ZgB8iZ6IB0eof/CFXbSg/N6Bcd + x93LF3eIfq3XkBkrwyOx/NrogiQDOfZymXhqYrrsSNustB93r4INq1J+Ln8Hz/g0zRN5 + 655mMI/MT8WRbcNuXIcwC3acDbpaw6pnU7dcUcBn+P9BAVBAuyP9n3pHyZD9sJxG0x2E + +HihLvZ0WWBbzGhcmUxceA4//YEI/p4YF3CuTgZQVeUKHLdEpkUj3hoEnkt4W+7L20j8 + YJzj4V8vJQkhKBdjxpdbeoP5268QAeRS7tk/YK7hglNi/pzBa0Cavaxd91T6IalzzDqg + 8mSQ== +Received: by 10.216.206.164 with SMTP id l36mr14126494weo.154.1338906999766; + Tue, 05 Jun 2012 07:36:39 -0700 (PDT) +Received: from localhost (94-192-233-223.zone6.bethere.co.uk. + [94.192.233.223]) by mx.google.com with ESMTPS id + m20sm27402051wie.6.2012.06.05.07.36.37 (version=TLSv1/SSLv3 cipher=OTHER); + Tue, 05 Jun 2012 07:36:38 -0700 (PDT) +From: Mark Walters +To: notmuch@notmuchmail.org +Subject: [PATCH v2] cli: make the command line parser's errors more + informative. +Date: Tue, 5 Jun 2012 15:36:36 +0100 +Message-Id: <1338906996-18720-1-git-send-email-markwalters1009@gmail.com> +X-Mailer: git-send-email 1.7.9.1 +In-Reply-To: <87oboxakus.fsf@qmul.ac.uk> +References: <87oboxakus.fsf@qmul.ac.uk> +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +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: Tue, 05 Jun 2012 14:36:44 -0000 + +Previously, the cli parser was a little erratic in what errors it +reported and would fail silently in many cases (for example, when no +argument was passed to an integer option). This was particularly +annoying as the user could not (easily) tell whether the command +failed or just there were no search results. + +This patch tries to make the handling consistent and return a helpful +error message in all cases. +--- + command-line-arguments.c | 76 ++++++++++++++++++++++++++++++---------------- + 1 files changed, 50 insertions(+), 26 deletions(-) + +diff --git a/command-line-arguments.c b/command-line-arguments.c +index b0a0dab..bf9aeca 100644 +--- a/command-line-arguments.c ++++ b/command-line-arguments.c +@@ -15,7 +15,7 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char + + const notmuch_keyword_t *keywords = arg_desc->keywords; + +- if (next == 0) { ++ if (next == '\0') { + /* No keyword given */ + arg_str = ""; + } +@@ -29,17 +29,17 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char + } + keywords++; + } +- if (next != 0) +- fprintf (stderr, "unknown keyword: %s\n", arg_str); ++ 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\n", arg_desc->name); ++ fprintf (stderr, "Option \"%s\" needs a keyword argument.\n", arg_desc->name); + return FALSE; + } + + static notmuch_bool_t + _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) { + +- if (next == 0) { ++ if (next == '\0') { + *((notmuch_bool_t *)arg_desc->output_var) = TRUE; + return TRUE; + } +@@ -51,9 +51,43 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char + *((notmuch_bool_t *)arg_desc->output_var) = TRUE; + return TRUE; + } ++ fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name); + return FALSE; + } + ++static notmuch_bool_t ++_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; ++ } ++ ++ *((int *)arg_desc->output_var) = strtol (arg_str, &endptr, 10); ++ if (*endptr == '\0') ++ return TRUE; ++ ++ fprintf (stderr, "Unable to parse argument \"%s\" for option \"%s\" as an integer.\n", ++ arg_str, arg_desc->name); ++ return FALSE; ++} ++ ++static notmuch_bool_t ++_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; ++ } ++ if (arg_str[0] == '\0') { ++ fprintf (stderr, "String argument for option \"%s\" must be non-empty.\n", arg_desc->name); ++ return FALSE; ++ } ++ *((const char **)arg_desc->output_var) = arg_str; ++ return TRUE; ++} ++ + /* + Search for the {pos_arg_index}th position argument, return FALSE if + that does not exist. +@@ -93,26 +127,19 @@ parse_option (const char *arg, + + arg += 2; + +- const notmuch_opt_desc_t *try = options; +- while (try->opt_type != NOTMUCH_OPT_END) { ++ const notmuch_opt_desc_t *try; ++ for (try = options; try->opt_type != NOTMUCH_OPT_END; try++) { + if (try->name && strncmp (arg, try->name, strlen (try->name)) == 0) { + char next = arg[strlen (try->name)]; + const char *value= arg+strlen(try->name)+1; + +- char *endptr; +- +- /* Everything but boolean arguments (switches) needs a +- * delimiter, and a non-zero length value. Boolean +- * arguments may take an optional =true or =false value. +- */ +- if (next != '=' && next != ':' && next != 0) return FALSE; +- if (next == 0) { +- if (try->opt_type != NOTMUCH_OPT_BOOLEAN && +- try->opt_type != NOTMUCH_OPT_KEYWORD) +- return FALSE; +- } else { +- if (value[0] == 0) return FALSE; +- } ++ /* If we have not reached the end of the argument ++ (i.e. the next character is not a space or delimiter) ++ then the argument could still match a longer option ++ name later in the option table. ++ */ ++ if (next != '=' && next != ':' && next != '\0') ++ continue; + + if (try->output_var == NULL) + INTERNAL_ERROR ("output pointer NULL for option %s", try->name); +@@ -125,12 +152,10 @@ parse_option (const char *arg, + return _process_boolean_arg (try, next, value); + break; + case NOTMUCH_OPT_INT: +- *((int *)try->output_var) = strtol (value, &endptr, 10); +- return (*endptr == 0); ++ return _process_int_arg (try, next, value); + break; + case NOTMUCH_OPT_STRING: +- *((const char **)try->output_var) = value; +- return TRUE; ++ return _process_string_arg (try, next, value); + break; + case NOTMUCH_OPT_POSITION: + case NOTMUCH_OPT_END: +@@ -139,7 +164,6 @@ parse_option (const char *arg, + /*UNREACHED*/ + } + } +- try++; + } + fprintf (stderr, "Unrecognized option: --%s\n", arg); + return FALSE; +-- +1.7.9.1 + -- 2.26.2