From 94be674cf8bf411a8ced649dea86fde29132f48d Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Sat, 21 Mar 2020 02:23:07 -0400 Subject: [PATCH 1/5] Add parsing of comma-separated list options from conf/cmdline. --- src/option_parser.c | 36 ++++++++++++++++++++++++++++++++++++ src/option_parser.h | 7 +++++++ src/utils.c | 32 ++++++++++++++++++++++++++++++++ src/utils.h | 20 ++++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/src/option_parser.c b/src/option_parser.c index 2a11d6c..4f16d59 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -260,6 +260,15 @@ gint64 ini_get_time(const char *section, const char *key, gint64 def) return val; } +char **ini_get_list(const char *section, const char *key, const char *def) +{ + const char *value = get_value(section, key); + if (value) + return string_to_array(value); + else + return string_to_array(def); +} + int ini_get_int(const char *section, const char *key, int def) { const char *value = get_value(section, key); @@ -475,6 +484,16 @@ char *cmdline_get_path(const char *key, const char *def, const char *description return string_to_path(g_strdup(def)); } +char **cmdline_get_list(const char *key, const char *def, const char *description){ + cmdline_usage_append(key, "list", description); + const char *str = cmdline_get_value(key); + + if (str) + return string_to_array(str); + else + return string_to_array(def); +} + gint64 cmdline_get_time(const char *key, gint64 def, const char *description) { cmdline_usage_append(key, "time", description); @@ -574,6 +593,23 @@ gint64 option_get_time(const char *ini_section, return cmdline_get_time(cmdline_key, ini_val, description); } + +char **option_get_list(const char *ini_section, + const char *ini_key, + const char *cmdline_key, + const char *def, + const char *description) +{ + char **val = NULL; + if (cmdline_key) + val = cmdline_get_list(cmdline_key, NULL, description); + + if (val) + return val; + else + return ini_get_list(ini_section, ini_key, def); +} + int option_get_int(const char *ini_section, const char *ini_key, const char *cmdline_key, diff --git a/src/option_parser.h b/src/option_parser.h index f3861bc..88a7659 100644 --- a/src/option_parser.h +++ b/src/option_parser.h @@ -24,6 +24,7 @@ int load_ini_file(FILE *); char *ini_get_path(const char *section, const char *key, const char *def); char *ini_get_string(const char *section, const char *key, const char *def); gint64 ini_get_time(const char *section, const char *key, gint64 def); +char **ini_get_list(const char *section, const char *key, const char *def); int ini_get_int(const char *section, const char *key, int def); double ini_get_double(const char *section, const char *key, double def); int ini_get_bool(const char *section, const char *key, int def); @@ -34,6 +35,7 @@ void cmdline_load(int argc, char *argv[]); /* for all cmdline_get_* key can be either "-key" or "-key/-longkey" */ char *cmdline_get_string(const char *key, const char *def, const char *description); char *cmdline_get_path(const char *key, const char *def, const char *description); +char **cmdline_get_list(const char *key, const char *def, const char *description); int cmdline_get_int(const char *key, int def, const char *description); double cmdline_get_double(const char *key, double def, const char *description); int cmdline_get_bool(const char *key, int def, const char *description); @@ -55,6 +57,11 @@ gint64 option_get_time(const char *ini_section, const char *cmdline_key, gint64 def, const char *description); +char **option_get_list(const char *ini_section, + const char *ini_key, + const char *cmdline_key, + const char *def, + const char *description); int option_get_int(const char *ini_section, const char *ini_key, const char *cmdline_key, diff --git a/src/utils.c b/src/utils.c index 6a1d2fb..d837774 100644 --- a/src/utils.c +++ b/src/utils.c @@ -13,6 +13,17 @@ #include "log.h" +/* see utils.h */ +void free_string_array(char **arr) +{ + if (arr){ + for (int i = 0; arr[i]; i++){ + g_free(arr[i]); + } + } + g_free(arr); +} + /* see utils.h */ char *string_replace_char(char needle, char replacement, char *haystack) { @@ -135,6 +146,27 @@ void string_strip_delimited(char *str, char a, char b) str[iwrite] = 0; } +/* see utils.h */ +char **string_to_array(const char *string) +{ + char **arr = NULL; + if (string) { + char* dup = g_strdup(string); + char* tmp = dup; + int num_tokens = 0; + char *token = strsep(&tmp, ","); + while (token) { + arr = g_realloc_n(arr, num_tokens + 2, sizeof(char*)); + arr[num_tokens] = g_strdup(g_strstrip(token)); + num_tokens++; + token = strsep(&tmp, ","); + } + arr[num_tokens] = NULL; + g_free(dup); + } + return arr; +} + /* see utils.h */ char *string_to_path(char *string) { diff --git a/src/utils.h b/src/utils.h index 2278f6d..eea44c0 100644 --- a/src/utils.h +++ b/src/utils.h @@ -22,6 +22,15 @@ //! Convert a second into the internal time representation #define S2US(s) (((gint64)(s)) * 1000 * 1000) +/** + * Frees an array of strings whose last element is a NULL pointer. + * + * Assumes the last element is a NULL pointer, otherwise will result in a buffer overflow. + + * @param arr The array of strings to free + */ +void free_string_array(char **arr); + /** * Replaces all occurrences of the char \p needle with the char \p replacement in \p haystack. * @@ -82,6 +91,17 @@ char *string_strip_quotes(const char *value); */ void string_strip_delimited(char *str, char a, char b); +/** + * Parse a comma-delimited string into a dynamic array of tokens + * + * The string is split on commas and strips spaces from tokens. The last element + * of the array is NULL in order to avoid passing around a length variable. + * + * @param string The string to convert to an array + * @returns The array of tokens. + */ +char **string_to_array(const char *string); + /** * Replace tilde and path-specific values with its equivalents * From 506b4f2cfa27e1c6218908c2f22c79b2d6b848b3 Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Sun, 22 Mar 2020 02:37:04 -0400 Subject: [PATCH 2/5] Allow a mouse button to perform 1 or more actions in series. The user provides a comma-separated list of valid mouse actions that will be performed one after another when a notification is clicked. If any one of the provided actions is invalid, the value reverts to its default state. --- config.h | 6 ++--- src/option_parser.c | 24 ++++++++++++++++++- src/option_parser.h | 1 + src/settings.c | 26 ++++++++------------ src/settings.h | 6 ++--- src/x11/x.c | 58 +++++++++++++++++++++++---------------------- 6 files changed, 70 insertions(+), 51 deletions(-) diff --git a/config.h b/config.h index 14da2c4..f8ca9cf 100644 --- a/config.h +++ b/config.h @@ -103,11 +103,11 @@ struct settings defaults = { .code = 0,.sym = NoSymbol,.is_valid = false }, /* ignore this */ -.mouse_left_click = MOUSE_CLOSE_CURRENT, +.mouse_left_click = (enum mouse_action []){MOUSE_CLOSE_CURRENT, -1}, -.mouse_middle_click = MOUSE_DO_ACTION, +.mouse_middle_click = (enum mouse_action []){MOUSE_DO_ACTION, -1}, -.mouse_right_click = MOUSE_CLOSE_ALL, +.mouse_right_click = (enum mouse_action []){MOUSE_CLOSE_ALL, -1}, }; diff --git a/src/option_parser.c b/src/option_parser.c index 4f16d59..c958db3 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -141,6 +141,27 @@ bool string_parse_mouse_action(const char *s, enum mouse_action *ret) return false; } +bool string_parse_mouse_action_list(char **s, enum mouse_action **ret) +{ + ASSERT_OR_RET(s, false); + ASSERT_OR_RET(ret, false); + + int len = 0; + while (s[len]) + len++; + + *ret = g_malloc((len + 1) * sizeof(enum mouse_action)); + for (int i = 0; i < len; i++) { + if (!string_parse_mouse_action(s[i], *ret + i)) { + LOG_W("Unknown mouse action value: '%s'", s[i]); + g_free(*ret); + return false; + } + } + (*ret)[len] = -1; // sentinel end value + return true; +} + bool string_parse_sepcolor(const char *s, struct separator_color_data *ret) { ASSERT_OR_RET(STR_FULL(s), false); @@ -484,7 +505,8 @@ char *cmdline_get_path(const char *key, const char *def, const char *description return string_to_path(g_strdup(def)); } -char **cmdline_get_list(const char *key, const char *def, const char *description){ +char **cmdline_get_list(const char *key, const char *def, const char *description) +{ cmdline_usage_append(key, "list", description); const char *str = cmdline_get_value(key); diff --git a/src/option_parser.h b/src/option_parser.h index 88a7659..ce412eb 100644 --- a/src/option_parser.h +++ b/src/option_parser.h @@ -17,6 +17,7 @@ bool string_parse_icon_position(const char *s, enum icon_position *ret); bool string_parse_vertical_alignment(const char *s, enum vertical_alignment *ret); bool string_parse_markup_mode(const char *s, enum markup_mode *ret); bool string_parse_mouse_action(const char *s, enum mouse_action *ret); +bool string_parse_mouse_action_list(char **s, enum mouse_action **ret); bool string_parse_sepcolor(const char *s, struct separator_color_data *ret); bool string_parse_urgency(const char *s, enum urgency *ret); diff --git a/src/settings.c b/src/settings.c index a3dbf1e..e67446b 100644 --- a/src/settings.c +++ b/src/settings.c @@ -529,48 +529,42 @@ void load_settings(char *cmdline_config_path) } { - char *c = option_get_string( + char **c = option_get_list( "global", - "mouse_left_click", "-left_click", NULL, + "mouse_left_click", "-mouse_left_click", NULL, "Action of Left click event" ); - if (!string_parse_mouse_action(c, &settings.mouse_left_click)) { - if (c) - LOG_W("Unknown mouse action value: '%s'", c); + if (!string_parse_mouse_action_list(c, &settings.mouse_left_click)) { settings.mouse_left_click = defaults.mouse_left_click; } - g_free(c); + free_string_array(c); } { - char *c = option_get_string( + char **c = option_get_list( "global", "mouse_middle_click", "-mouse_middle_click", NULL, "Action of middle click event" ); - if (!string_parse_mouse_action(c, &settings.mouse_middle_click)) { - if (c) - LOG_W("Unknown mouse action value: '%s'", c); + if (!string_parse_mouse_action_list(c, &settings.mouse_middle_click)) { settings.mouse_middle_click = defaults.mouse_middle_click; } - g_free(c); + free_string_array(c); } { - char *c = option_get_string( + char **c = option_get_list( "global", "mouse_right_click", "-mouse_right_click", NULL, "Action of right click event" ); - if (!string_parse_mouse_action(c, &settings.mouse_right_click)) { - if (c) - LOG_W("Unknown mouse action value: '%s'", c); + if (!string_parse_mouse_action_list(c, &settings.mouse_right_click)) { settings.mouse_right_click = defaults.mouse_right_click; } - g_free(c); + free_string_array(c); } settings.colors_low.bg = option_get_string( diff --git a/src/settings.h b/src/settings.h index 0110745..6622880 100644 --- a/src/settings.h +++ b/src/settings.h @@ -88,9 +88,9 @@ struct settings { struct keyboard_shortcut context_ks; bool force_xinerama; int corner_radius; - enum mouse_action mouse_left_click; - enum mouse_action mouse_middle_click; - enum mouse_action mouse_right_click; + enum mouse_action *mouse_left_click; + enum mouse_action *mouse_middle_click; + enum mouse_action *mouse_right_click; }; extern struct settings settings; diff --git a/src/x11/x.c b/src/x11/x.c index 0604735..6595d28 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -400,49 +400,51 @@ bool x_is_idle(void) */ static void x_handle_click(XEvent ev) { - enum mouse_action act; + enum mouse_action *acts; switch (ev.xbutton.button) { case Button1: - act = settings.mouse_left_click; + acts = settings.mouse_left_click; break; case Button2: - act = settings.mouse_middle_click; + acts = settings.mouse_middle_click; break; case Button3: - act = settings.mouse_right_click; + acts = settings.mouse_right_click; break; default: LOG_W("Unsupported mouse button: '%d'", ev.xbutton.button); return; } - if (act == MOUSE_CLOSE_ALL) { - queues_history_push_all(); - - return; - } - - if (act == MOUSE_DO_ACTION || act == MOUSE_CLOSE_CURRENT) { - int y = settings.separator_height; - struct notification *n = NULL; - int first = true; - for (const GList *iter = queues_get_displayed(); iter; - iter = iter->next) { - n = iter->data; - if (ev.xbutton.y > y && ev.xbutton.y < y + n->displayed_height) - break; - - y += n->displayed_height + settings.separator_height; - if (first) - y += settings.frame_width; + for (int i = 0; acts[i]; i++) { + enum mouse_action act = acts[i]; + if (act == MOUSE_CLOSE_ALL) { + queues_history_push_all(); + return; } - if (n) { - if (act == MOUSE_CLOSE_CURRENT) - queues_notification_close(n, REASON_USER); - else - notification_do_action(n); + if (act == MOUSE_DO_ACTION || act == MOUSE_CLOSE_CURRENT) { + int y = settings.separator_height; + struct notification *n = NULL; + int first = true; + for (const GList *iter = queues_get_displayed(); iter; + iter = iter->next) { + n = iter->data; + if (ev.xbutton.y > y && ev.xbutton.y < y + n->displayed_height) + break; + + y += n->displayed_height + settings.separator_height; + if (first) + y += settings.frame_width; + } + + if (n) { + if (act == MOUSE_CLOSE_CURRENT) + queues_notification_close(n, REASON_USER); + else + notification_do_action(n); + } } } } From a4a1a1ac9ee33b3e553958410a3fb539c2ff7cb8 Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Sun, 5 Apr 2020 03:25:05 -0400 Subject: [PATCH 3/5] Add tests for parsing lists from conf/cmdline. --- test/data/test-ini | 10 +++++ test/option_parser.c | 93 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/test/data/test-ini b/test/data/test-ini index 6201cd7..639c05c 100644 --- a/test/data/test-ini +++ b/test/data/test-ini @@ -27,6 +27,16 @@ unquoted_comment = String with a # comment color_comment = "#ffffff" # comment +[list] + simple = A,simple,list + spaces = A, list, with, spaces + multiword = A list, with, multiword entries + quoted = "A, quoted, list" + quoted_with_quotes = "A, list, "with quotes"" + unquoted_with_quotes = A, list, "with quotes" + quoted_comment = "List, with, a" # comment + unquoted_comment = List, with, a # comment + [path] expand_tilde = ~/.path/to/tilde diff --git a/test/option_parser.c b/test/option_parser.c index f399c26..b058706 100644 --- a/test/option_parser.c +++ b/test/option_parser.c @@ -8,6 +8,7 @@ TEST test_next_section(void) const char *section = NULL; ASSERT_STR_EQ("bool", (section = next_section(section))); ASSERT_STR_EQ("string", (section = next_section(section))); + ASSERT_STR_EQ("list", (section = next_section(section))); ASSERT_STR_EQ("path", (section = next_section(section))); ASSERT_STR_EQ("int", (section = next_section(section))); ASSERT_STR_EQ("double", (section = next_section(section))); @@ -68,6 +69,55 @@ TEST test_ini_get_string(void) PASS(); } +enum greatest_test_res ARRAY_EQ(char **a, char **b){ + ASSERT(a); + ASSERT(b); + int i = 0; + while (a[i] && b[i]){ + ASSERT_STR_EQ(a[i], b[i]); + i++; + } + ASSERT_FALSE(a[i]); + ASSERT_FALSE(b[i]); + PASS(); +} + +TEST test_ini_get_list(void) +{ + char *list_section = "list"; + + char *cmp1[] = {"A", "simple", "list", NULL}; + char *cmp2[] = {"A", "list", "with", "spaces", NULL}; + char *cmp3[] = {"A list", "with", "multiword entries", NULL}; + char *cmp4[] = {"A", "quoted", "list", NULL}; + char *cmp5[] = {"A", "list", "\"with quotes\"", NULL}; + char *cmp6[] = {"List", "with", "a", NULL}; + + char **ptr; + CHECK_CALL(ARRAY_EQ(cmp1, (ptr = ini_get_list(list_section, "simple", "")))); + free_string_array(ptr); + + CHECK_CALL(ARRAY_EQ(cmp2, (ptr = ini_get_list(list_section, "spaces", "")))); + free_string_array(ptr); + + CHECK_CALL(ARRAY_EQ(cmp3, (ptr = ini_get_list(list_section, "multiword", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp4, (ptr = ini_get_list(list_section, "quoted", "")))); + free_string_array(ptr); + + CHECK_CALL(ARRAY_EQ(cmp5, (ptr = ini_get_list(list_section, "quoted_with_quotes", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp5, (ptr = ini_get_list(list_section, "unquoted_with_quotes", "")))); + free_string_array(ptr); + + CHECK_CALL(ARRAY_EQ(cmp6, (ptr = ini_get_list(list_section, "quoted_comment", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp6, (ptr = ini_get_list(list_section, "unquoted_comment", "")))); + free_string_array(ptr); + + PASS(); +} + TEST test_ini_get_path(void) { char *section = "path"; @@ -151,6 +201,22 @@ TEST test_cmdline_get_string(void) PASS(); } +TEST test_cmdline_get_list(void) +{ + char **ptr; + char *cmp1[] = {"A", "simple", "list", "from", "the", "cmdline", NULL}; + char *cmp2[] = {"A", "list", "with", "spaces", NULL}; + char *cmp3[] = {"A", "default", "list", NULL}; + + CHECK_CALL(ARRAY_EQ(cmp1, (ptr = cmdline_get_list("-list", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp2, (ptr = cmdline_get_list("-list2", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp3, (ptr = cmdline_get_list("-nonexistent", "A, default, list", "")))); + free_string_array(ptr); + PASS(); +} + TEST test_cmdline_get_int(void) { ASSERT_EQ(3, cmdline_get_int("-int", 0, "")); @@ -221,6 +287,29 @@ TEST test_option_get_string(void) PASS(); } +TEST test_option_get_list(void) +{ + char *list_section = "list"; + char **ptr; + + char *cmp1[] = {"A", "simple", "list", NULL}; + char *cmp2[] = {"A", "list", "with", "spaces", NULL}; + char *cmp3[] = {"A", "simple", "list", "from", "the", "cmdline", NULL}; + char *cmp4[] = {"A", "default", "list", NULL}; + + CHECK_CALL(ARRAY_EQ(cmp1, (ptr = option_get_list(list_section, "simple", "-nonexistent", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp2, (ptr = option_get_list(list_section, "quoted", "-list2", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp3, (ptr = option_get_list(list_section, "simple", "-list", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp3, (ptr = option_get_list(list_section, "simple", "-list/-l", "", "")))); + free_string_array(ptr); + CHECK_CALL(ARRAY_EQ(cmp4, (ptr = option_get_list(list_section, "nonexistent", "-nonexistent", "A, default, list", "")))); + free_string_array(ptr); + PASS(); +} + TEST test_option_get_path(void) { char *section = "path"; @@ -308,11 +397,13 @@ SUITE(suite_option_parser) RUN_TEST(test_next_section); RUN_TEST(test_ini_get_bool); RUN_TEST(test_ini_get_string); + RUN_TEST(test_ini_get_list); RUN_TEST(test_ini_get_path); RUN_TEST(test_ini_get_int); RUN_TEST(test_ini_get_double); char cmdline[] = "dunst -bool -b " "-string \"A simple string from the cmdline\" -s Single_word_string " + "-list A,simple,list,from,the,cmdline -list2 \"A, list, with, spaces\" " "-int 3 -i 2 -negative -7 -zeroes 04 -intdecim 2.5 " "-path ~/path/from/cmdline " "-simple_double 2 -double 5.2" @@ -322,6 +413,7 @@ SUITE(suite_option_parser) g_shell_parse_argv(&cmdline[0], &argc, &argv, NULL); cmdline_load(argc, argv); RUN_TEST(test_cmdline_get_string); + RUN_TEST(test_cmdline_get_list); RUN_TEST(test_cmdline_get_path); RUN_TEST(test_cmdline_get_int); RUN_TEST(test_cmdline_get_double); @@ -329,6 +421,7 @@ SUITE(suite_option_parser) RUN_TEST(test_cmdline_create_usage); RUN_TEST(test_option_get_string); + RUN_TEST(test_option_get_list); RUN_TEST(test_option_get_path); RUN_TEST(test_option_get_int); RUN_TEST(test_option_get_double); From 8bcc3070fb726e9da064ad885c9ff3634b4c9ee1 Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Sun, 5 Apr 2020 19:35:20 -0400 Subject: [PATCH 4/5] Use g_strsplit instead of homebrewing a solution Lesson: don't reinvent the wheel. Additionally, changed a `g_malloc` call to `g_malloc_n` for safety. --- src/option_parser.c | 2 +- src/utils.c | 14 +++----------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/option_parser.c b/src/option_parser.c index c958db3..7fa307c 100644 --- a/src/option_parser.c +++ b/src/option_parser.c @@ -150,7 +150,7 @@ bool string_parse_mouse_action_list(char **s, enum mouse_action **ret) while (s[len]) len++; - *ret = g_malloc((len + 1) * sizeof(enum mouse_action)); + *ret = g_malloc_n((len + 1), sizeof(enum mouse_action)); for (int i = 0; i < len; i++) { if (!string_parse_mouse_action(s[i], *ret + i)) { LOG_W("Unknown mouse action value: '%s'", s[i]); diff --git a/src/utils.c b/src/utils.c index d837774..64beed9 100644 --- a/src/utils.c +++ b/src/utils.c @@ -151,18 +151,10 @@ char **string_to_array(const char *string) { char **arr = NULL; if (string) { - char* dup = g_strdup(string); - char* tmp = dup; - int num_tokens = 0; - char *token = strsep(&tmp, ","); - while (token) { - arr = g_realloc_n(arr, num_tokens + 2, sizeof(char*)); - arr[num_tokens] = g_strdup(g_strstrip(token)); - num_tokens++; - token = strsep(&tmp, ","); + arr = g_strsplit(string, ",", 0); + for (int i = 0; arr[i]; i++){ + g_strstrip(arr[i]); } - arr[num_tokens] = NULL; - g_free(dup); } return arr; } From 92a21487b2cf2a91c84325a44df99a4386c71432 Mon Sep 17 00:00:00 2001 From: Michael Krasnitski Date: Sun, 5 Apr 2020 21:45:56 -0400 Subject: [PATCH 5/5] Make some changes to default dunstrc These make it more clear to the user that the mouse_click options can be defined as lists. --- dunstrc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dunstrc b/dunstrc index c226e9a..9718191 100644 --- a/dunstrc +++ b/dunstrc @@ -238,15 +238,17 @@ ### mouse - # Defines action of mouse event + # Defines list of actions for each mouse event # Possible values are: # * none: Don't do anything. # * do_action: If the notification has exactly one action, or one is marked as default, # invoke it. If there are multiple and no default, open the context menu. # * close_current: Close current notification. # * close_all: Close all notifications. + # These values can be strung together for each mouse event, and + # will be executed in sequence. mouse_left_click = close_current - mouse_middle_click = do_action + mouse_middle_click = do_action, close_current mouse_right_click = close_all # Experimental features that may or may not work correctly. Do not expect them