Allow undefined actions in keys configuration file

In the keys file there are three possibilities for each action:

1. One or several keys are assigned to it
2. It is marked as UNDEFINED (new)
3. It is missing from the file

On load of the keys file, calcurse respectively

1. Assigns the key(s)
2. Assigns "UNDEFINED" (new)
3. Assigns a default key if possible

If default keys were assigned, the user is informed of the number of
actions affected, and the keys file is updated.

After load each action must either have keys assigned or be undefined.
If not, calcurse exits with a failure. If there are syntax/semantic
errors in the file, calcurse rejects the file and exits.

When an interactive user leaves the keys configuration menu, a warning
is issued if any action is UNDEFINED. The keys file is always updated.

Addresses GitHub issue #298.

Additionally: Description of concepts and data structures used for
keyboard keys and virtual keys (actions) as well as name changes and
comments to improve readability.

Signed-off-by: Lars Henriksen <LarsHenriksen@get2net.dk>
Signed-off-by: Lukas Fleischer <lfleischer@calcurse.org>

1 files changed, 214 insertions, 116 deletions

diff --git a/src/keys.c b/src/keys.c
index 3a25a0b..a92c7bc 100644
--- a/src/keys.c
+++ b/src/keys.c
@@ -39,26 +39,73 @@
 
 #include "calcurse.h"
 
-#define MAXKEYVAL   KEY_MAX	/* ncurses defines KEY_MAX as maximum key value */
-
-struct keydef_s {
-	const char *label;
-	const char *binding;
-	const char *sb_label;
-};
+/*
+ * The interactive calcurse interface is controlled by "virtual keys", aka
+ * actions or commands. The virtual keys are defined by the type 'enum vkey',
+ * see calcurse.h. To each virtual key is assigned (or bound) zero or more
+ * keyboard keys/characters. A character (generated by a keyboard key) may be
+ * either an ordinary character or a pseudo-character. [An ordinary character
+ * is either a singlebyte ASCII character or a multibyte, UTF-8 encoded
+ * character; a pseudo-character (as supported by curses) is an escape sequence
+ * generated by a key.] A keyboard key/character is uniquely identified by its
+ * keyname (a character string) or by an integer (the Unicode code point of the
+ * character with a slight modification to accomodate the range of curses
+ * pseudo-characters). Mapping between the two forms is performed by the
+ * functions keys_str2int() and keys_int2str().
+ */
 
-static llist_t keys[NBKEYS];
-static enum key actions[MAXKEYVAL];
+/*
+ * Assignment of keys to virtual keys is held in the tabel keys[]. The entry for
+ * each virtual key is a linked list of keyboard keys (bindings); each list
+ * element is the keyname as returned by keys_int2str().
+ *
+ * At the very first run default keys are assigned to all virtual keys from a
+ * built-in table keydef[] and saved to disk in the calcurse config directory.
+ * Later the user may edit the key configuration and change the key bindings by
+ * adding/removing keys. If all keys are removed, the virtual key is left
+ * undefined. This state is also saved in the configuration file. The linked
+ * list for an undefined virtual key contains a single element with a null
+ * pointer as data.
+ */
+static llist_t keys[NBVKEYS];
 
+/*
+ * To cater for the other direction (which virtual key is a keyboard key
+ * assigned to), two constructions are needed: a table actions[] for the
+ * keyboard keys in the curses range, and a linked list actions_ext for
+ * multi-byte UTF-8 encoded keyboard characters.
+ *
+ * For each keyboard key (integer) in the curses key range, the virtual key
+ * (action) it is assigned to or, if not assigned, KEY_UNDEF.
+ */
+static enum vkey actions[KEY_MAX];
+/*
+ * For the millions of possible keyboard keys above the curses range, a linked
+ * list of keys which are actually bound to a virtual key.
+ * Each list element is a key_ext structure.
+ */
+llist_t actions_ext;
 struct key_ext {
-	int ch;
-	enum key action;
+	int key;
+	enum vkey action;
 };
 
-llist_t actions_ext;
+/*
+ * Assigning a keyboard key to a virtual key is accomplished by
+ * 1) either inserting the virtual key in the actions[] entry for the keyboard key
+ *    or adding the pair (key, virtual key) to the list actions_ext
+ * 2) adding it in keys[] to the list for the virtual key
+ * See keys_assign_binding() below.
+ */
 
+/* The default key bindings for the virtual keys. */
+struct keydef_s {
+	const char *label;	/* Name of the virtual key (action). */
+	const char *binding;	/* String of space-separated keynames bound to it. */
+	const char *sb_label;	/* Display name in the status bar menu. */
+};
 #define gettext_noop(s) s
-static struct keydef_s keydef[NBKEYS] = {
+static struct keydef_s keydef[NBVKEYS] = {
 	{ "generic-cancel", "ESC", gettext_noop("Cancel") },
 	{ "generic-select", "SPC", gettext_noop("Select") },
 	{ "generic-credits", "@", gettext_noop("Credits") },
@@ -132,10 +179,11 @@ void keys_init(void)
 	int i;
 	const char *cp;
 
-	for (i = 0; i < MAXKEYVAL; i++)
+	/* All keys unassigned. */
+	for (i = 0; i < KEY_MAX; i++)
 		actions[i] = KEY_UNDEF;
 	LLIST_INIT(&actions_ext);
-	for (i = 0; i < NBKEYS; i++)
+	for (i = 0; i < NBVKEYS; i++)
 		LLIST_INIT(&keys[i]);
 
 	/* Initialization of the keynames table. */
@@ -146,7 +194,7 @@ void keys_init(void)
 	for (i = 1; i < 128; i++)
 		if ((cp = keyname(i)))
 			keynames[i] = mem_strdup(cp);
-	/* ... and for the ncurses escape keys (pseudokeys). */
+	/* ... and for the ncurses pseudo-characters. */
 	for (i = KEY_MIN; i < KEY_MAX; i++)
 		if ((cp = keyname(i)))
 			keynames[i] = mem_strdup(cp);
@@ -189,7 +237,7 @@ void keys_free(void)
 {
 	int i;
 
-	for (i = 0; i < NBKEYS; i++) {
+	for (i = 0; i < NBVKEYS; i++) {
 		LLIST_FREE_INNER(&keys[i], key_free);
 		LLIST_FREE(&keys[i]);
 	}
@@ -205,31 +253,40 @@ void keys_dump_defaults(char *file)
 		_("FATAL ERROR: could not create default keys file."));
 
 	dump_intro(fd);
-	for (i = 0; i < NBKEYS; i++)
+	for (i = 0; i < NBVKEYS; i++)
 		fprintf(fd, "%s  %s\n", keydef[i].label,
 			keydef[i].binding);
 	file_close(fd, __FILE_POS__);
 }
 
-const char *keys_get_label(enum key key)
+const char *keys_get_label(enum vkey key)
 {
 	EXIT_IF(key < 0
-		|| key > NBKEYS,
+		|| key > NBVKEYS,
 		_("FATAL ERROR: key value out of bounds"));
 
 	return keydef[key].label;
 }
 
+const char *keys_get_binding(enum vkey key)
+{
+	EXIT_IF(key < 0
+		|| key > NBVKEYS,
+		_("FATAL ERROR: key value out of bounds"));
+
+	return keydef[key].binding;
+}
+
 static int key_ext_hasch(struct key_ext *k, void *cbdata)
 {
-	return (k->ch == *((int *)cbdata));
+	return (k->key == *((int *)cbdata));
 }
 
-enum key keys_get_action(int pressed)
+enum vkey keys_get_action(int pressed)
 {
 	if (pressed < 0) {
 		return -1;
-	} else if (pressed > MAXKEYVAL) {
+	} else if (pressed > KEY_MAX) {
 		llist_item_t *i = LLIST_FIND_FIRST(&actions_ext, &pressed,
 						   key_ext_hasch);
 		if (!i)
@@ -274,7 +331,7 @@ void keys_wait_for_any_key(WINDOW *win)
 	keys_wgetch(win);
 }
 
-enum key keys_get(WINDOW *win, int *count, int *reg)
+enum vkey keys_get(WINDOW *win, int *count, int *reg)
 {
 	int ch = '0';
 
@@ -312,60 +369,89 @@ enum key keys_get(WINDOW *win, int *count, int *reg)
 	}
 }
 
-static void add_key_str(enum key action, int key)
+static void add_if_undefined(enum vkey action)
 {
-	if (action > NBKEYS)
-		return;
+	/* If list is empty, mark action as UNDEFINED. */
+	if (!keys[action].head)
+		LLIST_ADD(&keys[action], NULL);
+}
 
-	LLIST_ADD(&keys[action], keys_int2str(key));
+static void del_if_undefined(enum vkey action)
+{
+	/* Action UNDEFINED? */
+	if (!LLIST_GET_DATA(LLIST_FIRST(&keys[action])))
+		LLIST_REMOVE(&keys[action], keys[action].head);
 }
 
-int keys_assign_binding(int key, enum key action)
+static void free_key_str(char *str)
 {
-	if (key < 0)
-		return 1;
-	if (key > KEY_MAX) {
-		llist_item_t *i = LLIST_FIND_FIRST(&actions_ext, &key, key_ext_hasch);
-		if (i)
-			return 1;
-		struct key_ext *k = mem_malloc(sizeof(struct key_ext));
-		k->ch = key;
-		k->action = action;
-		LLIST_ADD(&actions_ext, k);
-	} else {
-		if (actions[key] != KEY_UNDEF)
-			return 1;
-		actions[key] = action;
-	}
-	add_key_str(action, key);
-	return 0;
+	mem_free(str);
 }
 
-static void del_key_str(enum key action, int key)
+static void add_key_str(enum vkey action, int key)
+{
+	if (action > NBVKEYS)
+		return;
+
+	del_if_undefined(action);
+	LLIST_ADD(&keys[action], keys_int2str(key));
+}
+
+static void del_key_str(enum vkey action, int key)
 {
 	llist_item_t *i;
-	char *oldstr = keys_int2str(key);;
+	char *oldstr = keys_int2str(key), *j;
 
-	if (action > NBKEYS)
+	if (action > NBVKEYS)
 		return;
 
 	LLIST_FOREACH(&keys[action], i) {
-		if (strcmp(LLIST_GET_DATA(i), oldstr) == 0) {
+		if (strcmp((j = LLIST_GET_DATA(i)), oldstr) == 0) {
 			LLIST_REMOVE(&keys[action], i);
+			free_key_str(j);
 			goto cleanup;
 		}
 	}
 
 cleanup:
+	add_if_undefined(action);
 	mem_free(oldstr);
 }
 
-void keys_remove_binding(int key, enum key action)
+/*
+ * Assign keyboard key "key" to virtual key "action" by
+ *
+ * - marking keyboard key "key" as used for virtual key "actual"
+ * - adding "key" to the list of assigned keys for "action" in the tabel keys[]
+ *
+ * The former is done by either inserting "action" in the "key" entry of tabel
+ * actions[], or for keys above the curses range, inserting (key, action) in the
+ * list actions_ext.
+ */
+int keys_assign_binding(int key, enum vkey action)
+{
+	if (key > KEY_MAX) {
+		if (LLIST_FIND_FIRST(&actions_ext, &key, key_ext_hasch))
+			return 1;
+		struct key_ext *k = mem_malloc(sizeof(struct key_ext));
+		k->key = key;
+		k->action = action;
+		LLIST_ADD(&actions_ext, k);
+	} else if (key > -1) {
+		if (actions[key] != KEY_UNDEF)
+			return 1;
+		actions[key] = action;
+	}
+	add_key_str(action, key);
+	return 0;
+}
+
+void keys_remove_binding(int key, enum vkey action)
 {
 	if (key < 0)
 	       return;
 
-	if (key <= MAXKEYVAL) {
+	if (key <= KEY_MAX) {
 		actions[key] = KEY_UNDEF;
 	} else {
 		llist_item_t *i = LLIST_FIND_FIRST(&actions_ext, &key,
@@ -410,6 +496,8 @@ char *keys_int2str(int key)
 {
 	char *res;
 
+	if (key == -1)
+		return NULL;
 	if (key < KEY_MAX) {
 		if (strcmp(keynames[key], "") == 0)
 			return NULL;
@@ -421,50 +509,44 @@ char *keys_int2str(int key)
 	}
 }
 
-int keys_action_count_keys(enum key action)
+int keys_action_count_keys(enum vkey action)
 {
 	llist_item_t *i;
 	int n = 0;
 
+	/* Action UNDEFINED? */
+	if (!LLIST_GET_DATA(LLIST_FIRST(&keys[action])))
+		return 0;
+
 	LLIST_FOREACH(&keys[action], i)
-	    n++;
+		n++;
 
 	return n;
 }
 
-const char *keys_action_firstkey(enum key action)
+const char *keys_action_firstkey(enum vkey action)
 {
 	const char *s = LLIST_GET_DATA(LLIST_FIRST(&keys[action]));
 	return (s != NULL) ? s : "XXX";
 }
 
-const char *keys_action_nkey(enum key action, int keynum)
+const char *keys_action_nkey(enum vkey action, int keynum)
 {
 	return LLIST_GET_DATA(LLIST_NTH(&keys[action], keynum));
 }
 
-char *keys_action_allkeys(enum key action)
+char *keys_action_allkeys(enum vkey action)
 {
 	llist_item_t *i;
-	static char keystr[BUFSIZ];
-	int keystrlen = 0;
-	int entrylen;
-
-	if (!LLIST_FIRST(&keys[action]))
-		return NULL;
-
-	keystr[0] = '\0';
-	LLIST_FOREACH(&keys[action], i) {
-		entrylen = strlen(LLIST_GET_DATA(i)) + 1;
-		if (keystrlen + entrylen >= BUFSIZ)
-			break;
-		memcpy(keystr + keystrlen, LLIST_GET_DATA(i), entrylen - 1);
-		keystr[keystrlen + entrylen - 1] = ' ';
-		keystrlen += entrylen;
-	}
-
-	keystr[keystrlen] = '\0';
-	return keystr;
+	struct string keystr;
+
+	string_init(&keystr);
+	if (!LLIST_GET_DATA(LLIST_FIRST(&keys[action])))
+		string_catf(&keystr, "%s", "UNDEFINED");
+	else
+		LLIST_FOREACH(&keys[action], i)
+			string_catf(&keystr, "%s ", LLIST_GET_DATA(i));
+	return string_buf(&keystr);
 }
 
 /* Need this to display keys properly inside status bar. */
@@ -509,7 +591,7 @@ keys_display_bindings_bar(WINDOW * win, int *bindings, int count,
 
 		const char *label;
 
-		if (binding_key < NBKEYS) {
+		if (binding_key < NBVKEYS) {
 			strncpy(key, keys_action_firstkey(binding_key), UTF8_MAXLEN);
 			key[UTF8_MAXLEN] = '\0';
 			label = gettext(keydef[binding_key].sb_label);
@@ -561,9 +643,9 @@ keys_display_bindings_bar(WINDOW * win, int *bindings, int count,
  * Display information about the given key.
  * (could not add the keys descriptions to keydef variable, because of i18n).
  */
-void keys_popup_info(enum key key)
+void keys_popup_info(enum vkey key)
 {
-	char *info[NBKEYS];
+	char *info[NBVKEYS];
 	WINDOW *infowin;
 
 	info[KEY_GENERIC_CANCEL] = _("Cancel the ongoing action.");
@@ -651,7 +733,7 @@ void keys_popup_info(enum key key)
 	info[KEY_LOWER_PRIORITY] =
 	    _("Lower a task priority inside the todo panel.");
 
-	if (key > NBKEYS)
+	if (key > NBVKEYS)
 		return;
 
 #define WINROW 10
@@ -668,59 +750,75 @@ void keys_popup_info(enum key key)
 void keys_save_bindings(FILE * fd)
 {
 	int i;
-	char *action;
+	char *keys;
 
 	EXIT_IF(fd == NULL, _("FATAL ERROR: null file pointer."));
 	dump_intro(fd);
-	for (i = 0; i < NBKEYS; i++) {
-		action = keys_action_allkeys(i);
-		if (action)
-			fprintf(fd, "%s  %s\n", keydef[i].label, action);
+	for (i = 0; i < NBVKEYS; i++) {
+		if ((keys = keys_action_allkeys(i)))
+			fprintf(fd, "%s  %s\n", keydef[i].label, keys);
 	}
+	mem_free(keys);
 }
 
-int keys_check_missing_bindings(void)
+int keys_check_undefined(void)
 {
 	int i;
 
-	for (i = 0; i < NBKEYS; i++) {
-		if (!LLIST_FIRST(&keys[i]))
+	for (i = 0; i < NBVKEYS; i++) {
+		if (!LLIST_GET_DATA(LLIST_FIRST(&keys[i])))
 			return 1;
 	}
 	return 0;
 }
 
-void keys_fill_missing(void)
+int keys_check_missing(void)
 {
 	int i;
 
-	for (i = 0; i < NBKEYS; i++) {
-		if (!LLIST_FIRST(&keys[i])) {
-			char *p, tmpbuf[BUFSIZ];
-
-			strncpy(tmpbuf, keydef[i].binding, BUFSIZ);
-			tmpbuf[BUFSIZ - 1] = '\0';
-			p = tmpbuf;
-			for (;;) {
-				char key_ch[BUFSIZ];
-
-				while (*p == ' ')
-					p++;
-				if (sscanf(p, "%s", key_ch) == 1) {
-					int ch, used;
-
-					ch = keys_str2int(key_ch);
-					used = keys_assign_binding(ch, i);
-					if (used)
-						WARN_MSG(_("When adding default key for \"%s\", "
-							  "\"%s\" was already assigned!"),
-							 keydef[i].label,
-							 key_ch);
-					p += strlen(key_ch);
-				} else {
-					break;
-				}
-			}
+	for (i = 0; i < NBVKEYS; i++) {
+		if (!LLIST_FIRST(&keys[i]))
+			return 1;
+	}
+	return 0;
+}
+
+/*
+ * Insert default keybindings for missing actions.
+ * Return either the number of actions assigned to (on success) or, if default
+ * keys could not be assigned, the negative index into the keydef[] table of the
+ * failing action.
+ */
+int keys_fill_missing(void)
+{
+	int i, ch, assign, assigned;
+	char *p, key_ch[BUFSIZ];
+
+	for (i = assigned = 0; i < NBVKEYS; i++) {
+		if (LLIST_FIRST(&keys[i]))
+			continue;
+
+		p = (char *)keydef[i].binding;
+		for (assign = 0;;) {
+			while (*p == ' ')
+				p++;
+			if (sscanf(p, "%s", key_ch) == 1) {
+				ch = keys_str2int(key_ch);
+				if (keys_assign_binding(ch, i))
+					return -i;
+				else
+					assign = 1;
+				p += strlen(key_ch);
+			} else
+				break;
 		}
+		assigned += assign;
+	}
+
+	if (assigned) {
+		p = (assigned == 1) ? "": "s";
+		WARN_MSG(_("Default key(s) assigned to %d action%s."),
+			 assigned, p);
 	}
+	return assigned;
 }