config parser: recover after invalid input

This is done by ignoring the rest of the current line and jumping to the
nearest <error> token.

fixes #879

generate-command-parser.pl

@@ -193,7 +193,6 @@ say $callfh '        default:';
 say $callfh '            printf("BUG in the parser. state = %d\n", call_identifier);';
 say $callfh '            assert(false);';
 say $callfh '    }';
-say $callfh '    state = result->next_state;';
 say $callfh '}';
 close($callfh);
 

parser-specs/config.spec

@@ -15,6 +15,7 @@ state INITIAL:
   # We have an end token here for all the commands which just call some
   # function without using an explicit 'end' token.
   end ->
+  error ->
   '#'                                      -> IGNORE_LINE
   'set'                                    -> IGNORE_LINE
   bindtype = 'bindsym', 'bindcode', 'bind' -> BINDING
@@ -298,6 +299,7 @@ state MODEBRACE:
 
 state MODE:
   end ->
+  error ->
   '#' -> MODE_IGNORE_LINE
   'set' -> MODE_IGNORE_LINE
   bindtype = 'bindsym', 'bindcode', 'bind'
@@ -336,6 +338,7 @@ state BARBRACE:
 
 state BAR:
   end ->
+  error ->
   '#' -> BAR_IGNORE_LINE
   'set' -> BAR_IGNORE_LINE
   'i3bar_command'     -> BAR_BAR_COMMAND

src/commands_parser.c

@@ -190,6 +190,7 @@ static void next_state(const cmdp_token *token) {
         subcommand_output.json_gen = command_output.json_gen;
         subcommand_output.needs_tree_render = false;
         GENERATED_call(token->extra.call_identifier, &subcommand_output);
+        state = subcommand_output.next_state;
         /* If any subcommand requires a tree_render(), we need to make the
          * whole parser result request a tree_render(). */
         if (subcommand_output.needs_tree_render)

src/config_parser.c

@@ -21,6 +21,9 @@
  * 3. config_parser recognizes \n and \r as 'end' token, while commands_parser
  *    ignores them.
  *
+ * 4. config_parser skips the current line on invalid inputs and follows the
+ *    nearest <error> token.
+ *
  */
 #include <stdio.h>
 #include <stdlib.h>
@@ -221,23 +224,46 @@ static Match current_match;
 static struct ConfigResult subcommand_output;
 static struct ConfigResult command_output;
 
+/* A list which contains the states that lead to the current state, e.g.
+ * INITIAL, WORKSPACE_LAYOUT.
+ * When jumping back to INITIAL, statelist_idx will simply be set to 1
+ * (likewise for other states, e.g. MODE or BAR).
+ * This list is used to process the nearest error token. */
+static cmdp_state statelist[10] = { INITIAL };
+/* NB: statelist_idx points to where the next entry will be inserted */
+static int statelist_idx = 1;
+
 #include "GENERATED_config_call.h"
 
 
 static void next_state(const cmdp_token *token) {
+    cmdp_state _next_state = token->next_state;
+
 	//printf("token = name %s identifier %s\n", token->name, token->identifier);
 	//printf("next_state = %d\n", token->next_state);
     if (token->next_state == __CALL) {
         subcommand_output.json_gen = command_output.json_gen;
         GENERATED_call(token->extra.call_identifier, &subcommand_output);
+        _next_state = subcommand_output.next_state;
         clear_stack();
-        return;
     }
 
-    state = token->next_state;
+    state = _next_state;
     if (state == INITIAL) {
         clear_stack();
     }
+
+    /* See if we are jumping back to a state in which we were in previously
+     * (statelist contains INITIAL) and just move statelist_idx accordingly. */
+    for (int i = 0; i < statelist_idx; i++) {
+        if (statelist[i] != _next_state)
+            continue;
+        statelist_idx = i+1;
+        return;
+    }
+
+    /* Otherwise, the state is new and we add it to the list */
+    statelist[statelist_idx++] = _next_state;
 }
 
 /*
@@ -284,6 +310,7 @@ struct ConfigResult *parse_config(const char *input, struct context *context) {
         linecnt++;
     }
     state = INITIAL;
+    statelist_idx = 1;
 
 /* A YAJL JSON generator used for formatting replies. */
 #if YAJL_MAJOR >= 2
@@ -450,6 +477,10 @@ struct ConfigResult *parse_config(const char *input, struct context *context) {
                     tokenwalk += strlen(token->name + 1);
                     *tokenwalk++ = '\'';
                 } else {
+                    /* Skip error tokens in error messages, they are used
+                     * internally only and might confuse users. */
+                    if (strcmp(token->name, "error") == 0)
+                        continue;
                     /* Any other token is copied to the error message enclosed
                      * with angle brackets. */
                     *tokenwalk++ = '<';
@@ -531,10 +562,30 @@ struct ConfigResult *parse_config(const char *input, struct context *context) {
             ystr(position);
             y(map_close);
 
+            /* Skip the rest of this line, but continue parsing. */
+            while ((walk - input) <= len && *walk != '\n')
+                walk++;
+
             free(position);
             free(errormessage);
             clear_stack();
-            break;
+
+            /* To figure out in which state to go (e.g. MODE or INITIAL),
+             * we find the nearest state which contains an <error> token
+             * and follow that one. */
+            bool error_token_found = false;
+            for (int i = statelist_idx-1; (i >= 0) && !error_token_found; i--) {
+                cmdp_token_ptr *errptr = &(tokens[statelist[i]]);
+                for (int j = 0; j < errptr->n; j++) {
+                    if (strcmp(errptr->array[j].name, "error") != 0)
+                        continue;
+                    next_state(&(errptr->array[j]));
+                    error_token_found = true;
+                    break;
+                }
+            }
+
+            assert(error_token_found);
         }
     }
 

testcases/t/201-config-parser.t

@@ -362,6 +362,47 @@ is(parser_calls($config),
    $expected,
    'colors ok');
 
+################################################################################
+# Verify that errors don’t harm subsequent valid statements
+################################################################################
+
+$config = <<'EOT';
+hide_edge_border both
+client.focused          #4c7899 #285577 #ffffff #2e9ef4
+EOT
+
+$expected = <<'EOT';
+ERROR: CONFIG: Expected one of these tokens: <end>, '#', 'set', 'bindsym', 'bindcode', 'bind', 'bar', 'font', 'mode', 'floating_minimum_size', 'floating_maximum_size', 'floating_modifier', 'default_orientation', 'workspace_layout', 'new_window', 'new_float', 'hide_edge_borders', 'for_window', 'assign', 'focus_follows_mouse', 'force_focus_wrapping', 'force_xinerama', 'force-xinerama', 'workspace_auto_back_and_forth', 'fake_outputs', 'fake-outputs', 'force_display_urgency_hint', 'workspace', 'ipc_socket', 'ipc-socket', 'restart_state', 'popup_during_fullscreen', 'exec_always', 'exec', 'client.background', 'client.focused_inactive', 'client.focused', 'client.unfocused', 'client.urgent'
+ERROR: CONFIG: (in file <stdin>)
+ERROR: CONFIG: Line   1: hide_edge_border both
+ERROR: CONFIG:           ^^^^^^^^^^^^^^^^^^^^^
+ERROR: CONFIG: Line   2: client.focused          #4c7899 #285577 #ffffff #2e9ef4
+cfg_color(client.focused, #4c7899, #285577, #ffffff, #2e9ef4)
+EOT
+
+is(parser_calls($config),
+   $expected,
+   'errors dont harm subsequent statements');
+
+$config = <<'EOT';
+hide_edge_borders FOOBAR
+client.focused          #4c7899 #285577 #ffffff #2e9ef4
+EOT
+
+$expected = <<'EOT';
+ERROR: CONFIG: Expected one of these tokens: 'none', 'vertical', 'horizontal', 'both', '1', 'yes', 'true', 'on', 'enable', 'active'
+ERROR: CONFIG: (in file <stdin>)
+ERROR: CONFIG: Line   1: hide_edge_borders FOOBAR
+ERROR: CONFIG:                             ^^^^^^
+ERROR: CONFIG: Line   2: client.focused          #4c7899 #285577 #ffffff #2e9ef4
+cfg_color(client.focused, #4c7899, #285577, #ffffff, #2e9ef4)
+EOT
+
+is(parser_calls($config),
+   $expected,
+   'errors dont harm subsequent statements');
+
+
 ################################################################################
 # Error message with 2+2 lines of context
 ################################################################################
@@ -524,6 +565,7 @@ ERROR: CONFIG: Line   2:     output LVDS-1
 ERROR: CONFIG: Line   3:     unknown qux
 ERROR: CONFIG:               ^^^^^^^^^^^
 ERROR: CONFIG: Line   4: }
+cfg_bar_finish()
 EOT
 
 is(parser_calls($config),