cli: make the command line parser's errors more informative.

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.

diff --git a/command-line-arguments.c b/command-line-arguments.c
index b0a0dab098bc7bec9c6d25901eb77ea71c549028..bf9aecabe86923e7dc3f560bd61f5e7119c5289b 100644 (file)
--- 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;
 
 
     const notmuch_keyword_t *keywords = arg_desc->keywords;
 

-    if (next == 0) {
+    if (next == '\0') {

        /* No keyword given */
        arg_str = "";
     }
        /* No keyword given */
        arg_str = "";
     }


@@ -29,17 +29,17 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, char next, const char
        }
        keywords++;
     }
        }
        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
     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) {
 
     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;
     }
        *((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;
     }
        *((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;
 }
 
     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.
 /*
    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;
 
 
     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;
 
        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);
 
            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:
                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:
                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:
                break;
            case NOTMUCH_OPT_POSITION:
            case NOTMUCH_OPT_END:


@@ -139,7 +164,6 @@ parse_option (const char *arg,
                /*UNREACHED*/
            }
        }
                /*UNREACHED*/
            }
        }

-       try++;

     }
     fprintf (stderr, "Unrecognized option: --%s\n", arg);
     return FALSE;
     }
     fprintf (stderr, "Unrecognized option: --%s\n", arg);
     return FALSE;