]> git.sur5r.net Git - i3/i3/commitdiff
Display i3-nagbar when commands lead to an error
authorMichael Stapelberg <michael@stapelberg.de>
Thu, 2 Aug 2012 15:43:00 +0000 (17:43 +0200)
committerMichael Stapelberg <michael@stapelberg.de>
Thu, 2 Aug 2012 15:45:09 +0000 (17:45 +0200)
e.g. pressing Mod1+x when having the following in your configfile:

    bindsym Mod1+x some invalid command

will lead to an i3-nagbar instance popping up, offering you to view the
error log (which will contain parser errors from this commit on).

generate-command-parser.pl
include/all.h
include/key_press.h [new file with mode: 0644]
src/commands.c
src/commands_parser.c
src/handlers.c
src/key_press.c [new file with mode: 0644]
src/util.c
testcases/t/187-commands-parser.t

index 993c64fcaa570dcc73d229ef4bc6130223078369..01cbe462ec64d05c7a51c2d80da8e7b710c5820c 100755 (executable)
@@ -152,7 +152,7 @@ for my $state (@keys) {
         $cmd =~ s/[^(]+\(//;
         $cmd =~ s/\)$//;
         $cmd = ", $cmd" if length($cmd) > 0;
-        say $callfh qq|           printf("$fmt\\n"$cmd);|;
+        say $callfh qq|           fprintf(stderr, "$fmt\\n"$cmd);|;
         say $callfh '#endif';
         say $callfh "             state = $next_state;";
         say $callfh "             break;";
index 11eaaba49d7f5a48dda5ca4729e6b3ccb4f5a375..c6b0351f87d9c39e9f2024af3083e632c95a0b5f 100644 (file)
@@ -54,6 +54,7 @@
 #include "i3.h"
 #include "x.h"
 #include "click.h"
+#include "key_press.h"
 #include "floating.h"
 #include "config.h"
 #include "handlers.h"
diff --git a/include/key_press.h b/include/key_press.h
new file mode 100644 (file)
index 0000000..4d469ba
--- /dev/null
@@ -0,0 +1,32 @@
+/*
+ * vim:ts=4:sw=4:expandtab
+ *
+ * i3 - an improved dynamic tiling window manager
+ * © 2009-2012 Michael Stapelberg and contributors (see also: LICENSE)
+ *
+ * key_press.c: key press handler
+ *
+ */
+#ifndef _KEY_PRESS_H
+#define _KEY_PRESS_H
+
+/**
+ * There was a key press. We compare this key code with our bindings table and pass
+ * the bound action to parse_command().
+ *
+ */
+void handle_key_press(xcb_key_press_event_t *event);
+
+/**
+ * Kills the commanderror i3-nagbar process, if any.
+ *
+ * Called when reloading/restarting, since the user probably fixed his wrong
+ * keybindings.
+ *
+ * If wait_for_it is set (restarting), this function will waitpid(), otherwise,
+ * ev is assumed to handle it (reloading).
+ *
+ */
+void kill_commanderror_nagbar(bool wait_for_it);
+
+#endif
index 7b6d55a7aac7a4ef42492b42db5fd99a60df6dea..c0681d26642d0feb2f72cf4d5f6245ad6114ac8d 100644 (file)
@@ -1443,6 +1443,7 @@ void cmd_exit(I3_CMD) {
 void cmd_reload(I3_CMD) {
     LOG("reloading\n");
     kill_configerror_nagbar(false);
+    kill_commanderror_nagbar(false);
     load_configuration(conn, NULL, true);
     x_set_i3_atoms();
     /* Send an IPC event just in case the ws names have changed */
index e505c94449e6be4de66c4247f39c789138cfab76..9af8e981227581422f3386619b790b9cf72b21fb 100644 (file)
@@ -389,9 +389,9 @@ struct CommandResult *parse_command(const char *input) {
                 position[(copywalk - input)] = (copywalk >= walk ? '^' : ' ');
             position[len] = '\0';
 
-            printf("%s\n", errormessage);
-            printf("Your command: %s\n", input);
-            printf("              %s\n", position);
+            ELOG("%s\n", errormessage);
+            ELOG("Your command: %s\n", input);
+            ELOG("              %s\n", position);
 
             /* Format this error message as a JSON reply. */
             y(map_open);
@@ -435,7 +435,15 @@ void debuglog(char *fmt, ...) {
     va_list args;
 
     va_start(args, fmt);
-    fprintf(stderr, "# ");
+    fprintf(stdout, "# ");
+    vfprintf(stdout, fmt, args);
+    va_end(args);
+}
+
+void errorlog(char *fmt, ...) {
+    va_list args;
+
+    va_start(args, fmt);
     vfprintf(stderr, fmt, args);
     va_end(args);
 }
index cb76eb788d99b328f67a194bf4cc34ad72cf2ee7..048d3a3a17992008326f6994bc57a99421ab7202 100644 (file)
@@ -77,58 +77,6 @@ bool event_is_ignored(const int sequence, const int response_type) {
     return false;
 }
 
-
-/*
- * There was a key press. We compare this key code with our bindings table and pass
- * the bound action to parse_command().
- *
- */
-static void handle_key_press(xcb_key_press_event_t *event) {
-
-    last_timestamp = event->time;
-
-    DLOG("Keypress %d, state raw = %d\n", event->detail, event->state);
-
-    /* Remove the numlock bit, all other bits are modifiers we can bind to */
-    uint16_t state_filtered = event->state & ~(xcb_numlock_mask | XCB_MOD_MASK_LOCK);
-    DLOG("(removed numlock, state = %d)\n", state_filtered);
-    /* Only use the lower 8 bits of the state (modifier masks) so that mouse
-     * button masks are filtered out */
-    state_filtered &= 0xFF;
-    DLOG("(removed upper 8 bits, state = %d)\n", state_filtered);
-
-    if (xkb_current_group == XkbGroup2Index)
-        state_filtered |= BIND_MODE_SWITCH;
-
-    DLOG("(checked mode_switch, state %d)\n", state_filtered);
-
-    /* Find the binding */
-    Binding *bind = get_binding(state_filtered, event->detail);
-
-    /* No match? Then the user has Mode_switch enabled but does not have a
-     * specific keybinding. Fall back to the default keybindings (without
-     * Mode_switch). Makes it much more convenient for users of a hybrid
-     * layout (like us, ru). */
-    if (bind == NULL) {
-        state_filtered &= ~(BIND_MODE_SWITCH);
-        DLOG("no match, new state_filtered = %d\n", state_filtered);
-        if ((bind = get_binding(state_filtered, event->detail)) == NULL) {
-            ELOG("Could not lookup key binding (modifiers %d, keycode %d)\n",
-                 state_filtered, event->detail);
-            return;
-        }
-    }
-
-    char *command_copy = sstrdup(bind->command);
-    struct CommandResult *command_output = parse_command(command_copy);
-    free(command_copy);
-
-    if (command_output->needs_tree_render)
-        tree_render();
-
-    yajl_gen_free(command_output->json_gen);
-}
-
 /*
  * Called with coordinates of an enter_notify event or motion_notify event
  * to check if the user crossed virtual screen boundaries and adjust the
diff --git a/src/key_press.c b/src/key_press.c
new file mode 100644 (file)
index 0000000..9aaea8f
--- /dev/null
@@ -0,0 +1,303 @@
+/*
+ * vim:ts=4:sw=4:expandtab
+ *
+ * i3 - an improved dynamic tiling window manager
+ * © 2009-2012 Michael Stapelberg and contributors (see also: LICENSE)
+ *
+ * key_press.c: key press handler
+ *
+ */
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <fcntl.h>
+#include "all.h"
+
+static int current_nesting_level;
+static bool success_key;
+static bool command_failed;
+
+/* XXX: I don’t want to touch too much of the nagbar code at once, but we
+ * should refactor this with src/cfgparse.y into a clean generic nagbar
+ * interface. It might come in handy in other situations within i3, too. */
+static char *pager_script_path;
+static pid_t nagbar_pid = -1;
+
+/*
+ * Handler which will be called when we get a SIGCHLD for the nagbar, meaning
+ * it exited (or could not be started, depending on the exit code).
+ *
+ */
+static void nagbar_exited(EV_P_ ev_child *watcher, int revents) {
+    ev_child_stop(EV_A_ watcher);
+
+    if (unlink(pager_script_path) != 0)
+        warn("Could not delete temporary i3-nagbar script %s", pager_script_path);
+
+    if (!WIFEXITED(watcher->rstatus)) {
+        fprintf(stderr, "ERROR: i3-nagbar did not exit normally.\n");
+        return;
+    }
+
+    int exitcode = WEXITSTATUS(watcher->rstatus);
+    printf("i3-nagbar process exited with status %d\n", exitcode);
+    if (exitcode == 2) {
+        fprintf(stderr, "ERROR: i3-nagbar could not be found. Is it correctly installed on your system?\n");
+    }
+
+    nagbar_pid = -1;
+}
+
+/* We need ev >= 4 for the following code. Since it is not *that* important (it
+ * only makes sure that there are no i3-nagbar instances left behind) we still
+ * support old systems with libev 3. */
+#if EV_VERSION_MAJOR >= 4
+/*
+ * Cleanup handler. Will be called when i3 exits. Kills i3-nagbar with signal
+ * SIGKILL (9) to make sure there are no left-over i3-nagbar processes.
+ *
+ */
+static void nagbar_cleanup(EV_P_ ev_cleanup *watcher, int revent) {
+    if (nagbar_pid != -1) {
+        LOG("Sending SIGKILL (%d) to i3-nagbar with PID %d\n", SIGKILL, nagbar_pid);
+        kill(nagbar_pid, SIGKILL);
+    }
+}
+#endif
+
+/*
+ * Writes the given command as a shell script to path.
+ * Returns true unless something went wrong.
+ *
+ */
+static bool write_nagbar_script(const char *path, const char *command) {
+    int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IXUSR);
+    if (fd == -1) {
+        warn("Could not create temporary script to store the nagbar command");
+        return false;
+    }
+    write(fd, "#!/bin/sh\n", strlen("#!/bin/sh\n"));
+    write(fd, command, strlen(command));
+    close(fd);
+    return true;
+}
+
+/*
+ * Starts an i3-nagbar process which alerts the user that his configuration
+ * file contains one or more errors. Also offers two buttons: One to launch an
+ * $EDITOR on the config file and another one to launch a $PAGER on the error
+ * logfile.
+ *
+ */
+static void start_commanderror_nagbar(void) {
+    if (nagbar_pid != -1) {
+        DLOG("i3-nagbar for command error already running, not starting again.\n");
+        return;
+    }
+
+    DLOG("Starting i3-nagbar due to command error\n");
+
+    /* We need to create a custom script containing our actual command
+     * since not every terminal emulator which is contained in
+     * i3-sensible-terminal supports -e with multiple arguments (and not
+     * all of them support -e with one quoted argument either).
+     *
+     * NB: The paths need to be unique, that is, don’t assume users close
+     * their nagbars at any point in time (and they still need to work).
+     * */
+    pager_script_path = get_process_filename("nagbar-cfgerror-pager");
+
+    nagbar_pid = fork();
+    if (nagbar_pid == -1) {
+        warn("Could not fork()");
+        return;
+    }
+
+    /* child */
+    if (nagbar_pid == 0) {
+        char *pager_command;
+        sasprintf(&pager_command, "i3-sensible-pager \"%s\"\n", errorfilename);
+        if (!write_nagbar_script(pager_script_path, pager_command))
+            return;
+
+        char *pageraction;
+        sasprintf(&pageraction, "i3-sensible-terminal -e \"%s\"", pager_script_path);
+        char *argv[] = {
+            NULL, /* will be replaced by the executable path */
+            "-t",
+            "error",
+            "-m",
+            "The configured command for this shortcut could not be run successfully.",
+            "-b",
+            "show errors",
+            pageraction,
+            NULL
+        };
+        exec_i3_utility("i3-nagbar", argv);
+    }
+
+    /* parent */
+    /* install a child watcher */
+    ev_child *child = smalloc(sizeof(ev_child));
+    ev_child_init(child, &nagbar_exited, nagbar_pid, 0);
+    ev_child_start(main_loop, child);
+
+/* We need ev >= 4 for the following code. Since it is not *that* important (it
+ * only makes sure that there are no i3-nagbar instances left behind) we still
+ * support old systems with libev 3. */
+#if EV_VERSION_MAJOR >= 4
+    /* install a cleanup watcher (will be called when i3 exits and i3-nagbar is
+     * still running) */
+    ev_cleanup *cleanup = smalloc(sizeof(ev_cleanup));
+    ev_cleanup_init(cleanup, nagbar_cleanup);
+    ev_cleanup_start(main_loop, cleanup);
+#endif
+}
+
+/*
+ * Kills the commanderror i3-nagbar process, if any.
+ *
+ * Called when reloading/restarting, since the user probably fixed his wrong
+ * keybindings.
+ *
+ * If wait_for_it is set (restarting), this function will waitpid(), otherwise,
+ * ev is assumed to handle it (reloading).
+ *
+ */
+void kill_commanderror_nagbar(bool wait_for_it) {
+    if (nagbar_pid == -1)
+        return;
+
+    if (kill(nagbar_pid, SIGTERM) == -1)
+        warn("kill(configerror_nagbar) failed");
+
+    if (!wait_for_it)
+        return;
+
+    /* When restarting, we don’t enter the ev main loop anymore and after the
+     * exec(), our old pid is no longer watched. So, ev won’t handle SIGCHLD
+     * for us and we would end up with a <defunct> process. Therefore we
+     * waitpid() here. */
+    waitpid(nagbar_pid, NULL, 0);
+}
+
+static int json_boolean(void *ctx, int boolval) {
+    DLOG("Got bool: %d, success_key %d, nesting_level %d\n", boolval, success_key, current_nesting_level);
+
+    if (success_key && current_nesting_level == 1 && !boolval)
+        command_failed = true;
+
+    return 1;
+}
+
+#if YAJL_MAJOR >= 2
+static int json_map_key(void *ctx, const unsigned char *stringval, size_t stringlen) {
+#else
+static int json_map_key(void *ctx, const unsigned char *stringval, unsigned int stringlen) {
+#endif
+    success_key = (stringlen >= strlen("success") &&
+                   strncmp((const char*)stringval, "success", strlen("success")) == 0);
+    return 1;
+}
+
+static int json_start_map(void *ctx) {
+    current_nesting_level++;
+    return 1;
+}
+
+static int json_end_map(void *ctx) {
+    current_nesting_level--;
+    return 1;
+}
+
+static yajl_callbacks command_error_callbacks = {
+    NULL,
+    &json_boolean,
+    NULL,
+    NULL,
+    NULL,
+    NULL,
+    &json_start_map,
+    &json_map_key,
+    &json_end_map,
+    NULL,
+    NULL
+};
+
+/*
+ * There was a key press. We compare this key code with our bindings table and pass
+ * the bound action to parse_command().
+ *
+ */
+void handle_key_press(xcb_key_press_event_t *event) {
+
+    last_timestamp = event->time;
+
+    DLOG("Keypress %d, state raw = %d\n", event->detail, event->state);
+
+    /* Remove the numlock bit, all other bits are modifiers we can bind to */
+    uint16_t state_filtered = event->state & ~(xcb_numlock_mask | XCB_MOD_MASK_LOCK);
+    DLOG("(removed numlock, state = %d)\n", state_filtered);
+    /* Only use the lower 8 bits of the state (modifier masks) so that mouse
+     * button masks are filtered out */
+    state_filtered &= 0xFF;
+    DLOG("(removed upper 8 bits, state = %d)\n", state_filtered);
+
+    if (xkb_current_group == XkbGroup2Index)
+        state_filtered |= BIND_MODE_SWITCH;
+
+    DLOG("(checked mode_switch, state %d)\n", state_filtered);
+
+    /* Find the binding */
+    Binding *bind = get_binding(state_filtered, event->detail);
+
+    /* No match? Then the user has Mode_switch enabled but does not have a
+     * specific keybinding. Fall back to the default keybindings (without
+     * Mode_switch). Makes it much more convenient for users of a hybrid
+     * layout (like us, ru). */
+    if (bind == NULL) {
+        state_filtered &= ~(BIND_MODE_SWITCH);
+        DLOG("no match, new state_filtered = %d\n", state_filtered);
+        if ((bind = get_binding(state_filtered, event->detail)) == NULL) {
+            ELOG("Could not lookup key binding (modifiers %d, keycode %d)\n",
+                 state_filtered, event->detail);
+            return;
+        }
+    }
+
+    char *command_copy = sstrdup(bind->command);
+    struct CommandResult *command_output = parse_command(command_copy);
+    free(command_copy);
+
+    if (command_output->needs_tree_render)
+        tree_render();
+
+    /* We parse the JSON reply to figure out whether there was an error
+     * ("success" being false in on of the returned dictionaries). */
+    const unsigned char *reply;
+#if YAJL_MAJOR >= 2
+    size_t length;
+    yajl_handle handle = yajl_alloc(&command_error_callbacks, NULL, NULL);
+#else
+    unsigned int length;
+    yajl_parser_config parse_conf = { 0, 0 };
+
+    yajl_handle handle = yajl_alloc(&command_error_callbacks, &parse_conf, NULL, NULL);
+#endif
+    yajl_gen_get_buf(command_output->json_gen, &reply, &length);
+
+    current_nesting_level = 0;
+    success_key = false;
+    command_failed = false;
+    yajl_status state = yajl_parse(handle, reply, length);
+    if (state != yajl_status_ok) {
+        ELOG("Could not parse my own reply. That's weird. reply is %.*s\n", length, reply);
+    } else {
+        if (command_failed)
+            start_commanderror_nagbar();
+    }
+
+    yajl_free(handle);
+
+    yajl_gen_free(command_output->json_gen);
+}
index d337963eaa093b2c1bd8f7d29684640f4b099bb8..76a74db1bd7de3277d2ebccb35cecaa964a5a904 100644 (file)
@@ -303,6 +303,7 @@ void i3_restart(bool forget_layout) {
     char *restart_filename = forget_layout ? NULL : store_restart_layout();
 
     kill_configerror_nagbar(true);
+    kill_commanderror_nagbar(true);
 
     restore_geometry();
 
index 8b57a0a1c7c9c4789871353b3e9d0dd5827181eb..335c775de346e1b0c9661e2e21e4a608470a0d83 100644 (file)
@@ -12,7 +12,7 @@ sub parser_calls {
 
     # TODO: use a timeout, so that we can error out if it doesn’t terminate
     # TODO: better way of passing arguments
-    my $stdout = qx(../test.commands_parser '$command' 2>&-);
+    my $stdout = qx(../test.commands_parser '$command' 2>&1 >&-);
 
     # Filter out all debugging output.
     my @lines = split("\n", $stdout);
@@ -127,15 +127,15 @@ is(parser_calls("\nworkspace test"),
 ################################################################################
 
 is(parser_calls('unknown_literal'),
-   "Expected one of these tokens: <end>, '[', 'move', 'exec', 'exit', 'restart', 'reload', 'border', 'layout', 'append_layout', 'workspace', 'focus', 'kill', 'open', 'fullscreen', 'split', 'floating', 'mark', 'resize', 'rename', 'nop', 'scratchpad', 'mode'\n" .
-   "Your command: unknown_literal\n" .
-   "              ^^^^^^^^^^^^^^^",
+   "ERROR: Expected one of these tokens: <end>, '[', 'move', 'exec', 'exit', 'restart', 'reload', 'border', 'layout', 'append_layout', 'workspace', 'focus', 'kill', 'open', 'fullscreen', 'split', 'floating', 'mark', 'resize', 'rename', 'nop', 'scratchpad', 'mode'\n" .
+   "ERROR: Your command: unknown_literal\n" .
+   "ERROR:               ^^^^^^^^^^^^^^^",
    'error for unknown literal ok');
 
 is(parser_calls('move something to somewhere'),
-   "Expected one of these tokens: 'window', 'container', 'to', 'workspace', 'output', 'scratchpad', 'left', 'right', 'up', 'down', 'position', 'absolute'\n" .
-   "Your command: move something to somewhere\n" .
-   "                   ^^^^^^^^^^^^^^^^^^^^^^",
+   "ERROR: Expected one of these tokens: 'window', 'container', 'to', 'workspace', 'output', 'scratchpad', 'left', 'right', 'up', 'down', 'position', 'absolute'\n" .
+   "ERROR: Your command: move something to somewhere\n" .
+   "ERROR:                    ^^^^^^^^^^^^^^^^^^^^^^",
    'error for unknown literal ok');
 
 ################################################################################