[PATCH] cli: make the command line parser's errors more informative.
Peter Wang
novalazy at gmail.com
Tue Jun 5 01:40:37 PDT 2012
On Sun, 3 Jun 2012 12:48:48 +0100, Mark Walters <markwalters1009 at gmail.com> wrote:
>
> +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;
It's usually clearer to write '\0' for the null character.
> @@ -99,20 +133,13 @@ parse_option (const char *arg,
> 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 this is not the end of the argument (i.e. the next
> + * character is not a space or a delimiter) we stop
> + * parsing for this option but allow the parsing to
> + * continue to for other options. This should allow
> + * options to be initial segments of other options. */
It took me a little while to figure out what the last sentence was
about. Perhaps:
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.
(otherwise, "continue to for other")
> + if (next != '=' && next != ':' && next != 0)
> + goto DONE_THIS_OPTION;
The `goto' could be expressed as a `continue' in a `for' loop, AFAICS.
>
> 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,6 +164,7 @@ parse_option (const char *arg,
> /*UNREACHED*/
> }
> }
> + DONE_THIS_OPTION:
> try++;
> }
> fprintf (stderr, "Unrecognized option: --%s\n", arg);
Peter
More information about the notmuch
mailing list