Add comments for bind commands [#809]

include/tig/argv.h

@@ -24,6 +24,7 @@
 #define DIFF_ARGS "%(diffargs)"
 
 bool argv_to_string(const char *argv[], char *buf, size_t buflen, const char *sep);
+char *argv_to_string_alloc_prefix(const char *argv[], const char *sep, const char *prefix);
 char *argv_to_string_alloc(const char *argv[], const char *sep);
 bool argv_to_string_quoted(const char *argv[SIZEOF_ARG], char *buf, size_t buflen, const char *sep);
 bool argv_from_string_no_quotes(const char *argv[SIZEOF_ARG], int *argc, char *cmd);

include/tig/keys.h

@@ -90,10 +90,12 @@ struct run_request {
 	struct keymap *keymap;
 	struct run_request_flags flags;
 	const char **argv;
+	char *name;
+	char *help;
 };
 
 struct run_request *get_run_request(enum request request);
-enum status_code add_run_request(struct keymap *keymap, const struct key key[], size_t keys, const char **argv);
+enum status_code add_run_request(struct keymap *keymap, const struct key key[], size_t keys, const char **argv, const char *help);
 enum status_code parse_run_request_flags(struct run_request_flags *flags, const char **argv);
 const char *format_run_request_flags(const struct run_request *req);
 

include/tig/options.h

@@ -216,7 +216,7 @@ struct option_info *find_column_option_info(enum view_column_type type, union vi
 					const char *option, struct option_info *column_info, const char **column_name);
 enum status_code parse_int(int *opt, const char *arg, int min, int max);
 enum status_code parse_step(double *opt, const char *arg);
-enum status_code set_option(const char *opt, int argc, const char *argv[]);
+enum status_code set_option(const char *opt, int argc, const char *argv[], const char *help);
 enum status_code load_options(void);
 enum status_code load_git_config(void);
 enum status_code save_options(const char *path);

src/argv.c

@@ -56,16 +56,43 @@ concat_argv(const char *argv[], char *buf, size_t buflen, const char *sep, bool
 }
 
 char *
-argv_to_string_alloc(const char *argv[], const char *sep)
+argv_to_string_alloc_prefix(const char *argv[], const char *sep, const char *prefix)
 {
-	size_t i, size = 0;
-	char *buf;
+	size_t i, size = 1;
+	for (i = 0; argv[i]; i++)
+		size += strlen(argv[i]);
+
+	/* Increase buffer size by separator size and number of separators */
+	if (sep) if (i) size += strlen(sep) * (i - 1);
+
+	/* Increase buffer size by prefix size */
+	if (prefix) size += strlen(prefix);
+
+	char *buf = malloc(size);
+	if (buf) {
+
+		/* Preload buffer with prefix */
+		for (i = 0; prefix[i]; i++) buf[i] = prefix[i];
+
+		/* Load buffer after prefix with concatenated version of argv */
+		if (concat_argv(argv, buf + i, size, sep, false)) return buf;
+	}
+	free(buf);
+	return NULL;
+}
 
+char *
+argv_to_string_alloc(const char *argv[], const char *sep)
+{
+	size_t i, size = 1;
 	for (i = 0; argv[i]; i++)
 		size += strlen(argv[i]);
 
-	buf = malloc(size + 1);
-	if (buf && argv_to_string(argv, buf, size + 1, sep))
+	/* Increase buffer size by separator size and number of separators */
+	if (sep) if (i) size += strlen(sep) * (i - 1);
+
+	char *buf = malloc(size);
+	if (buf && concat_argv(argv, buf, size, sep, false))
 		return buf;
 	free(buf);
 	return NULL;

src/help.c

@@ -28,6 +28,7 @@ struct help_state {
 struct help {
 	struct keymap *keymap;
 	enum request request;
+	const char *key;
 	union {
 		const char *text;
 		const struct request_info *req_info;
@@ -50,30 +51,35 @@ help_draw(struct view *view, struct line *line, unsigned int lineno)
 
 	} else if (help->request > REQ_RUN_REQUESTS) {
 		struct run_request *req = get_run_request(help->request);
-		const char *key = get_keys(keymap, help->request, true);
-		const char *sep = format_run_request_flags(req);
-		int i;
 
-		if (draw_field(view, LINE_DEFAULT, key, state->keys_width + 2, ALIGN_RIGHT, false))
+		if (draw_field(view, LINE_DEFAULT, help->key, state->keys_width + 2, ALIGN_RIGHT, false))
 			return true;
 
-		for (i = 0; req->argv[i]; i++) {
-			if (draw_formatted(view, LINE_HELP_ACTION, "%s%s", sep, req->argv[i]))
+		/* If there is req->help text to draw, then first draw req->name as a fixed-width field */
+		if (req->help) {
+			if (draw_field(view, LINE_HELP_ACTION, req->name, state->name_width, ALIGN_LEFT, false))
 				return true;
-			sep = " ";
+			draw_text(view, LINE_DEFAULT, req->help);
 		}
 
+		/* Else just draw req->name as free-form text */
+		else draw_text(view, LINE_HELP_ACTION, req->name);
+
 	} else {
 		const struct request_info *req_info = help->data.req_info;
-		const char *key = get_keys(keymap, req_info->request, true);
 
-		if (draw_field(view, LINE_DEFAULT, key, state->keys_width + 2, ALIGN_RIGHT, false))
+		if (draw_field(view, LINE_DEFAULT, help->key, state->keys_width + 2, ALIGN_RIGHT, false))
 			return true;
 
-		if (draw_field(view, LINE_HELP_ACTION, enum_name(req_info->name), state->name_width, ALIGN_LEFT, false))
-			return true;
+		/* If there is req_info->help text to draw, then first draw req_info->name as a fixed-width field */
+		if (req_info->help) {
+			if (draw_field(view, LINE_HELP_ACTION, enum_name(req_info->name), state->name_width, ALIGN_LEFT, false))
+				return true;
+			draw_text(view, LINE_DEFAULT, req_info->help);
+		}
 
-		draw_text(view, LINE_DEFAULT, req_info->help);
+		/* Else just draw req_info->name as free-form text */
+		else draw_text(view, LINE_HELP_ACTION, enum_name(req_info->name));
 	}
 
 	return true;
@@ -97,19 +103,13 @@ help_grep(struct view *view, struct line *line)
 
 	} else if (help->request > REQ_RUN_REQUESTS) {
 		struct run_request *req = get_run_request(help->request);
-		const char *key = get_keys(keymap, help->request, true);
-		char buf[SIZEOF_STR] = "";
-		const char *text[] = { key, buf, NULL };
-
-		if (!argv_to_string(req->argv, buf, sizeof(buf), " "))
-			return false;
+		const char *text[] = { help->key, req->name, req->help, NULL };
 
 		return grep_text(view, text);
 
 	} else {
 		const struct request_info *req_info = help->data.req_info;
-		const char *key = get_keys(keymap, req_info->request, true);
-		const char *text[] = { key, enum_name(req_info->name), req_info->help, NULL };
+		const char *text[] = { help->key, enum_name(req_info->name), req_info->help, NULL };
 
 		return grep_text(view, text);
 	}
@@ -161,12 +161,24 @@ help_keys_visitor(void *data, const char *group, struct keymap *keymap,
 	if (!add_help_line(view, &help, keymap, LINE_DEFAULT))
 		return false;
 
-	state->keys_width = MAX(state->keys_width, strlen(key));
+	int len = strlen(key);
+	state->keys_width = MAX(state->keys_width, len);
+	/* Since the key string is available, cache it for when help lines need to be drawn */
+	help->key = len > 0 ? strdup(key) : NULL;
+
 	help->request = request;
 
 	if (req_info) {
-		state->name_width = MAX(state->name_width, strlen(enum_name(req_info->name)));
 		help->data.req_info = req_info;
+		/* Include req_info->name in the MAX calculation but only if there is help text */
+		if (req_info->help && strlen(req_info->help) > 0)
+			state->name_width = MAX(state->name_width, strlen(enum_name(req_info->name)));
+	}
+
+	if (run_req) {
+		/* Include run_req->name in the MAX calculation but only if there is help text */
+		if (run_req->help && strlen(run_req->help) > 0)
+			state->name_width = MAX(state->name_width, strlen(run_req->name));
 	}
 
 	return true;
@@ -178,6 +190,12 @@ help_open(struct view *view, enum open_flags flags)
 	struct help_request_iterator iterator = { view };
 	struct help *help;
 
+	/* Need to free any key strings that have been cached */
+	int i; for (i = 0; i < view->lines; i++) {
+		const struct help *h = view->line[i].data;
+		free((void *)h->key);
+	}
+
 	reset_view(view);
 
 	if (!add_help_line(view, &help, NULL, LINE_HEADER))

src/keys.c

@@ -486,12 +486,12 @@ parse_run_request_flags(struct run_request_flags *flags, const char **argv)
 
 enum status_code
 add_run_request(struct keymap *keymap, const struct key key[],
-		size_t keys, const char **argv)
+		size_t keys, const char **argv, const char *help)
 {
 	struct run_request *req;
 	struct run_request_flags flags = {0};
-	enum status_code code = parse_run_request_flags(&flags, argv);
 
+	enum status_code code = parse_run_request_flags(&flags, argv);
 	if (code != SUCCESS)
 		return code;
 
@@ -505,6 +505,19 @@ add_run_request(struct keymap *keymap, const struct key key[],
 	req->flags = flags;
 	req->keymap = keymap;
 
+	/* Duplicate a displayable version of **argv into the run_request struct */
+	req->name = NULL;
+	if (argv) {
+		if ((req->name = argv_to_string_alloc_prefix(argv, " ", format_run_request_flags(req))) == NULL)
+			return ERROR_OUT_OF_MEMORY;
+	}
+
+	/* If there is help text, then dupe it into the run_request struct */
+	req->help = NULL;
+	if (help)
+		if ((req->help = strdup(help)) == NULL)
+			return ERROR_OUT_OF_MEMORY;
+
 	return add_keybinding(keymap, REQ_RUN_REQUESTS + run_requests, key, keys);
 }
 

src/options.c

@@ -790,7 +790,7 @@ option_set_command(int argc, const char *argv[])
 
 /* Wants: mode request key */
 static enum status_code
-option_bind_command(int argc, const char *argv[])
+option_bind_command(int argc, const char *argv[], const char *help)
 {
 	struct key key[16];
 	size_t keys = 0;
@@ -870,7 +870,7 @@ option_bind_command(int argc, const char *argv[])
 			const char *toggle[] = { ":toggle", mapped, arg, NULL};
 			const char *other[] = { mapped, NULL };
 			const char **prompt = *mapped == ':' ? other : toggle;
-			enum status_code code = add_run_request(keymap, key, keys, prompt);
+			enum status_code code = add_run_request(keymap, key, keys, prompt, NULL);
 
 			if (code == SUCCESS)
 				code = error("%s has been replaced by `%s%s%s%s'",
@@ -882,7 +882,7 @@ option_bind_command(int argc, const char *argv[])
 	}
 
 	if (request == REQ_UNKNOWN)
-		return add_run_request(keymap, key, keys, argv + 2);
+		return add_run_request(keymap, key, keys, argv + 2, help);
 
 	return add_keybinding(keymap, request, key, keys);
 }
@@ -916,7 +916,7 @@ option_source_command(int argc, const char *argv[])
 }
 
 enum status_code
-set_option(const char *opt, int argc, const char *argv[])
+set_option(const char *opt, int argc, const char *argv[], const char *help)
 {
 	if (!strcmp(opt, "color"))
 		return option_color_command(argc, argv);
@@ -925,7 +925,7 @@ set_option(const char *opt, int argc, const char *argv[])
 		return option_set_command(argc, argv);
 
 	if (!strcmp(opt, "bind"))
-		return option_bind_command(argc, argv);
+		return option_bind_command(argc, argv, help);
 
 	if (!strcmp(opt, "source"))
 		return option_source_command(argc, argv);
@@ -957,15 +957,16 @@ read_option(char *opt, size_t optlen, char *value, size_t valuelen, void *data)
 		const char *argv[SIZEOF_ARG];
 		int argc = 0;
 
-		if (len < valuelen) {
+		bool have_comments = len < valuelen;
+		if (have_comments) {
 			valuelen = len;
 			value[valuelen] = 0;
 		}
 
 		if (!argv_from_string(argv, &argc, value))
 			status = error("Too many option arguments for %s", opt);
 		else
-			status = set_option(opt, argc, argv);
+			status = set_option(opt, argc, argv, have_comments ? string_trim(value + len + 1) : NULL);
 	}
 
 	if (status != SUCCESS) {
@@ -1322,16 +1323,15 @@ set_remote_branch(const char *name, const char *value, size_t valuelen)
 }
 
 static void
-set_repo_config_option(char *name, char *value, enum status_code (*cmd)(int, const char **))
+set_repo_config_option(char *name, char *value, const char *cmd)
 {
-	const char *argv[SIZEOF_ARG] = { name, "=" };
-	int argc = 1 + (cmd == option_set_command);
-	enum status_code code;
+	const char *argv[SIZEOF_ARG]; int argc;
+	if (!strcmp(cmd, "set")) { argv[0] = name; argv[1] = "=";  argc = 2; }
+	else                     { argv[0] = name;                 argc = 1; }
 
-	if (!argv_from_string(argv, &argc, value))
-		code = error("Too many arguments");
-	else
-		code = cmd(argc, argv);
+	enum status_code code = !argv_from_string(argv, &argc, value)
+		? error("Too many arguments")
+		: set_option(cmd, argc, argv, NULL);
 
 	if (code != SUCCESS)
 		warn("Option 'tig.%s': %s", name, get_status_message(code));
@@ -1443,13 +1443,13 @@ read_repo_config_option(char *name, size_t namelen, char *value, size_t valuelen
 		parse_bool(&opt_status_show_untracked_files, value);
 
 	else if (!prefixcmp(name, "tig.color."))
-		set_repo_config_option(name + 10, value, option_color_command);
+		set_repo_config_option(name + 10, value, "color");
 
 	else if (!prefixcmp(name, "tig.bind."))
-		set_repo_config_option(name + 9, value, option_bind_command);
+		set_repo_config_option(name + 9, value, "bind");
 
 	else if (!prefixcmp(name, "tig."))
-		set_repo_config_option(name + 4, value, option_set_command);
+		set_repo_config_option(name + 4, value, "set");
 
 	else if (!prefixcmp(name, "color."))
 		set_git_color_option(name + STRING_SIZE("color."), value);

src/prompt.c

@@ -1051,7 +1051,7 @@ run_prompt_command(struct view *view, const char *argv[])
 		if (request != REQ_UNKNOWN)
 			return request;
 
-		code = set_option(argv[0], argv_size(argv + 1), &argv[1]);
+		code = set_option(cmd, argv_size(argv + 1), &argv[1], NULL);
 		if (code != SUCCESS) {
 			report("%s", get_status_message(code));
 			return REQ_NONE;