Proposal for settings refactoring

config.h

@@ -46,8 +46,7 @@ settings_t defaults = {
 .separator_height = 2,       /* height of the separator line between two notifications */
 .padding = 0,
 .h_padding = 0,              /* horizontal padding */
-.sep_color = SEP_AUTO,       /* SEP_AUTO, SEP_FOREGROUND, SEP_FRAME, SEP_CUSTOM */
-.sep_custom_color_str = NULL,/* custom color if sep_color is set to CUSTOM */
+.sep_color = {SEP_AUTO, NULL},/* SEP_AUTO, SEP_FOREGROUND, SEP_FRAME, SEP_CUSTOM */
 
 .frame_width = 0,
 .frame_color = "#888888",
@@ -102,6 +101,7 @@ settings_t defaults = {
         .code = 0,.sym = NoSymbol,.is_valid = false
 },                              /* ignore this */
 
+.log_level = G_LOG_LEVEL_WARNING,
 };
 
 rule_t default_rules[] = {

src/draw.c

@@ -92,14 +92,14 @@ static color_t calculate_foreground_color(color_t bg)
 
 static color_t layout_get_sepcolor(colored_layout *cl, colored_layout *cl_next)
 {
-        switch (settings.sep_color) {
+        switch (settings.sep_color.type) {
         case SEP_FRAME:
                 if (cl_next->n->urgency > cl->n->urgency)
                         return cl_next->frame;
                 else
                         return cl->frame;
         case SEP_CUSTOM:
-                return string_to_color(settings.sep_custom_color_str);
+                return string_to_color(settings.sep_color.sep_color);
         case SEP_FOREGROUND:
                 return cl->fg;
         case SEP_AUTO:
@@ -491,7 +491,7 @@ static cairo_surface_t *render_background(cairo_surface_t *srf,
         draw_rounded_rect(c, x, y, width, height, corner_radius, first, last);
         cairo_fill(c);
 
-        if (   settings.sep_color != SEP_FRAME
+        if (   settings.sep_color.type != SEP_FRAME
             && settings.separator_height > 0
             && !last) {
                 color_t sep_color = layout_get_sepcolor(cl, cl_next);

src/dunst.c

@@ -125,23 +125,16 @@ int dunst_main(int argc, char *argv[])
 
         dunst_log_init(false);
 
-        if (cmdline_get_bool("-v/-version", false, "Print version")
-            || cmdline_get_bool("--version", false, "Print version")) {
+        if (cmdline_flag("-v/-version/--version", "Print version")) {
                 print_version();
         }
 
-        char *verbosity = cmdline_get_string("-verbosity", NULL, "Minimum level for message");
-        log_set_level_from_string(verbosity);
-        g_free(verbosity);
-
-        char *cmdline_config_path;
-        cmdline_config_path =
+        const char *cmdline_config_path =
             cmdline_get_string("-conf/-config", NULL,
                                "Path to configuration file");
         load_settings(cmdline_config_path);
 
-        if (cmdline_get_bool("-h/-help", false, "Print help")
-            || cmdline_get_bool("--help", false, "Print help")) {
+        if (cmdline_flag("-h/-help/--help", "Print help")) {
                 usage(EXIT_SUCCESS);
         }
 

src/log.c

@@ -8,10 +8,10 @@
 
 #include <glib.h>
 
-static GLogLevelFlags log_level = G_LOG_LEVEL_WARNING;
+#include "settings.h"
 
 /* see log.h */
-static const char *log_level_to_string(GLogLevelFlags level)
+const char *log_level_to_string(GLogLevelFlags level)
 {
         switch (level) {
         case G_LOG_LEVEL_ERROR: return "ERROR";
@@ -25,36 +25,32 @@ static const char *log_level_to_string(GLogLevelFlags level)
 }
 
 /* see log.h */
-void log_set_level_from_string(const char *level)
+GLogLevelFlags string_parse_loglevel(const char *level, GLogLevelFlags def)
 {
         if (!level)
-                return;
+                return def;
 
         if (g_ascii_strcasecmp(level, "critical") == 0)
-                log_level = G_LOG_LEVEL_CRITICAL;
+                return G_LOG_LEVEL_CRITICAL;
         else if (g_ascii_strcasecmp(level, "crit") == 0)
-                log_level = G_LOG_LEVEL_CRITICAL;
+                return G_LOG_LEVEL_CRITICAL;
         else if (g_ascii_strcasecmp(level, "warning") == 0)
-                log_level = G_LOG_LEVEL_WARNING;
+                return G_LOG_LEVEL_WARNING;
         else if (g_ascii_strcasecmp(level, "warn") == 0)
-                log_level = G_LOG_LEVEL_WARNING;
+                return G_LOG_LEVEL_WARNING;
         else if (g_ascii_strcasecmp(level, "message") == 0)
-                log_level = G_LOG_LEVEL_MESSAGE;
+                return G_LOG_LEVEL_MESSAGE;
         else if (g_ascii_strcasecmp(level, "mesg") == 0)
-                log_level = G_LOG_LEVEL_MESSAGE;
+                return G_LOG_LEVEL_MESSAGE;
         else if (g_ascii_strcasecmp(level, "info") == 0)
-                log_level = G_LOG_LEVEL_INFO;
+                return G_LOG_LEVEL_INFO;
         else if (g_ascii_strcasecmp(level, "debug") == 0)
-                log_level = G_LOG_LEVEL_DEBUG;
+                return G_LOG_LEVEL_DEBUG;
         else if (g_ascii_strcasecmp(level, "deb") == 0)
-                log_level = G_LOG_LEVEL_DEBUG;
-        else
-                LOG_W("Unknown log level: '%s'", level);
-}
+                return G_LOG_LEVEL_DEBUG;
 
-void log_set_level(GLogLevelFlags level)
-{
-        log_level = level;
+        LOG_W("Unknown log level: '%s'", level);
+        return def;
 }
 
 /**
@@ -77,7 +73,7 @@ static void dunst_log_handler(
 /* if you want to have a debug build, you want to log anything,
  * unconditionally, without specifying debug log level again */
 #ifndef DEBUG_BUILD
-        if (log_level < message_level)
+        if (settings.log_level < message_level)
                 return;
 #endif
         const char *log_level_str =
@@ -94,6 +90,7 @@ static void dunst_log_handler(
 void dunst_log_init(bool testing)
 {
         g_log_set_default_handler(dunst_log_handler, (void*)testing);
+        settings.log_level = G_LOG_LEVEL_MESSAGE;
 }
 
 /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */

src/log.h

@@ -16,24 +16,35 @@
 
 #define DIE(...) do { LOG_C(__VA_ARGS__); exit(EXIT_FAILURE); } while (0)
 
+
 /**
- * @return the string representation of the given `level`
+ * Return the corresponding string representation of `level`
+ *
+ * @param level The level to convert to a string
+ *
+ * @return the representation of the given level
+ * @return `"UNKNOWN"` if `level` is an invalid
  */
-void log_set_level(GLogLevelFlags level);
+const char *log_level_to_string(GLogLevelFlags level);
 
 /**
- * Set the current loglevel to `level`
+ * Return the corresponding log level for given string
  *
- * @param level The desired log level
+ * @param level The string to convert. It shall not
+ *              contain waste characters.
+ * @param def The default to fallback to
  *
- * If `level` is `NULL`, nothing will be done.
- * If `level` is an invalid value, nothing will be done.
+ * @return the representation of the given logstring
+ * @return `def` if `string` is an invalid value or `NULL`
  */
-void log_set_level_from_string(const char* level);
+GLogLevelFlags string_parse_loglevel(const char *level, GLogLevelFlags def);
 
 /**
  * Initialise log handling. Can be called any time.
  *
+ * To avoid any issues, it'll set #settings_t.log_level of
+ * the #settings variable to `G_LOG_LEVEL_MESSAGE`.
+ *
  * @param testing If we're in testing mode and should
  *                suppress all output
  */

src/menu.c

@@ -50,6 +50,49 @@ void regex_teardown(void)
         }
 }
 
+/* see menu.h */
+char **split_into_cmd(const char *command, ...)
+{
+        GError *error = NULL;
+        char **split;
+        if (!g_shell_parse_argv(command, NULL, &split, &error)) {
+                LOG_W("Unable to parse command: '%s'.", error->message);
+                g_error_free(error);
+                split = NULL;
+        }
+
+        if (split) {
+                va_list args;
+                char *cur;
+                int len_args = 0;
+
+                va_start(args, command);
+                while ((cur = va_arg(args, char*)))
+                        len_args++;
+                va_end(args);
+
+                if (len_args > 0) {
+                        size_t len_cmd = g_strv_length(split);
+                        gchar **cmd = g_new(gchar*, len_cmd+len_args+1);
+                        memcpy(cmd, split, len_cmd*(sizeof(gchar*)));
+
+                        len_args = 0;
+                        va_start(args, command);
+                        while ((cur = va_arg(args, char*))) {
+                                cmd[len_cmd+len_args] = g_strdup(cur);
+                                len_args++;
+                        }
+                        va_end(args);
+                        cmd[len_cmd+len_args] = NULL;
+
+                        g_free(split);
+                        split = cmd;
+                }
+        }
+
+        return split;
+}
+
 /*
  * Exctract all urls from a given string.
  *
@@ -116,12 +159,11 @@ void open_browser(const char *in)
                 if (browser_pid2) {
                         exit(0);
                 } else {
-                        char *browser_cmd = g_strconcat(settings.browser, " ", url, NULL);
-                        char **cmd = g_strsplit(browser_cmd, " ", 0);
-                        execvp(cmd[0], cmd);
+                        char **browser_cmd = split_into_cmd(settings.browser, url, NULL);
+                        execvp(browser_cmd[0], browser_cmd);
                         // execvp won't return if it's successful
                         // so, if we're here, it's definitely an error
-                        fprintf(stderr, "Warning: failed to execute '%s': %s\n",
+                        LOG_W("Failed to execute '%s': %s\n",
                                         settings.browser,
                                         strerror(errno));
                         exit(EXIT_FAILURE);
@@ -193,10 +235,6 @@ void dispatch_menu_result(const char *input)
  */
 void context_menu(void)
 {
-        if (settings.dmenu_cmd == NULL) {
-                LOG_C("Unable to open dmenu: No dmenu command set.");
-                return;
-        }
         char *dmenu_input = NULL;
 
         for (const GList *iter = queues_get_displayed(); iter;
@@ -243,10 +281,17 @@ void context_menu(void)
                         LOG_W("dup(): error in parent: %s", strerror(errno));
                         exit(EXIT_FAILURE);
                 }
-                execvp(settings.dmenu_cmd[0], settings.dmenu_cmd);
-                fprintf(stderr, "Warning: failed to execute '%s': %s\n",
-                                settings.dmenu,
-                                strerror(errno));
+
+                char **dmenu_cmd = split_into_cmd(settings.dmenu, NULL);
+
+                if (dmenu_cmd) {
+                        execvp(dmenu_cmd[0], dmenu_cmd);
+                        LOG_W("Failed to execute '%s': %s\n",
+                                        settings.dmenu,
+                                        strerror(errno));
+                } else {
+                        LOG_W("Cannot execute '%s'.", settings.dmenu);
+                }
                 exit(EXIT_FAILURE);
         } else {
                 close(child_io[0]);

src/menu.h

@@ -7,5 +7,14 @@ void open_browser(const char *in);
 void invoke_action(const char *action);
 void regex_teardown(void);
 
+/** Split the command into its shell equivalent parts
+ *
+ * @return a NULL terminated string vector containing
+ *         the command split into its arguments
+ *         with additional non-splitted arguments,
+ *         which got passed as the variadic arguments
+ */
+char **split_into_cmd(const char *command, ...);
+
 #endif
 /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */