user-notification: 'productify' the user-warning system

* Rename user_warning to user_notification
* Add warning and error types (in addition to the existing deprecated)
* Simplify logic when emitting a user notification after forking; we
  don't need to copy the notification data since we're in a new
  process and have total control over that memory.

config.c

@@ -310,16 +310,16 @@ parse_section_main(const char *key, const char *value, struct config *conf,
         LOG_WARN("deprecated: 'scrollback' option, "
                  "use 'lines' in the '[scrollback]' section instead'");
 
-        const char *fmt = "%s:%d: \e[1mscrollback\e[21m option, use \e[1mlines\e[21m in the \e[1m[scrollback]\e[21m section";
+        const char *fmt = "%s:%d: \e[1mscrollback\e[21m option, use \e[1mscrollback.lines\e[21m instead";
         int len = snprintf(NULL, 0, fmt, path, lineno);
         char *text = malloc(len + 1);
         snprintf(text, len + 1, fmt, path, lineno);
 
-        struct user_warning warning = {
+        struct user_notification deprecation = {
-            .kind = USER_WARNING_DEPRECATION,
+            .kind = USER_NOTIFICATION_DEPRECATED,
             .text = text,
         };
-        tll_push_back(conf->warnings, warning);
+        tll_push_back(conf->notifications, deprecation);
 
         unsigned long lines;
         if (!str_to_ulong(value, 10, &lines)) {
@@ -581,10 +581,9 @@ parse_section_csd(const char *key, const char *value, struct config *conf,
 }
 
 static bool
-verify_key_combo(const struct config *conf, enum bind_action_normal action,
+verify_key_combo(const struct config *conf, const char *combo, const char *path, unsigned lineno)
-                 const char *const binding_action_map[],
-                 const char *combo, const char *path, unsigned lineno)
 {
+    /* Check regular key bindings */
     tll_foreach(conf->bindings.key, it) {
         char *copy = strdup(it->item.key);
 
@@ -593,8 +592,31 @@ verify_key_combo(const struct config *conf, enum bind_action_normal action,
              collision = strtok_r(NULL, " ", &save))
         {
             if (strcmp(combo, collision) == 0) {
-                LOG_ERR("%s:%d: %s already mapped to %s", path, lineno, combo,
+                bool has_pipe = it->item.pipe.cmd != NULL;
-                        binding_action_map[it->item.action]);
+                LOG_ERR("%s:%d: %s already mapped to '%s%s%s%s'", path, lineno, combo,
+                        binding_action_map[it->item.action],
+                        has_pipe ? " [" : "",
+                        has_pipe ? it->item.pipe.cmd : "",
+                        has_pipe ? "]" : "");
+                free(copy);
+                return false;
+            }
+        }
+
+        free(copy);
+    }
+
+    /* Check scrollback search bindings */
+    tll_foreach(conf->bindings.search, it) {
+        char *copy = strdup(it->item.key);
+
+        for (char *save = NULL, *collision = strtok_r(copy, " ", &save);
+             collision != NULL;
+             collision = strtok_r(NULL, " ", &save))
+        {
+            if (strcmp(combo, collision) == 0) {
+                LOG_ERR("%s:%d: %s already mapped to '%s'", path, lineno, combo,
+                        search_binding_action_map[it->item.action]);
                 free(copy);
                 return false;
             }
@@ -620,6 +642,29 @@ verify_key_combo(const struct config *conf, enum bind_action_normal action,
     return true;
 }
 
+static int
+argv_compare(char *const *argv1, char *const *argv2)
+{
+    assert(argv1 != NULL);
+    assert(argv2 != NULL);
+
+    for (size_t i = 0; ; i++) {
+        if (argv1[i] == NULL && argv2[i] == NULL)
+            return 0;
+        if (argv1[i] == NULL)
+            return -1;
+        if (argv2[i] == NULL)
+            return 1;
+
+        int ret = strcmp(argv1[i], argv2[i]);
+        if (ret != 0)
+            return ret;
+    }
+
+    assert(false);
+    return 1;
+}
+
 static bool
 parse_section_key_bindings(
     const char *key, const char *value, struct config *conf,
@@ -674,7 +719,7 @@ parse_section_key_bindings(
             return true;
         }
 
-        if (!verify_key_combo(conf, action, binding_action_map, value, path, lineno)) {
+        if (!verify_key_combo(conf, value, path, lineno)) {
             free(pipe_argv);
             free(pipe_cmd);
             return false;
@@ -744,7 +789,7 @@ parse_section_search_bindings(
             return true;
         }
 
-        if (!verify_key_combo(conf, action, search_binding_action_map, value, path, lineno)) {
+        if (!verify_key_combo(conf, value, path, lineno)) {
             return false;
         }
 
@@ -1168,7 +1213,7 @@ config_load(struct config *conf, const char *conf_path)
             .max_shm_pool_size = 512 * 1024 * 1024,
         },
 
-        .warnings = tll_init(),
+        .notifications = tll_init(),
     };
 
     struct config_key_binding_normal scrollback_up =   {BIND_ACTION_SCROLLBACK_UP,   strdup("Shift+Page_Up")};
@@ -1285,9 +1330,9 @@ config_free(struct config conf)
     tll_free(conf.bindings.mouse);
     tll_free(conf.bindings.search);
 
-    tll_foreach(conf.warnings, it)
+    tll_foreach(conf.notifications, it)
         free(it->item.text);
-    tll_free(conf.warnings);
+    tll_free(conf.notifications);
 }
 
 struct config_font

config.h

@@ -6,6 +6,7 @@
 #include <tllist.h>
 
 #include "terminal.h"
+#include "user-notification.h"
 #include "wayland.h"
 
 struct config_font {
@@ -123,7 +124,7 @@ struct config {
         off_t max_shm_pool_size;
     } tweak;
 
-    user_warning_list_t warnings;
+    user_notifications_t notifications;
 };
 
 bool config_load(struct config *conf, const char *path);

meson.build

@@ -126,6 +126,7 @@ executable(
   'spawn.c', 'spawn.h',
   'terminal.c', 'terminal.h',
   'tokenize.c', 'tokenize.h',
+  'user-notification.h',
   'vt.c', 'vt.h',
   'wayland.c', 'wayland.h',
   wl_proto_src + wl_proto_headers, version,

slave.c

@@ -67,9 +67,52 @@ err:
     return false;
 }
 
+static bool
+emit_one_notification(int fd, const struct user_notification *notif)
+{
+    const char *prefix = NULL;
+    const char *postfix = "\e[m\n";
+
+    switch (notif->kind) {
+    case USER_NOTIFICATION_DEPRECATED:
+        prefix = "\e[33;1mdeprecated\e[39;21m: ";
+        break;
+
+    case USER_NOTIFICATION_WARNING:
+        prefix = "\e[33;1mwarning\e[39;21m: ";
+        break;
+
+    case USER_NOTIFICATION_ERROR:
+        prefix = "\e[31;1merror\e[39;21m: ";
+        break;
+    }
+
+    assert(prefix != NULL);
+
+    if (write(fd, prefix, strlen(prefix)) < 0 ||
+        write(fd, notif->text, strlen(notif->text)) < 0 ||
+        write(fd, postfix, strlen(postfix)) < 0)
+    {
+        return false;
+    }
+
+    return true;
+}
+
+static bool
+emit_notifications(int fd, const user_notifications_t *notifications)
+{
+    tll_foreach(*notifications, it) {
+        if (!emit_one_notification(fd, &it->item))
+            return false;
+    }
+
+    return true;
+}
+
 static void
 slave_exec(int ptmx, char *argv[], int err_fd, bool login_shell,
-           size_t warning_count, struct user_warning warnings[static warning_count])
+           const user_notifications_t *notifications)
 {
     int pts = -1;
     const char *pts_name = ptsname(ptmx);
@@ -124,23 +167,8 @@ slave_exec(int ptmx, char *argv[], int err_fd, bool login_shell,
         goto err;
     }
 
-    for (size_t i = 0; i < warning_count; i++) {
+    if (!emit_notifications(pts, notifications))
-        switch (warnings[i].kind) {
+        goto err;
-        case USER_WARNING_DEPRECATION:
-            if (write(pts, "\e[33;1;5mdeprecated:\e[39;21;25m ", 32) < 0)
-                goto err;
-        }
-
-        if (write(pts, warnings[i].text, strlen(warnings[i].text)) < 0 ||
-            write(pts, "\e[m\n", 4) < 0)
-        {
-            goto err;
-        }
-        free(warnings[i].text);
-        warnings[i].text = NULL;
-    }
-    free(warnings);
-    warnings = NULL;
 
     close(pts);
     pts = -1;
@@ -167,18 +195,13 @@ err:
     if (ptmx != -1)
         close(ptmx);
     close(err_fd);
-
-    for (size_t i = 0; i < warning_count; i++)
-        free(warnings[i].text);
-    free(warnings);
-
     _exit(errno);
 }
 
 pid_t
 slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
             const char *term_env, const char *conf_shell, bool login_shell,
-            const user_warning_list_t *warnings)
+            const user_notifications_t *notifications)
 {
     int fork_pipe[2];
     if (pipe2(fork_pipe, O_CLOEXEC) < 0) {
@@ -246,20 +269,7 @@ slave_spawn(int ptmx, int argc, const char *cwd, char *const *argv,
         if (is_valid_shell(shell_argv[0]))
             setenv("SHELL", shell_argv[0], 1);
 
-        struct user_warning *warnings_copy = malloc(
+        slave_exec(ptmx, shell_argv, fork_pipe[1], login_shell, notifications);
-            tll_length(*warnings) * sizeof(warnings_copy[0]));
-        {
-            size_t i = 0;
-            tll_foreach(*warnings, it){
-                warnings_copy[i] = (struct user_warning){
-                    .kind = it->item.kind,
-                    .text = strdup(it->item.text),
-                };
-            }
-        }
-
-        slave_exec(ptmx, shell_argv, fork_pipe[1], login_shell,
-                   tll_length(*warnings), warnings_copy);
         assert(false);
         break;
 

slave.h

@@ -3,9 +3,9 @@
 
 #include <sys/types.h>
 
-#include "terminal.h"
+#include "user-notification.h"
 
 pid_t slave_spawn(
     int ptmx, int argc, const char *cwd, char *const *argv, const char *term_env,
     const char *conf_shell, bool login_shell,
-    const user_warning_list_t *warnings);
+    const user_notifications_t *notifications);

terminal.c

@@ -944,7 +944,7 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
     if ((term->slave = slave_spawn(
              term->ptmx, argc, term->cwd, argv,
              conf->term, conf->shell, conf->login_shell,
-             &conf->warnings)) == -1)
+             &conf->notifications)) == -1)
     {
         goto err;
     }

terminal.h

@@ -206,12 +206,6 @@ enum term_surface {
     TERM_SURF_BUTTON_CLOSE,
 };
 
-struct user_warning {
-    enum { USER_WARNING_DEPRECATION } kind;
-    char *text;
-};
-typedef tll(struct user_warning) user_warning_list_t;
-
 struct terminal {
     struct fdm *fdm;
     struct reaper *reaper;

user-notification.h

@@ -0,0 +1,14 @@
+#pragma once
+
+#include <tllist.h>
+
+struct user_notification {
+    enum {
+        USER_NOTIFICATION_DEPRECATED,
+        USER_NOTIFICATION_WARNING,
+        USER_NOTIFICATION_ERROR,
+    } kind;
+    char *text;
+};
+
+typedef tll(struct user_notification) user_notifications_t;