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>

5 files changed, 327 insertions, 252 deletions

diff --git a/src/calcurse.h b/src/calcurse.h
index d5ba355..06deaa0 100644
--- a/src/calcurse.h
+++ b/src/calcurse.h
@@ -515,8 +515,8 @@ struct io_file {
 	char *name;
 };
 
-/* Available keys. */
-enum key {
+/* Virtual keys. */
+enum vkey {
 	KEY_GENERIC_CANCEL,
 	KEY_GENERIC_SELECT,
 	KEY_GENERIC_CREDITS,
@@ -566,7 +566,7 @@ enum key {
 	KEY_RAISE_PRIORITY,
 	KEY_LOWER_PRIORITY,
 
-	NBKEYS,
+	NBVKEYS,
 	KEY_UNDEF,
 
 	/* Non-configurable, context sensitive key bindings. */
@@ -946,24 +946,26 @@ int io_get_modified(void);
 void keys_init(void);
 void keys_free(void);
 void keys_dump_defaults(char *);
-const char *keys_get_label(enum key);
-enum key keys_get_action(int);
+const char *keys_get_label(enum vkey);
+const char *keys_get_binding(enum vkey);
+enum vkey keys_get_action(int);
 int keys_wgetch(WINDOW *);
 void keys_wait_for_any_key(WINDOW *);
-enum key keys_get(WINDOW *, int *, int *);
-int keys_assign_binding(int, enum key);
-void keys_remove_binding(int, enum key);
+enum vkey keys_get(WINDOW * win, int *, int *);
+int keys_assign_binding(int, enum vkey);
+void keys_remove_binding(int, enum vkey);
 int keys_str2int(const char *);
 char *keys_int2str(int);
-int keys_action_count_keys(enum key);
-const char *keys_action_firstkey(enum key);
-const char *keys_action_nkey(enum key, int);
-char *keys_action_allkeys(enum key);
+int keys_action_count_keys(enum vkey);
+const char *keys_action_firstkey(enum vkey);
+const char *keys_action_nkey(enum vkey, int);
+char *keys_action_allkeys(enum vkey);
 void keys_display_bindings_bar(WINDOW *, int *, int, int, int);
-void keys_popup_info(enum key);
+void keys_popup_info(enum vkey);
 void keys_save_bindings(FILE *);
-int keys_check_missing_bindings(void);
-void keys_fill_missing(void);
+int keys_check_missing(void);
+int keys_check_undefined(void);
+int keys_fill_missing(void);
 
 /* listbox.c */
 void listbox_init(struct listbox *, int, int, int, int, const char *,
diff --git a/src/custom.c b/src/custom.c
index 0e5e554..accd8d3 100644
--- a/src/custom.c
+++ b/src/custom.c
@@ -975,10 +975,11 @@ print_keys_bindings(WINDOW * win, int selected_row, int selected_elm,
 	const int XPOS = 1;
 	const int EQUALPOS = 23;
 	const int KEYPOS = 25;
-	int noelm, action, y;
+	int noelm, action, y, pos;
+	const char *key = NULL;
 
 	noelm = y = 0;
-	for (action = 0; action < NBKEYS; action++) {
+	for (action = 0; action < NBVKEYS; action++) {
 		char *actionstr;
 		int nbkeys;
 
@@ -990,18 +991,15 @@ print_keys_bindings(WINDOW * win, int selected_row, int selected_elm,
 		mem_free(actionstr);
 		mvwaddstr(win, y, EQUALPOS, "=");
 		if (nbkeys == 0)
-			mvwaddstr(win, y, KEYPOS, _("undefined"));
+			mvwaddstr(win, y, KEYPOS, _("UNDEFINED"));
 		if (action == selected_row)
 			custom_remove_attr(win, ATTR_HIGHEST);
 		if (nbkeys > 0) {
 			if (action == selected_row) {
-				const char *key;
-				int pos;
-
+				/* Elements may have been added or deleted. */
+				wclrtoeol(win);
 				pos = KEYPOS;
-				while ((key =
-					keys_action_nkey(action,
-							 noelm)) != NULL) {
+				while ((key = keys_action_nkey(action, noelm))) {
 					if (noelm == selected_elm)
 						print_key_incolor(win, key,
 								  y, pos);
@@ -1012,8 +1010,9 @@ print_keys_bindings(WINDOW * win, int selected_row, int selected_elm,
 					pos += utf8_strwidth((char *)key) + 1;
 				}
 			} else {
-				mvwaddstr(win, y, KEYPOS,
-					  keys_action_allkeys(action));
+				key = keys_action_allkeys(action);
+				mvwaddstr(win, y, KEYPOS, key);
+				mem_free((char *)key);
 			}
 		}
 		y += yoff;
@@ -1045,9 +1044,11 @@ void custom_keys_config(void)
 	const int LABELLINES = 3;
 
 	clear();
-	nbdisplayed = ((notify_bar() ? row - 3 : row - 2) - LABELLINES) / LINESPERKEY;
-	wins_scrollwin_init(&kwin, 0, 0, notify_bar() ? row - 3 : row - 2, col, _("keys configuration"));
-	wins_scrollwin_set_pad(&kwin, NBKEYS * LINESPERKEY);
+	nbdisplayed = ((notify_bar() ? row - 3 : row - 2) -
+		       LABELLINES) / LINESPERKEY;
+	wins_scrollwin_init(&kwin, 0, 0, notify_bar() ? row - 3 : row - 2, col,
+			    _("keys configuration"));
+	wins_scrollwin_set_pad(&kwin, NBVKEYS * LINESPERKEY);
 	wins_scrollwin_draw_deco(&kwin, 0);
 	custom_keys_config_bar();
 	selrow = selelm = 0;
@@ -1072,7 +1073,7 @@ void custom_keys_config(void)
 			}
 			break;
 		case KEY_MOVE_DOWN:
-			if (selrow < NBKEYS - 1) {
+			if (selrow < NBVKEYS - 1) {
 				selrow++;
 				selelm = 0;
 				if (selrow == lastrow) {
@@ -1103,7 +1104,7 @@ void custom_keys_config(void)
 					keys_get_label(selrow), 0);
 			for (;;) {
 				ch = keys_wgetch(grabwin);
-				enum key action = keys_get_action(ch);
+				enum vkey action = keys_get_action(ch);
 				/* Is the key already used by this action? */
 				if (action == selrow)
 					break;
@@ -1142,10 +1143,8 @@ void custom_keys_config(void)
 				selelm--;
 			break;
 		case KEY_GENERIC_QUIT:
-			if (keys_check_missing_bindings() != 0) {
-				WARN_MSG(_("Some actions do not have any associated "
-					  "key bindings!"));
-			}
+			if (keys_check_undefined())
+				WARN_MSG(_("Some actions are left undefined!"));
 			wins_scrollwin_delete(&kwin);
 			return;
 		}
diff --git a/src/help.c b/src/help.c
index 398fb6d..40a03b8 100644
--- a/src/help.c
+++ b/src/help.c
@@ -105,7 +105,7 @@ int display_help(const char *topic)
 
 	if (!io_file_exists(path)) {
 		int ch = keys_str2int(topic);
-		enum key action = keys_get_action(ch);
+		enum vkey action = keys_get_action(ch);
 		if (ch > 0 && action > 0 && action != KEY_UNDEF) {
 			topic = keys_get_label(action);
 			mem_free(path);
diff --git a/src/io.c b/src/io.c
index 71960f5..43a8390 100644
--- a/src/io.c
+++ b/src/io.c
@@ -50,8 +50,8 @@
 
 struct ht_keybindings_s {
 	const char *label;
-	enum key key;
-	 HTABLE_ENTRY(ht_keybindings_s);
+	enum vkey key;
+	HTABLE_ENTRY(ht_keybindings_s);
 };
 
 static void load_keys_ht_getkey(struct ht_keybindings_s *, const char **,
@@ -940,16 +940,6 @@ load_keys_ht_compare(struct ht_keybindings_s *data1,
 }
 
 /*
- * isblank(3) is protected by the __BSD_VISIBLE macro and this fails to be
- * visible in some specific cases. Thus replace it by the following is_blank()
- * function.
- */
-static int is_blank(int c)
-{
-	return c == ' ' || c == '\t';
-}
-
-/*
  * Load user-definable keys from file.
  * A hash table is used to speed up loading process in avoiding string
  * comparisons.
@@ -958,21 +948,21 @@ static int is_blank(int c)
  */
 void io_load_keys(const char *pager)
 {
-	struct ht_keybindings_s keys[NBKEYS];
+	struct ht_keybindings_s virt_keys[NBVKEYS], *ht_elm, ht_entry;
 	FILE *keyfp;
-	char buf[BUFSIZ];
+	char buf[BUFSIZ], key_label[BUFSIZ], key_str[BUFSIZ];
+	char *p, *msg;
 	struct io_file *log;
-	int i, skipped, loaded, line;
-	const int MAX_ERRORS = 5;
+	int i, n, skipped, loaded, line, assigned, undefined, key;
 
 	keys_init();
 
 	struct ht_keybindings ht_keys = HTABLE_INITIALIZER(&ht_keys);
 
-	for (i = 0; i < NBKEYS; i++) {
-		keys[i].key = (enum key)i;
-		keys[i].label = keys_get_label((enum key)i);
-		HTABLE_INSERT(ht_keybindings, &ht_keys, &keys[i]);
+	for (i = 0; i < NBVKEYS; i++) {
+		virt_keys[i].key = (enum vkey)i;
+		virt_keys[i].label = keys_get_label((enum vkey)i);
+		HTABLE_INSERT(ht_keybindings, &ht_keys, &virt_keys[i]);
 	}
 
 	keyfp = fopen(path_keys, "r");
@@ -981,111 +971,97 @@ void io_load_keys(const char *pager)
 	log = io_log_init();
 	skipped = loaded = line = 0;
 	while (fgets(buf, BUFSIZ, keyfp) != NULL) {
-		char key_label[BUFSIZ], *p;
-		struct ht_keybindings_s *ht_elm, ht_entry;
-		const int AWAITED = 1;
-		int assigned;
-
 		line++;
-		if (skipped > MAX_ERRORS) {
-			const char *too_many =
-			    _("\nToo many errors while reading configuration file!\n"
-			     "Please backup your keys file, remove it from directory, "
-			     "and launch calcurse again.\n");
-
-			io_log_print(log, line, too_many);
-			break;
-		}
-		for (p = buf; is_blank((int)*p); p++) ;
-		if (p != buf)
-			memmove(buf, p, strlen(p));
-		if (buf[0] == '#' || buf[0] == '\n')
+		p = buf;
+		while (*p == ' ' || *p == '\t') p++;
+		if (*p == '#' || *p == '\n')
 			continue;
 
-		if (sscanf(buf, "%s", key_label) != AWAITED) {
+		/* Find the virtual key by key label. */
+		if (sscanf(p, "%s", key_label) != 1) {
 			skipped++;
 			io_log_print(log, line,
 				     _("Could not read key label"));
 			continue;
 		}
-
-		/* Skip legacy entries. */
-		if (strcmp(key_label, "generic-cut") == 0)
-			continue;
-
+		p += strlen(key_label);
 		ht_entry.label = key_label;
-		p = buf + strlen(key_label) + 1;
-		ht_elm =
-		    HTABLE_LOOKUP(ht_keybindings, &ht_keys, &ht_entry);
+		ht_elm = HTABLE_LOOKUP(ht_keybindings, &ht_keys, &ht_entry);
 		if (!ht_elm) {
 			skipped++;
-			io_log_print(log, line,
-				     _("Key label not recognized"));
+			asprintf(&msg,
+				 _("Key label not recognized: \"%s\""),
+				 key_label);
+			io_log_print(log, line, msg);
+			mem_free(msg);
 			continue;
 		}
-		assigned = 0;
-		for (;;) {
-			char key_ch[BUFSIZ], tmpbuf[BUFSIZ];
-
-			while (*p == ' ')
-				p++;
-			(void)strncpy(tmpbuf, p, BUFSIZ);
-			tmpbuf[BUFSIZ - 1] = '\0';
-			if (sscanf(tmpbuf, "%s", key_ch) == AWAITED) {
-				int ch;
-
-				if ((ch = keys_str2int(key_ch)) < 0) {
-					char *unknown_key;
 
+		/* Assign keyboard keys to the virtual key. */
+		assigned = undefined = 0;
+		for (;;) {
+			if (sscanf(p, "%s%n", key_str, &n) != 1) {
+				if (assigned || undefined)
+					loaded++;
+				else {
 					skipped++;
-					asprintf(&unknown_key,
-						 _("Error reading key: \"%s\""),
-						 key_ch);
-					io_log_print(log, line, unknown_key);
-					mem_free(unknown_key);
-				} else {
-					int used;
-
-					used =
-					    keys_assign_binding(ch,
-								ht_elm->
-								key);
-					if (used) {
-						char *already_assigned;
-
-						skipped++;
-						asprintf(&already_assigned,
-							 _("\"%s\" assigned multiple times!"),
-							 key_ch);
-						io_log_print(log, line,
-							     already_assigned);
-						mem_free(already_assigned);
-					} else {
-						assigned++;
-					}
+					asprintf(&msg,
+						 _("No keys assigned to "
+						   "\"%s\"."),
+						 key_label);
+					io_log_print(log, line, msg);
+					mem_free(msg);
 				}
-				p += strlen(key_ch) + 1;
-			} else {
-				if (assigned)
-					loaded++;
 				break;
 			}
+			p += n;
+			if (!strcmp(key_str, "UNDEFINED")) {
+				undefined++;
+				keys_assign_binding(-1, ht_elm->key);
+			} else if ((key = keys_str2int(key_str)) < 0) {
+				skipped++;
+				asprintf(&msg,
+					 _("Keyname not recognized: \"%s\""),
+					 key_str);
+				io_log_print(log, line, msg);
+				mem_free(msg);
+			} else if (keys_assign_binding(key, ht_elm->key)) {
+				skipped++;
+				asprintf(&msg,
+					 _("\"%s\" assigned twice: \"%s\"."),
+					 key_str, key_label);
+				io_log_print(log, line, msg);
+				mem_free(msg);
+			} else
+				assigned++;
 		}
 	}
 	file_close(keyfp, __FILE_POS__);
+	if (loaded < NBVKEYS && (i = keys_fill_missing()) < 1) {
+		skipped++;
+		strcpy(key_label, keys_get_label((enum vkey)(-i)));
+		strcpy(key_str, keys_get_binding((enum vkey)(-i)));
+		asprintf(&msg, _("Action \"%s\" absent, but default key \"%s\" "
+				 "assigned to another action."),
+				 key_label, key_str);
+		io_log_print(log, line, msg);
+		mem_free(msg);
+	}
 	file_close(log->fd, __FILE_POS__);
 	if (skipped > 0) {
-		const char *view_log =
-			_("There were some errors when loading keys file.");
-		io_log_display(log, view_log, pager);
+		msg = _("Errors in the keys file.");
+		io_log_display(log, msg, pager);
+		WARN_MSG(_("Remove offending line(s) from the keys file, "
+			   "aborting..."));
+		exit_calcurse(EXIT_FAILURE);
 	}
 	io_log_free(log);
-	EXIT_IF(skipped > MAX_ERRORS,
-		_("Too many errors while reading keys file, aborting..."));
-	if (loaded < NBKEYS)
-		keys_fill_missing();
-	if (keys_check_missing_bindings())
-		WARN_MSG(_("Some actions do not have any associated key bindings!"));
+	/* Default keys were inserted. */
+	if (loaded < NBVKEYS)
+		io_save_keys();
+	/* Should never occur. */
+	EXIT_IF(keys_check_missing(),
+		_("Some actions do not have any associated key bindings!"));
 }
 
 int io_check_dir(const char *dir)
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;
 }