Improve config file error reporting

Show the original line from the config file, followed by the line number and
a specific error message. Also show all errors from the config file before
waiting for key press.

doc/fixes36.1

@@ -420,6 +420,7 @@ surviving a gas spore's explosion would leave that explosion as a stale reason
 add database entry for "martial arts"
 starting inventory for rogues specified +9 lock pick, where +/-N is meaningless
 fix pile mark when picking up all-but-one items while invisible
+improve config file error reporting
 
 
 Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository

include/extern.h

@@ -198,7 +198,7 @@ E int NDECL(enter_explore_mode);
 E void FDECL(enlightenment, (int, int));
 E void FDECL(youhiding, (BOOLEAN_P, int));
 E void FDECL(show_conduct, (int));
-E void FDECL(bind_key, (UCHAR_P, const char *));
+E boolean FDECL(bind_key, (UCHAR_P, const char *));
 E void NDECL(dokeylist);
 E int FDECL(xytod, (SCHAR_P, SCHAR_P));
 E void FDECL(dtoxy, (coord *, int));
@@ -767,6 +767,9 @@ E void FDECL(unlock_file, (const char *));
 #ifdef USER_SOUNDS
 E boolean FDECL(can_read_file, (const char *));
 #endif
+E void FDECL(config_error_init, (BOOLEAN_P, const char *));
+E void FDECL(config_error_add, (const char *, ...)) PRINTF_F(1, 2);
+E int NDECL(config_error_done);
 E boolean FDECL(read_config_file, (const char *, int));
 E void FDECL(check_recordfile, (const char *));
 E void NDECL(read_wizkit);
@@ -1679,7 +1682,7 @@ E boolean FDECL(match_optname, (const char *, const char *, int, BOOLEAN_P));
 E void NDECL(initoptions);
 E void NDECL(initoptions_init);
 E void NDECL(initoptions_finish);
-E void FDECL(parseoptions, (char *, BOOLEAN_P, BOOLEAN_P));
+E boolean FDECL(parseoptions, (char *, BOOLEAN_P, BOOLEAN_P));
 E int NDECL(doset);
 E int NDECL(dotogglepickup);
 E void NDECL(option_help);
@@ -1687,7 +1690,7 @@ E void FDECL(next_opt, (winid, const char *));
 E int FDECL(fruitadd, (char *, struct fruit *));
 E int FDECL(choose_classes_menu, (const char *, int, BOOLEAN_P,
                                   char *, char *));
-E void FDECL(parsebindings, (char *));
+E boolean FDECL(parsebindings, (char *));
 E void FDECL(add_menu_cmd_alias, (CHAR_P, CHAR_P));
 E char FDECL(get_menu_cmd_key, (CHAR_P));
 E char FDECL(map_menu_cmd, (CHAR_P));

src/cmd.c

@@ -3056,7 +3056,7 @@ uchar key;
     return (char *) 0;
 }
 
-void
+boolean
 bind_key(key, command)
 uchar key;
 const char *command;
@@ -3066,19 +3066,17 @@ const char *command;
     /* special case: "nothing" is reserved for unbinding */
     if (!strcmp(command, "nothing")) {
         Cmd.commands[key] = (struct ext_func_tab *) 0;
-        return;
+        return TRUE;
     }
 
     for (extcmd = extcmdlist; extcmd->ef_txt; extcmd++) {
         if (strcmp(command, extcmd->ef_txt))
             continue;
         Cmd.commands[key] = extcmd;
-        return;
+        return TRUE;
     }
 
-    pline(
+    return FALSE;
-        "Bad command %s matched with key %c (ASCII %i).  Ignoring command.\n",
-          command, key, key);
 }
 
 /* initialize all keyboard commands */
@@ -3091,27 +3089,27 @@ commands_init()
         if (extcmd->key)
             Cmd.commands[extcmd->key] = extcmd;
 
-    bind_key(C('l'), "redraw"); /* if number_pad is set */
+    (void) bind_key(C('l'), "redraw"); /* if number_pad is set */
     /*       'b', 'B' : go sw */
     /*       'F' : fight (one time) */
     /*       'g', 'G' : multiple go */
     /*       'h', 'H' : go west */
-    bind_key('h',    "help"); /* if number_pad is set */
+    (void) bind_key('h',    "help"); /* if number_pad is set */
-    bind_key('j',    "jump"); /* if number_pad is on */
+    (void) bind_key('j',    "jump"); /* if number_pad is on */
     /*       'j', 'J', 'k', 'K', 'l', 'L', 'm', 'M', 'n', 'N' move commands */
-    bind_key('k',    "kick"); /* if number_pad is on */
+    (void) bind_key('k',    "kick"); /* if number_pad is on */
-    bind_key('l',    "loot"); /* if number_pad is on */
+    (void) bind_key('l',    "loot"); /* if number_pad is on */
-    bind_key(C('n'), "annotate"); /* if number_pad is on */
+    (void) bind_key(C('n'), "annotate"); /* if number_pad is on */
-    bind_key(M('n'), "name");
+    (void) bind_key(M('n'), "name");
-    bind_key(M('N'), "name");
+    (void) bind_key(M('N'), "name");
-    bind_key('u',    "untrap"); /* if number_pad is on */
+    (void) bind_key('u',    "untrap"); /* if number_pad is on */
 
     /* alt keys: */
-    bind_key(M('O'), "overview");
+    (void) bind_key(M('O'), "overview");
-    bind_key(M('2'), "twoweapon");
+    (void) bind_key(M('2'), "twoweapon");
 
     /* wait_on_space */
-    bind_key(' ',    "wait");
+    (void) bind_key(' ',    "wait");
 }
 
 int
@@ -4007,7 +4005,7 @@ boolean initial;
         }
         backed_dir_cmd = TRUE;
         for (i = 0; i < 8; i++)
-            bind_key(Cmd.dirchars[i], "nothing");
+            (void) bind_key(Cmd.dirchars[i], "nothing");
     }
 }
 

src/files.c

@@ -2,6 +2,8 @@
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /* NetHack may be freely redistributed.  See license for details. */
 
+#define NEED_VARARGS
+
 #include "hack.h"
 #include "dlb.h"
 
@@ -198,6 +200,7 @@ STATIC_DCL void FDECL(set_symhandling, (char *, int));
 #ifdef NOCWD_ASSUMPTIONS
 STATIC_DCL void FDECL(adjust_prefix, (char *, int));
 #endif
+STATIC_DCL void FDECL(config_error_nextline, (const char *));
 STATIC_DCL void NDECL(free_config_sections);
 STATIC_DCL char *FDECL(choose_random_part, (char *, CHAR_P));
 STATIC_DCL boolean FDECL(is_config_section, (const char *));
@@ -2240,6 +2243,7 @@ int src;
     char *bufp, *altp, buf[4 * BUFSZ];
     uchar translate[MAXPCHARS];
     int len;
+    int retval = 1;
 
     /* convert any tab to space, condense consecutive spaces into one,
        remove leading and trailing spaces (exception: if there is nothing
@@ -2257,8 +2261,10 @@ int src;
     altp = index(buf, ':');
     if (!bufp || (altp && altp < bufp))
         bufp = altp;
-    if (!bufp)
+    if (!bufp) {
+        config_error_add("Not a config statement, missing '='");
         return 0;
+    }
     /* skip past '=', then space between it and value, if any */
     ++bufp;
     if (*bufp == ' ')
@@ -2278,7 +2284,8 @@ int src;
             bufp = altp;
         ++bufp; /* skip '='; parseoptions() handles spaces */
 
-        parseoptions(bufp, TRUE, TRUE);
+        if (!parseoptions(bufp, TRUE, TRUE))
+            retval = 0;
     } else if (match_varname(buf, "CHOOSE", 6)) {
         char *section;
         if (config_section_chosen)
@@ -2289,11 +2296,13 @@ int src;
     } else if (match_varname(buf, "AUTOPICKUP_EXCEPTION", 5)) {
         add_autopickup_exception(bufp);
     } else if (match_varname(buf, "BINDINGS", 4)) {
-        parsebindings(bufp);
+        if (!parsebindings(bufp))
+            retval = 0;
     } else if (match_varname(buf, "AUTOCOMPLETE", 5)) {
         parseautocomplete(bufp, TRUE);
     } else if (match_varname(buf, "MSGTYPE", 7)) {
-        (void) msgtype_parse_add(bufp);
+        if (!msgtype_parse_add(bufp))
+            retval = 0;
 #ifdef NOCWD_ASSUMPTIONS
     } else if (match_varname(buf, "HACKDIR", 4)) {
         adjust_prefix(bufp, HACKPREFIX);
@@ -2539,7 +2548,8 @@ int src;
         (void) get_uchars(fp, buf, bufp, &iflags.bouldersym, TRUE, 1,
                           "BOULDER");
     } else if (match_varname(buf, "MENUCOLOR", 9)) {
-        (void) add_menu_coloring(bufp);
+        if (!add_menu_coloring(bufp))
+            retval = 0;
     } else if (match_varname(buf, "WARNINGS", 5)) {
         (void) get_uchars(fp, buf, bufp, translate, FALSE, WARNCOUNT,
                           "WARNINGS");
@@ -2719,7 +2729,7 @@ int src;
 #endif
     } else
         return 0;
-    return 1;
+    return retval;
 }
 
 #ifdef USER_SOUNDS
@@ -2731,6 +2741,81 @@ const char *filename;
 }
 #endif /* USER_SOUNDS */
 
+static int config_err_line_num = 0;
+static int config_err_num_errors = 0;
+static boolean config_err_origline_shown = FALSE;
+static boolean config_err_fromfile = FALSE;
+static char config_err_origline[4 * BUFSZ];
+static char config_err_source[BUFSZ];
+
+void
+config_error_init(from_file, sourcename)
+boolean from_file;
+const char *sourcename;
+{
+    config_err_line_num = 0;
+    config_err_num_errors = 0;
+    config_err_origline_shown = FALSE;
+    config_err_fromfile = from_file;
+    config_err_origline[0] = '\0';
+    if (sourcename && sourcename[0])
+        Strcpy(config_err_source, sourcename);
+    else
+        config_err_source[0] = '\0';
+}
+
+STATIC_OVL void
+config_error_nextline(line)
+const char *line;
+{
+    config_err_line_num++;
+    config_err_origline_shown = FALSE;
+    if (line && line[0])
+        Strcpy(config_err_origline, line);
+    else
+        config_err_origline[0] = '\0';
+}
+
+/*VARARGS1*/
+void config_error_add
+VA_DECL(const char *, str)
+/*const char *errmsg;*/
+{
+    VA_START(str);
+    VA_INIT(str, char *);
+    char buf[BUFSZ];
+    char lineno[QBUFSZ];
+
+    Vsprintf(buf, str, VA_ARGS);
+
+    config_err_num_errors++;
+    if (!config_err_origline_shown) {
+        pline("\n%s", config_err_origline);
+        config_err_origline_shown = TRUE;
+    }
+    if (config_err_line_num > 0) {
+        Sprintf(lineno, "Line %i: ", config_err_line_num);
+    } else
+        lineno[0] = '\0';
+    pline(" * %s%s.", lineno, (buf && buf[0]) ? buf : "Unknown error");
+
+    VA_END();
+}
+
+int
+config_error_done()
+{
+    int n = config_err_num_errors;
+    if (n) {
+        pline("\n%i error%s in %s.\n", n,
+                   (n > 1) ? "s" : "",
+                   *config_err_source ? config_err_source : configfile);
+        wait_synch();
+    }
+    config_error_init(FALSE, "");
+    return n;
+}
+
 boolean
 read_config_file(filename, src)
 const char *filename;
@@ -2748,6 +2833,8 @@ int src;
     free_config_sections();
 
     while (fgets(buf, sizeof buf, fp)) {
+        strip_newline(buf);
+        config_error_nextline(buf);
 #ifdef notyet
 /*
 XXX Don't call read() in parse_config_line, read as callback or reassemble
@@ -2755,17 +2842,8 @@ line at this level.
 OR: Forbid multiline stuff for alternate config sources.
 */
 #endif
-        if (!parse_config_line(fp, strip_newline(buf), src)) {
+        if (!parse_config_line(fp, buf, src))
-            static const char badoptionline[] = "Bad option line: \"%s\"";
-
-            /* truncate buffer if it's long; this is actually conservative */
-            if (strlen(buf) > BUFSZ - sizeof badoptionline)
-                buf[BUFSZ - sizeof badoptionline] = '\0';
-
-            raw_printf(badoptionline, buf);
-            wait_synch();
             rv = FALSE;
-        }
     }
     (void) fclose(fp);