rcxml: refactor mousebind implementation

include/config/mousebind.h

@@ -4,30 +4,27 @@
 #include "ssd.h"
 #include <wayland-util.h>
 
-enum action_mouse_did
+enum mouse_event {
-{
+	MOUSE_ACTION_NONE = 0,
 	MOUSE_ACTION_DOUBLECLICK,
-	MOUSE_ACTION_NONE
 };
 
 struct mousebind {
-	enum ssd_part_type context; /* ex: titlebar */
+	enum ssd_part_type context;
 
 	/* ex: BTN_LEFT, BTN_RIGHT from linux/input_event_codes.h */
 	uint32_t button;
 
-	/* ex: doubleclick, press, drag, etc */
+	/* ex: doubleclick, press, drag */
-	enum action_mouse_did mouse_action;
+	enum mouse_event mouse_event;
-
-	/* what to do because mouse did previous action */
 	const char *action;
-
 	const char *command;
-	struct wl_list link;
+
+	struct wl_list link; /* rcxml::mousebinds */
 };
 
-struct mousebind *
+enum mouse_event mousebind_event_from_str(const char *str);
-mousebind_create(const char *context_str, const char *mouse_button_str,
+uint32_t mousebind_button_from_str(const char *str);
-    const char *action_mouse_did_str, const char *action, const char *command);
+struct mousebind *mousebind_create(const char *context);
 
 #endif /* __LABWC_MOUSEBIND_H */

src/config/mousebind.c

@@ -1,91 +1,60 @@
 #define _POSIX_C_SOURCE 200809L
-#include "config/mousebind.h"
+#include <assert.h>
-#include "config/rcxml.h"
 #include <linux/input-event-codes.h>
 #include <strings.h>
 #include <unistd.h>
 #include <wlr/util/log.h>
+#include "config/mousebind.h"
+#include "config/rcxml.h"
+
+uint32_t
+mousebind_button_from_str(const char *str)
+{
+	assert(str);
+	if (!strcasecmp(str, "Left")) {
+		return BTN_LEFT;
+	} else if (!strcasecmp(str, "Right")) {
+		return BTN_RIGHT;
+	} else if (!strcasecmp(str, "Middle")) {
+		return BTN_MIDDLE;
+	} else {
+		wlr_log(WLR_ERROR, "unknown button (%s)", str);
+		return UINT32_MAX;
+	}
+}
+
+enum mouse_event
+mousebind_event_from_str(const char *str)
+{
+	assert(str);
+	if (strcasecmp(str, "doubleclick") == 0) {
+		return MOUSE_ACTION_DOUBLECLICK;
+	} else {
+		wlr_log(WLR_ERROR, "unknown mouse action (%s)", str);
+		return MOUSE_ACTION_NONE;
+	}
+}
 
 static enum ssd_part_type
 context_from_str(const char *str)
 {
-	if (str == NULL) {
+	if (!strcasecmp(str, "Titlebar")) {
-		return LAB_SSD_NONE;
-	} else if (strcasecmp(str, "Titlebar") == 0) {
 		return LAB_SSD_PART_TITLEBAR;
 	} else {
+		wlr_log(WLR_ERROR, "unknown mouse context (%s)", str);
 		return LAB_SSD_NONE;
 	}
 }
 
-static uint32_t
-mouse_button_from_str(const char *str)
-{
-	if (str == NULL) {
-		return UINT32_MAX;
-	} else if (strcasecmp(str, "Left") == 0) {
-		return BTN_LEFT;
-	} else if (strcasecmp(str, "Right") == 0) {
-		return BTN_RIGHT;
-	} else if (strcasecmp(str, "Middle") == 0) {
-		return BTN_MIDDLE;
-	} else {
-		return UINT32_MAX;
-	}
-}
-
-static enum action_mouse_did
-action_mouse_did_from_str(const char *str)
-{
-	if (str == NULL) {
-		return MOUSE_ACTION_NONE;
-	} else if (strcasecmp(str, "doubleclick") == 0) {
-		return MOUSE_ACTION_DOUBLECLICK;
-	} else {
-		return MOUSE_ACTION_NONE;
-	}
-}
-
 struct mousebind *
-mousebind_create(const char *context_str, const char *mouse_button_str,
+mousebind_create(const char *context)
-    const char *action_mouse_did_str, const char *action, const char *command)
 {
-	struct mousebind *m = calloc(1, sizeof(struct mousebind));
+	if (!context) {
-
+		wlr_log(WLR_ERROR, "mousebind context not specified");
-	enum ssd_part_type context = context_from_str(context_str);
-	uint32_t button = mouse_button_from_str(mouse_button_str);
-	enum action_mouse_did action_mouse_did =
-	    action_mouse_did_from_str(action_mouse_did_str);
-
-	if (context == LAB_SSD_NONE) {
-		wlr_log(WLR_ERROR, "unknown mouse context (%s)", context_str);
-		goto CREATE_ERROR;
-	}
-	if (button == UINT32_MAX) {
-		wlr_log(WLR_ERROR, "unknown button (%s)", mouse_button_str);
-		goto CREATE_ERROR;
-	}
-	if (action_mouse_did == MOUSE_ACTION_NONE) {
-		wlr_log(WLR_ERROR, "unknown mouse action (%s)",
-		    action_mouse_did_str);
-		goto CREATE_ERROR;
-	}
-	if (action == NULL) {
-		wlr_log(WLR_ERROR, "action is NULL\n");
-		goto CREATE_ERROR;
-	}
-
-	m->context = context;
-	m->button = button;
-	m->mouse_action = action_mouse_did;
-	m->action = strdup(action); /* TODO: replace with strndup? */
-	if (command && !strcasecmp(action, "Execute")) {
-		m->command = strdup(command);
-	}
-
-	return m;
-
-CREATE_ERROR:
-	free(m);
 		return NULL;
 	}
+	struct mousebind *m = calloc(1, sizeof(struct mousebind));
+	m->context = context_from_str(context);
+	wl_list_insert(&rc.mousebinds, &m->link);
+	return m;
+}

src/config/rcxml.c

@@ -1,11 +1,4 @@
 #define _POSIX_C_SOURCE 200809L
-#include "config/rcxml.h"
-#include "common/dir.h"
-#include "common/nodename.h"
-#include "common/string-helpers.h"
-#include "common/zfree.h"
-#include "config/keybind.h"
-#include "config/mousebind.h"
 #include <assert.h>
 #include <ctype.h>
 #include <errno.h>
@@ -19,12 +12,20 @@
 #include <unistd.h>
 #include <wayland-server-core.h>
 #include <wlr/util/log.h>
+#include "common/dir.h"
+#include "common/nodename.h"
+#include "common/string-helpers.h"
+#include "common/zfree.h"
+#include "config/keybind.h"
+#include "config/mousebind.h"
+#include "config/rcxml.h"
 
 static bool in_keybind = false;
 static bool in_mousebind = false;
 static bool is_attribute = false;
 static struct keybind *current_keybind;
-static const char *current_mouse_context = "";
+static struct mousebind *current_mousebind;
+static const char *current_mouse_context;
 
 enum font_place
 {
@@ -34,28 +35,7 @@ enum font_place
 	/* TODO: Add all places based on Openbox's rc.xml */
 };
 
-/*
+static void load_default_key_bindings(void);
- * unchecked mousebind params. we fill these out one at a time, then pass them
- * all to mousebind_create() once we are ready
- */
-static const char *current_mouse_button = "";
-static const char *current_action_mouse_did = "";
-struct mouse_action {
-	const char *action;
-	const char *command;
-};
-/*
- * A given mousebind can have multiple actions associated with it.
- * This array is a list of the actions for the currently-being-parsed mousebind
- *
- * TODO: make it a linked list?
- */
-#define MAX_MOUSE_ACTIONS 32
-static struct mouse_action mouse_actions[MAX_MOUSE_ACTIONS] = {{0}};
-static int num_mouse_actions = 0;
-
-static void
-load_default_key_bindings(void);
 
 static void
 fill_keybind(char *nodename, char *content)
@@ -86,71 +66,36 @@ fill_keybind(char *nodename, char *content)
 	}
 }
 
-static void
-add_new_mousebinds(void)
-{
-	for (int i = 0; i < num_mouse_actions; i++) {
-		struct mousebind *m = mousebind_create(current_mouse_context,
-		    current_mouse_button, current_action_mouse_did,
-		    mouse_actions[i].action, mouse_actions[i].command);
-		if (m != NULL) {
-			wl_list_insert(&rc.mousebinds, &m->link);
-		} else {
-			wlr_log(WLR_ERROR,
-			    "failed to create mousebind\n"
-			    "    context: (%s)\n"
-			    "    button: (%s)\n"
-			    "    mouse action (%s)\n"
-			    "    action (%s)\n"
-			    "    command: (%s)\n",
-			    current_mouse_context, current_mouse_button,
-			    current_action_mouse_did, mouse_actions[i].action,
-			    mouse_actions[i].command);
-		}
-	}
-
-	num_mouse_actions = 0;
-	memset(
-	    mouse_actions, 0, sizeof(struct mouse_action) * MAX_MOUSE_ACTIONS);
-}
-
 static void
 fill_mousebind(char *nodename, char *content)
 {
 	/*
-	 * Example of what we're parsing:
+	 * Example of what we are parsing:
-	 *
 	 * <mousebind button="Left" action="DoubleClick">
 	 *   <action name="ToggleMaximize"/>
 	 * </mousebind>
 	 */
+
+	if (!strcmp(nodename, "mousebind.context.mouse")) {
+		wlr_log(WLR_INFO, "create mousebind for %s",
+			current_mouse_context);
+		current_mousebind = mousebind_create(current_mouse_context);
+	}
 	if (!content) {
 		return;
 	}
-
 	string_truncate_at_pattern(nodename, ".mousebind.context.mouse");
 
-	if (is_attribute && !strcmp(nodename, "button")) {
+	if (!strcmp(nodename, "button")) {
-		current_mouse_button = content;
+		current_mousebind->button = mousebind_button_from_str(content);
 	} else if (!strcmp(nodename, "action")) {
-		/*
+		 /* <mousebind button="" action="EVENT"> */
-		 * checking for is_attribute fails even though we are looking
+		current_mousebind->mouse_event =
-		 * for the attribute of mousebind named action. initial thoughts
+			mousebind_event_from_str(content);
-		 * were to check for is_attribute to distinguish the attribute
+	} else if (!strcmp(nodename, "name.action")) {
-		 * of mousebind named action from the child of mousebind named
+		current_mousebind->action = strdup(content);
-		 * action. since the child of mousebind named action doesn't
-		 * have any content, I don't think we need to make this
-		 * distinction since we already filtered out nodes that don't
-		 * have content
-		 */
-		current_action_mouse_did = content;
-	} else if (is_attribute && !strcmp(nodename, "name.action")) {
-		if (num_mouse_actions < MAX_MOUSE_ACTIONS) {
-			num_mouse_actions++;
-			mouse_actions[num_mouse_actions - 1].action = content;
-		}
 	} else if (!strcmp(nodename, "command.action")) {
-		mouse_actions[num_mouse_actions - 1].command = content;
+		current_mousebind->command = strdup(content);
 	}
 }
 
@@ -246,6 +191,13 @@ entry(xmlNode *node, char *nodename, char *content)
 		printf("%s: %s\n", nodename, content);
 	}
 
+	if (in_keybind) {
+		fill_keybind(nodename, content);
+	}
+	if (in_mousebind) {
+		fill_mousebind(nodename, content);
+	}
+
 	/* handle nodes without content, e.g. <keyboard><default /> */
 	if (!strcmp(nodename, "default.keyboard")) {
 		load_default_key_bindings();
@@ -256,14 +208,6 @@ entry(xmlNode *node, char *nodename, char *content)
 	if (!content) {
 		return;
 	}
-	if (in_keybind) {
-		fill_keybind(nodename, content);
-	}
-
-	if (in_mousebind) {
-		fill_mousebind(nodename, content);
-	}
-
 	if (is_attribute && !strcmp(nodename, "place.font.theme")) {
 		font_place = enum_font_place(content);
 	}
@@ -328,8 +272,7 @@ process_node(xmlNode *node)
 	entry(node, name, content);
 }
 
-static void
+static void xml_tree_walk(xmlNode *node);
-xml_tree_walk(xmlNode *node);
 
 static void
 traverse(xmlNode *n)
@@ -360,7 +303,6 @@ xml_tree_walk(xmlNode *node)
 			in_mousebind = true;
 			traverse(n);
 			in_mousebind = false;
-			add_new_mousebinds();
 			continue;
 		}
 		traverse(n);
@@ -522,8 +464,7 @@ rcxml_finish(void)
 	zfree(rc.theme_name);
 
 	struct keybind *k, *k_tmp;
-	wl_list_for_each_safe(k, k_tmp, &rc.keybinds, link)
+	wl_list_for_each_safe(k, k_tmp, &rc.keybinds, link) {
-	{
 		wl_list_remove(&k->link);
 		zfree(k->command);
 		zfree(k->action);
@@ -532,8 +473,7 @@ rcxml_finish(void)
 	}
 
 	struct mousebind *m, *m_tmp;
-	wl_list_for_each_safe(m, m_tmp, &rc.mousebinds, link)
+	wl_list_for_each_safe(m, m_tmp, &rc.mousebinds, link) {
-	{
 		wl_list_remove(&m->link);
 		zfree(m->command);
 		zfree(m->action);

src/cursor.c

@@ -346,8 +346,9 @@ cursor_button(struct wl_listener *listener, void *data)
 	if (is_double_click(rc.doubleclick_time) && view_area == LAB_SSD_PART_TITLEBAR) {
 		struct mousebind* mousebind;
 		wl_list_for_each_reverse(mousebind, &rc.mousebinds, link) {
+			/* TODO: make this more generic */
 			if( (mousebind->context == LAB_SSD_PART_TITLEBAR) &&
-				(mousebind->mouse_action == MOUSE_ACTION_DOUBLECLICK) &&
+				(mousebind->mouse_event == MOUSE_ACTION_DOUBLECLICK) &&
 				(mousebind->button == event->button) ) {
 				action(server, mousebind->action, mousebind->command);
 			}