4e/7ef8ac83c4723e53af951fb8a87337f3d4fb96

   1 Return-Path: <markwalters1009@gmail.com>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id B1028431FB6\r
   6         for <notmuch@notmuchmail.org>; Tue,  5 Jun 2012 07:36:44 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0.201\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.201 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_ENVFROM_END_DIGIT=1, FREEMAIL_FROM=0.001,\r
  14         RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  15 Received: from olra.theworths.org ([127.0.0.1])\r
  16         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  17         with ESMTP id uV8m17Ibeigq for <notmuch@notmuchmail.org>;\r
  18         Tue,  5 Jun 2012 07:36:42 -0700 (PDT)\r
  19 Received: from mail-wg0-f45.google.com (mail-wg0-f45.google.com\r
  20  [74.125.82.45])        (using TLSv1 with cipher RC4-SHA (128/128 bits))        (No client\r
  21  certificate requested) by olra.theworths.org (Postfix) with ESMTPS id\r
  22  8C2CD431FAE    for <notmuch@notmuchmail.org>; Tue,  5 Jun 2012 07:36:42 -0700\r
  23  (PDT)\r
  24 Received: by wgbdt14 with SMTP id dt14so4367607wgb.2\r
  25         for <notmuch@notmuchmail.org>; Tue, 05 Jun 2012 07:36:39 -0700 (PDT)\r
  26 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
  27         h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references;\r
  28         bh=HWxzz8H+vobZV9ooepheMwydwZagFl56U1Ylacl7IMo=;\r
  29         b=NZw9goUOlmoSQD9T4S57D54wU5K5A4c2dtBiNfX9ZgB8iZ6IB0eof/CFXbSg/N6Bcd\r
  30         x93LF3eIfq3XkBkrwyOx/NrogiQDOfZymXhqYrrsSNustB93r4INq1J+Ln8Hz/g0zRN5\r
  31         655mMI/MT8WRbcNuXIcwC3acDbpaw6pnU7dcUcBn+P9BAVBAuyP9n3pHyZD9sJxG0x2E\r
  32         +HihLvZ0WWBbzGhcmUxceA4//YEI/p4YF3CuTgZQVeUKHLdEpkUj3hoEnkt4W+7L20j8\r
  33         YJzj4V8vJQkhKBdjxpdbeoP5268QAeRS7tk/YK7hglNi/pzBa0Cavaxd91T6IalzzDqg\r
  34         8mSQ==\r
  35 Received: by 10.216.206.164 with SMTP id l36mr14126494weo.154.1338906999766;\r
  36         Tue, 05 Jun 2012 07:36:39 -0700 (PDT)\r
  37 Received: from localhost (94-192-233-223.zone6.bethere.co.uk.\r
  38  [94.192.233.223])      by mx.google.com with ESMTPS id\r
  39  m20sm27402051wie.6.2012.06.05.07.36.37 (version=TLSv1/SSLv3 cipher=OTHER);\r
  40         Tue, 05 Jun 2012 07:36:38 -0700 (PDT)\r
  41 From: Mark Walters <markwalters1009@gmail.com>\r
  42 To: notmuch@notmuchmail.org\r
  43 Subject: [PATCH v2] cli: make the command line parser's errors more\r
  44         informative.\r
  45 Date: Tue,  5 Jun 2012 15:36:36 +0100\r
  46 Message-Id: <1338906996-18720-1-git-send-email-markwalters1009@gmail.com>\r
  47 X-Mailer: git-send-email 1.7.9.1\r
  48 In-Reply-To: <87oboxakus.fsf@qmul.ac.uk>\r
  49 References: <87oboxakus.fsf@qmul.ac.uk>\r
  50 X-BeenThere: notmuch@notmuchmail.org\r
  51 X-Mailman-Version: 2.1.13\r
  52 Precedence: list\r
  53 List-Id: "Use and development of the notmuch mail system."\r
  54         <notmuch.notmuchmail.org>\r
  55 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  56         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  57 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  58 List-Post: <mailto:notmuch@notmuchmail.org>\r
  59 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  60 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  61         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  62 X-List-Received-Date: Tue, 05 Jun 2012 14:36:44 -0000\r
  63 \r
  64 Previously, the cli parser was a little erratic in what errors it\r
  65 reported and would fail silently in many cases (for example, when no\r
  66 argument was passed to an integer option). This was particularly\r
  67 annoying as the user could not (easily) tell whether the command\r
  68 failed or just there were no search results.\r
  69 \r
  70 This patch tries to make the handling consistent and return a helpful\r
  71 error message in all cases.\r
  72 ---\r
  73  command-line-arguments.c |   76 ++++++++++++++++++++++++++++++----------------\r
  74  1 files changed, 50 insertions(+), 26 deletions(-)\r
  75 \r
  76 diff --git a/command-line-arguments.c b/command-line-arguments.c\r
  77 index b0a0dab..bf9aeca 100644\r
  78 --- a/command-line-arguments.c\r
  79 +++ b/command-line-arguments.c\r
  80 @@ -15,7 +15,7 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char\r
  81  \r
  82      const notmuch_keyword_t *keywords = arg_desc->keywords;\r
  83  \r
  84 -    if (next == 0) {\r
  85 +    if (next == '\0') {\r
  86         /* No keyword given */\r
  87         arg_str = "";\r
  88      }\r
  89 @@ -29,17 +29,17 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char\r
  90         }\r
  91         keywords++;\r
  92      }\r
  93 -    if (next != 0)\r
  94 -       fprintf (stderr, "unknown keyword: %s\n", arg_str);\r
  95 +    if (next != '\0')\r
  96 +       fprintf (stderr, "Unknown keyword argument \"%s\" for option \"%s\".\n", arg_str, arg_desc->name);\r
  97      else\r
  98 -       fprintf (stderr, "option %s needs a keyword\n", arg_desc->name);\r
  99 +       fprintf (stderr, "Option \"%s\" needs a keyword argument.\n", arg_desc->name);\r
 100      return FALSE;\r
 101  }\r
 102  \r
 103  static notmuch_bool_t\r
 104  _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {\r
 105  \r
 106 -    if (next == 0) {\r
 107 +    if (next == '\0') {\r
 108         *((notmuch_bool_t *)arg_desc->output_var) = TRUE;\r
 109         return TRUE;\r
 110      }\r
 111 @@ -51,9 +51,43 @@ _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const char\r
 112         *((notmuch_bool_t *)arg_desc->output_var) = TRUE;\r
 113         return TRUE;\r
 114      }\r
 115 +    fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", arg_str, arg_desc->name);\r
 116      return FALSE;\r
 117  }\r
 118  \r
 119 +static notmuch_bool_t\r
 120 +_process_int_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {\r
 121 +\r
 122 +    char *endptr;\r
 123 +    if (next == '\0' || arg_str[0] == '\0') {\r
 124 +       fprintf (stderr, "Option \"%s\" needs an integer argument.\n", arg_desc->name);\r
 125 +       return FALSE;\r
 126 +    }\r
 127 +\r
 128 +    *((int *)arg_desc->output_var) = strtol (arg_str, &endptr, 10);\r
 129 +    if (*endptr == '\0')\r
 130 +       return TRUE;\r
 131 +\r
 132 +    fprintf (stderr, "Unable to parse argument \"%s\" for option \"%s\" as an integer.\n",\r
 133 +            arg_str, arg_desc->name);\r
 134 +    return FALSE;\r
 135 +}\r
 136 +\r
 137 +static notmuch_bool_t\r
 138 +_process_string_arg (const notmuch_opt_desc_t *arg_desc, char next, const char *arg_str) {\r
 139 +\r
 140 +    if (next == '\0') {\r
 141 +       fprintf (stderr, "Option \"%s\" needs a string argument.\n", arg_desc->name);\r
 142 +       return FALSE;\r
 143 +    }\r
 144 +    if (arg_str[0] == '\0') {\r
 145 +       fprintf (stderr, "String argument for option \"%s\" must be non-empty.\n", arg_desc->name);\r
 146 +       return FALSE;\r
 147 +    }\r
 148 +    *((const char **)arg_desc->output_var) = arg_str;\r
 149 +    return TRUE;\r
 150 +}\r
 151 +\r
 152  /*\r
 153     Search for the {pos_arg_index}th position argument, return FALSE if\r
 154     that does not exist.\r
 155 @@ -93,26 +127,19 @@ parse_option (const char *arg,\r
 156  \r
 157      arg += 2;\r
 158  \r
 159 -    const notmuch_opt_desc_t *try = options;\r
 160 -    while (try->opt_type != NOTMUCH_OPT_END) {\r
 161 +    const notmuch_opt_desc_t *try;\r
 162 +    for (try = options; try->opt_type != NOTMUCH_OPT_END; try++) {\r
 163         if (try->name && strncmp (arg, try->name, strlen (try->name)) == 0) {\r
 164             char next = arg[strlen (try->name)];\r
 165             const char *value= arg+strlen(try->name)+1;\r
 166  \r
 167 -           char *endptr;\r
 168 -\r
 169 -           /* Everything but boolean arguments (switches) needs a\r
 170 -            * delimiter, and a non-zero length value. Boolean\r
 171 -            * arguments may take an optional =true or =false value.\r
 172 -            */\r
 173 -           if (next != '=' && next != ':' && next != 0) return FALSE;\r
 174 -           if (next == 0) {\r
 175 -               if (try->opt_type != NOTMUCH_OPT_BOOLEAN &&\r
 176 -                   try->opt_type != NOTMUCH_OPT_KEYWORD)\r
 177 -                   return FALSE;\r
 178 -           } else {\r
 179 -               if (value[0] == 0) return FALSE;\r
 180 -           }\r
 181 +           /* If we have not reached the end of the argument\r
 182 +              (i.e. the next character is not a space or delimiter)\r
 183 +              then the argument could still match a longer option\r
 184 +              name later in the option table.\r
 185 +           */\r
 186 +           if (next != '=' && next != ':' && next != '\0')\r
 187 +               continue;\r
 188  \r
 189             if (try->output_var == NULL)\r
 190                 INTERNAL_ERROR ("output pointer NULL for option %s", try->name);\r
 191 @@ -125,12 +152,10 @@ parse_option (const char *arg,\r
 192                 return _process_boolean_arg (try, next, value);\r
 193                 break;\r
 194             case NOTMUCH_OPT_INT:\r
 195 -               *((int *)try->output_var) = strtol (value, &endptr, 10);\r
 196 -               return (*endptr == 0);\r
 197 +               return _process_int_arg (try, next, value);\r
 198                 break;\r
 199             case NOTMUCH_OPT_STRING:\r
 200 -               *((const char **)try->output_var) = value;\r
 201 -               return TRUE;\r
 202 +               return _process_string_arg (try, next, value);\r
 203                 break;\r
 204             case NOTMUCH_OPT_POSITION:\r
 205             case NOTMUCH_OPT_END:\r
 206 @@ -139,7 +164,6 @@ parse_option (const char *arg,\r
 207                 /*UNREACHED*/\r
 208             }\r
 209         }\r
 210 -       try++;\r
 211      }\r
 212      fprintf (stderr, "Unrecognized option: --%s\n", arg);\r
 213      return FALSE;\r
 214 -- \r
 215 1.7.9.1\r
 216 \r