lexer/parser: proper error messages

Error messages now look like this:

13.02.2010 19:42:30 - ERROR:
13.02.2010 19:42:30 - ERROR: CONFIG: syntax error, unexpected <word>,
expecting default/stacking/tabbed or stack-limit
13.02.2010 19:42:30 - ERROR: CONFIG: in file "inv", line 15:
13.02.2010 19:42:30 - ERROR: CONFIG:   new_container foobar
13.02.2010 19:42:30 - ERROR: CONFIG:                 ^^^^^^
13.02.2010 19:42:30 - ERROR:

diff --git a/include/config.h b/include/config.h
index fe5405bc260c27120ad70a2ca986a4fbe623b7f1..627fe525c43291e104b0857c14b3dd2712e60ceb 100644 (file)
--- a/include/config.h
+++ b/include/config.h
@@ -3,7 +3,7 @@
  *
  * i3 - an improved dynamic tiling window manager
  *
- * © 2009 Michael Stapelberg and contributors
+ * © 2009-2010 Michael Stapelberg and contributors
  *
  * See file LICENSE for license information.
  *
@@ -25,6 +25,21 @@ typedef struct Config Config;
 extern Config config;
 extern SLIST_HEAD(modes_head, Mode) modes;
 
+/**
+ * Used during the config file lexing/parsing to keep the state of the lexer
+ * in order to provide useful error messages in yyerror().
+ *
+ */
+struct context {
+        int line_number;
+        char *line_copy;
+        const char *filename;
+
+        /* These are the same as in YYLTYPE */
+        int first_column;
+        int last_column;
+};
+
 /**
  * Part of the struct Config. It makes sense to group colors for background,
  * border and text as every element in i3 has them (window decorations, bar).

diff --git a/src/cfgparse.l b/src/cfgparse.l
index ce912549568819b4eb38e919c8a3bc33539d25cd..1982452d790c29bb33579e5eabaaec99dceba7b4 100644 (file)
--- a/src/cfgparse.l
+++ b/src/cfgparse.l
@@ -1,5 +1,7 @@
 %option nounput
 %option noinput
+%option noyy_top_state
+%option stack
 
 %{
 /*
@@ -13,19 +15,57 @@
 
 #include "data.h"
 #include "config.h"
+#include "log.h"
+#include "util.h"
+
+int yycolumn = 1;
+
+#define YY_DECL int yylex (struct context *context)
+
+#define YY_USER_ACTION { \
+        context->first_column = yycolumn; \
+        context->last_column = yycolumn+yyleng-1; \
+        yycolumn += yyleng; \
+}
+
 %}
 
-%Start BIND_COND
-%Start BINDSYM_COND
-%Start BIND_AWS_COND
-%Start BINDSYM_AWS_COND
-%Start BIND_A2WS_COND
-%Start ASSIGN_COND
-%Start COLOR_COND
-%Start SCREEN_COND
-%Start SCREEN_AWS_COND
+EOL    (\r?\n)
+
+%s BIND_COND
+%s BINDSYM_COND
+%s BIND_AWS_COND
+%s BINDSYM_AWS_COND
+%s BIND_A2WS_COND
+%s ASSIGN_COND
+%s COLOR_COND
+%s SCREEN_COND
+%s SCREEN_AWS_COND
+%x BUFFER_LINE
 
 %%
+
+       {
+               /* This is called when a new line is lexed. We only want the
+                * first line to match to go into state BUFFER_LINE */
+               if (context->line_number == 0) {
+                       context->line_number = 1;
+                       BEGIN(INITIAL);
+                       yy_push_state(BUFFER_LINE);
+               }
+       }
+
+<BUFFER_LINE>^[^\r\n]*/{EOL}? {
+       /* save whole line */
+       context->line_copy = strdup(yytext);
+
+       yyless(0);
+       yy_pop_state();
+       yy_set_bol(true);
+       yycolumn = 1;
+}
+
+
 <BIND_A2WS_COND>[^\n]+          { BEGIN(INITIAL); yylval.string = strdup(yytext); return STR; }
 ^[ \t]*#[^\n]*                  { return TOKCOMMENT; }
 <COLOR_COND>[0-9a-fA-F]+        { yylval.string = strdup(yytext); return HEX; }
@@ -69,7 +109,11 @@ control                         { return TOKCONTROL; }
 ctrl                            { return TOKCONTROL; }
 shift                           { return TOKSHIFT; }
 →                               { return TOKARROW; }
-\n                              /* ignore end of line */;
+{EOL}                           {
+                                  FREE(context->line_copy);
+                                  context->line_number++;
+                                  yy_push_state(BUFFER_LINE);
+                                }
 <SCREEN_AWS_COND>x              { return (int)yytext[0]; }
 <BIND_COND>[ \t]+               { BEGIN(BIND_AWS_COND); return WHITESPACE; }
 <BINDSYM_COND>[ \t]+            { BEGIN(BINDSYM_AWS_COND); return WHITESPACE; }
@@ -91,4 +135,11 @@ shift                           { return TOKSHIFT; }
 <BINDSYM_AWS_COND>[a-zA-Z0-9_]+ { yylval.string = strdup(yytext); return WORD; }
 [a-zA-Z]+                       { yylval.string = strdup(yytext); return WORD; }
 .                               { return (int)yytext[0]; }
+
+<<EOF>> {
+        while (yy_start_stack_ptr > 0)
+                yy_pop_state();
+        yyterminate();
+}
+
 %%

diff --git a/src/cfgparse.y b/src/cfgparse.y
index 1ce7572476335bfa8ab0ecd6e8f7d32b6e186776..3879cf97421e3b82e2308205eaa5447bcd4652e4 100644 (file)
--- a/src/cfgparse.y
+++ b/src/cfgparse.y
@@ -24,17 +24,32 @@
 #include "log.h"
 
 typedef struct yy_buffer_state *YY_BUFFER_STATE;
-extern int yylex(void);
+extern int yylex(struct context *context);
 extern int yyparse(void);
 extern FILE *yyin;
 YY_BUFFER_STATE yy_scan_string(const char *);
 
 static struct bindings_head *current_bindings;
-
-int yydebug = 1;
-
-void yyerror(const char *str) {
-        fprintf(stderr,"error: %s\n",str);
+static struct context *context;
+
+/* We don’t need yydebug for now, as we got decent error messages using
+ * yyerror(). Should you ever want to extend the parser, it might be handy
+ * to just comment it in again, so it stays here. */
+//int yydebug = 1;
+
+void yyerror(const char *error_message) {
+        ELOG("\n");
+        ELOG("CONFIG: %s\n", error_message);
+        ELOG("CONFIG: in file \"%s\", line %d:\n",
+                context->filename, context->line_number);
+        ELOG("CONFIG:   %s\n", context->line_copy);
+        ELOG("CONFIG:   ");
+        for (int c = 1; c <= context->last_column; c++)
+                if (c >= context->first_column)
+                        printf("^");
+                else printf(" ");
+        printf("\n");
+        ELOG("\n");
 }
 
 int yywrap() {
@@ -149,11 +164,16 @@ void parse_file(const char *f) {
 
         yy_scan_string(new);
 
+        context = scalloc(sizeof(struct context));
+        context->filename = f;
+
         if (yyparse() != 0) {
                 fprintf(stderr, "Could not parse configfile\n");
                 exit(1);
         }
 
+        FREE(context->line_copy);
+        free(context);
         free(new);
         free(buf);
 }
@@ -162,6 +182,7 @@ void parse_file(const char *f) {
 
 %expect 1
 %error-verbose
+%lex-param { struct context *context }
 
 %union {
         int number;

diff --git a/src/mainx.c b/src/mainx.c
index 389fc76c05af3d083973e4262146641c9da53995..6dd72a179b03b8bcd76efff439b50cd7f82af4c3 100644 (file)
--- a/src/mainx.c
+++ b/src/mainx.c
@@ -150,6 +150,7 @@ int main(int argc, char *argv[], char *env[]) {
         int i, screens, opt;
         char *override_configpath = NULL;
         bool autostart = true;
+        bool only_check_config = false;
         xcb_connection_t *conn;
         xcb_property_handlers_t prophs;
         xcb_intern_atom_cookie_t atom_cookies[NUM_ATOMS];
@@ -170,7 +171,7 @@ int main(int argc, char *argv[], char *env[]) {
 
         start_argv = argv;
 
-        while ((opt = getopt_long(argc, argv, "c:vahld:V", long_options, &option_index)) != -1) {
+        while ((opt = getopt_long(argc, argv, "c:Cvahld:V", long_options, &option_index)) != -1) {
                 switch (opt) {
                         case 'a':
                                 LOG("Autostart disabled using -a\n");
@@ -179,6 +180,10 @@ int main(int argc, char *argv[], char *env[]) {
                         case 'c':
                                 override_configpath = sstrdup(optarg);
                                 break;
+                        case 'C':
+                                LOG("Checking configuration file only (-C)\n");
+                                only_check_config = true;
+                                break;
                         case 'v':
                                 printf("i3 version " I3_VERSION " © 2009 Michael Stapelberg and contributors\n");
                                 exit(EXIT_SUCCESS);
@@ -218,6 +223,10 @@ int main(int argc, char *argv[], char *env[]) {
                 die("Cannot open display\n");
 
         load_configuration(conn, override_configpath, false);
+        if (only_check_config) {
+                LOG("Done checking configuration file. Exiting.\n");
+                exit(0);
+        }
 
         /* Create the initial container on the first workspace. This used to
          * be part of init_table, but since it possibly requires an X