Convert wayvncctl subcommands to use option-parser

Also cleans up access to unparsed options.

Signed-off-by: Jim Ramsay <i.am@jimramsay.com>

include/ctl-client.h

@@ -20,6 +20,7 @@
 #include <stdbool.h>
 
 struct ctl_client;
+struct option_parser;
 
 void ctl_client_debug_log(bool enable);
 
@@ -32,7 +33,7 @@ void* ctl_client_userdata(struct ctl_client*);
 #define CTL_CLIENT_RECONNECT   (1 << 2)
 
 int ctl_client_run_command(struct ctl_client* self,
-		int argc, char* argv[], unsigned flags);
+		struct option_parser* parent_options, unsigned flags);
 
 void ctl_client_print_command_list(FILE* stream);
 void ctl_client_print_event_list(FILE* stream);

include/option-parser.h

@@ -35,6 +35,7 @@ struct wv_option_value {
 };
 
 struct option_parser {
+	const char* name;
 	const struct wv_option* options;
 	int n_opts;
 
@@ -42,7 +43,8 @@ struct option_parser {
 	int n_values;
 	int position;
 
-	int endpos;
+	size_t remaining_argc;
+	const char* const* remaining_argv;
 };
 
 void option_parser_init(struct option_parser* self,

src/ctl-client.c

@@ -32,6 +32,7 @@
 #include "ctl-server.h"
 #include "strlcpy.h"
 #include "util.h"
+#include "option-parser.h"
 
 #define LOG(level, fmt, ...) \
 	fprintf(stderr, "[%s:%d] <" level "> " fmt "\n", __FILE__, __LINE__, \
@@ -170,42 +171,26 @@ void* ctl_client_userdata(struct ctl_client* self)
 }
 
 static struct jsonipc_request* ctl_client_parse_args(struct ctl_client* self,
-		int argc, char* argv[])
+		enum cmd_type* cmd, struct option_parser* options)
 {
 	struct jsonipc_request* request = NULL;
-	const char* method = argv[0];
 	json_t* params = json_object();
-	bool show_usage = false;
+	struct cmd_info* info = ctl_command_by_type(*cmd);
-	for (int i = 1; i < argc; ++i) {
+	if (option_parser_get_value(options, "help")) {
-		char* key = argv[i];
+		json_object_set_new(params, "command", json_string(info->name));
-		char* value = NULL;
+		*cmd = CMD_HELP;
-		if (strcmp(key, "--help") == 0 || strcmp(key, "-h") == 0) {
+		info = ctl_command_by_type(*cmd);
-			show_usage = true;
+		goto out;
+	}
+	for (int i = 0; info->params[i].name != NULL; ++i) {
+		const char* key = info->params[i].name;
+		const char* value = option_parser_get_value(options, key);
+		if (!value)
 			continue;
-		}
-		if (key[0] == '-' && key[1] == '-')
-			key += 2;
-		char* delim = strchr(key, '=');
-		if (delim) {
-			*delim = '\0';
-			value = delim + 1;
-		} else if (++i < argc) {
-			value = argv[i];
-		} else {
-			WARN("Argument must be of the format --key=value or --key value");
-			goto failure;
-		}
 		json_object_set_new(params, key, json_string(value));
 	}
-	if (show_usage) {
+out:
-		// Special case for "foo --help"; convert into "help --command=foo"
+	request = jsonipc_request_new(info->name, params);
-		json_object_clear(params);
-		json_object_set_new(params, "command", json_string(method));
-		method = "help";
-	}
-	request = jsonipc_request_new(method, params);
-
-failure:
 	json_decref(params);
 	return request;
 }
@@ -660,31 +645,18 @@ void ctl_client_print_event_list(FILE* stream)
 	printf("\nRun 'wayvncctl help --event=event-name' for event-specific details.\n");
 }
 
-static void print_cmd_info_params(const struct cmd_info* info)
+static int print_command_usage(const char* cmd_name,
+		struct option_parser* cmd_options,
+		struct option_parser* parent_options)
 {
-	if (info->params[0].name != NULL) {
+	struct cmd_info* info = ctl_command_by_name(cmd_name);
-		printf("\nParameters:");
+	printf("Usage: wayvncctl [options] %s [parameters]\n\n%s\n\n", cmd_name,
-		for (int i = 0; info->params[i].name != NULL; ++i)
-			printf("\n  --%s=...\n    %s\n", info->params[i].name,
-					info->params[i].description);
-	}
-}
-
-static int print_command_usage(const char* cmd_name)
-{
-	enum cmd_type type = ctl_command_parse_name(cmd_name);
-	struct cmd_info* info = ctl_command_by_type(type);
-	if (!info) {
-		WARN("No such command \"%s\"\n", cmd_name);
-		return 1;
-	}
-	bool params = info->params[0].name != NULL;
-	printf("Usage: wayvncctl [options] %s%s\n\n%s\n", info->name,
-			params ? " [params]" : "",
 			info->description);
-	print_cmd_info_params(info);
+	option_parser_print_options(cmd_options, stdout);
-	printf("\nRun 'wayvncctl --help' for allowed options\n");
+	printf("\n");
-	if (type == CMD_EVENT_RECEIVE) {
+	option_parser_print_options(parent_options, stdout);
+	enum cmd_type cmd = ctl_command_parse_name(cmd_name);
+	if (cmd == CMD_EVENT_RECEIVE) {
 		printf("\n");
 		ctl_client_print_event_list(stdout);
 	}
@@ -700,12 +672,19 @@ static int print_event_details(const char* evt_name)
 	}
 	printf("Event: %s\n\n%s\n", info->name,
 			info->description);
-	print_cmd_info_params(info);
+	if (info->params[0].name != NULL) {
+		printf("\nData fields:");
+		for (int i = 0; info->params[i].name != NULL; ++i)
+			printf("\n  %s=...\n    %s\n", info->params[i].name,
+					info->params[i].description);
+	}
 	return 0;
 }
 
 static int ctl_client_print_help(struct ctl_client* self,
-		struct jsonipc_request* request)
+		struct jsonipc_request* request,
+		struct option_parser* cmd_options,
+		struct option_parser* parent_options)
 {
 	if (self->flags & CTL_CLIENT_PRINT_JSON) {
 		WARN("JSON output is not supported for the \"help\" command");
@@ -720,7 +699,8 @@ static int ctl_client_print_help(struct ctl_client* self,
 				"event", &evt_name);
 
 	if (cmd_name)
-		return print_command_usage(cmd_name);
+		return print_command_usage(cmd_name, cmd_options,
+				parent_options);
 	if (evt_name)
 		return print_event_details(evt_name);
 
@@ -731,17 +711,68 @@ static int ctl_client_print_help(struct ctl_client* self,
 	return 0;
 }
 
+int ctl_client_init_cmd_parser(struct option_parser* parser, enum cmd_type cmd)
+{
+	struct cmd_info* info = ctl_command_by_type(cmd);
+	if (!info) {
+		printf("Invalid command");
+		return -1;
+	}
+
+	size_t param_count = 0;
+	while (info->params[param_count].name != NULL)
+		param_count++;
+
+	// Add 2: one for --help and one to null-terminate the list
+	struct wv_option* options = calloc(param_count + 2,
+			sizeof(struct wv_option));
+	size_t i;
+	for (i = 0; i < param_count; ++i) {
+		struct wv_option* option = &options[i];
+		option->long_opt = info->params[i].name;
+		option->help = info->params[i].description;
+		option->schema = "<value>";
+	}
+	options[i].long_opt = "help";
+	options[i].short_opt = 'h';
+	options[i].help = "Display this help text";
+	option_parser_init(parser, options);
+	parser->name = "Parameters";
+	return 0;
+}
+
+static void ctl_client_destroy_cmd_parser(struct option_parser* parser)
+{
+	// const in the struct, but we allocated it above
+	free((void*)parser->options);
+}
+
 int ctl_client_run_command(struct ctl_client* self,
-		int argc, char* argv[], unsigned flags)
+		struct option_parser* parent_options, unsigned flags)
 {
 	self->flags = flags;
 	int result = 1;
-	struct jsonipc_request*	request = ctl_client_parse_args(self, argc,
+
-			argv);
+	const char* method = option_parser_get_value(parent_options, "command");
+	enum cmd_type cmd = ctl_command_parse_name(method);
+	if (cmd == CMD_UNKNOWN) {
+		WARN("No such command \"%s\"\n", method);
+		return 1;
+	}
+
+	struct option_parser cmd_options = { };
+	if (ctl_client_init_cmd_parser(&cmd_options, cmd) != 0)
+		return 1;
+
+	if (option_parser_parse(&cmd_options, parent_options->remaining_argc,
+				parent_options->remaining_argv) != 0)
+		goto parse_failure;
+
+	struct jsonipc_request*	request = ctl_client_parse_args(self, &cmd,
+			&cmd_options);
 	if (!request)
 		goto parse_failure;
 
-	enum cmd_type cmd = ctl_command_parse_name(request->method);
 	if (cmd != CMD_HELP) {
 		int timeout = (flags & CTL_CLIENT_SOCKET_WAIT) ? -1 : 0;
 		result = ctl_client_connect(self, timeout);
@@ -751,7 +782,8 @@ int ctl_client_run_command(struct ctl_client* self,
 
 	switch (cmd) {
 	case CMD_HELP:
-		result = ctl_client_print_help(self, request);
+		result = ctl_client_print_help(self, request, &cmd_options,
+				parent_options);
 		break;
 	case CMD_EVENT_RECEIVE:
 		result = ctl_client_event_loop(self, request);
@@ -763,5 +795,6 @@ int ctl_client_run_command(struct ctl_client* self,
 
 	jsonipc_request_destroy(request);
 parse_failure:
+	ctl_client_destroy_cmd_parser(&cmd_options);
 	return result;
 }

src/option-parser.c

@@ -39,6 +39,7 @@ void option_parser_init(struct option_parser* self,
 
 	self->options = options;
 	self->n_opts = count_options(options);
+	self->name = "Options";
 }
 
 static int get_left_col_width(const struct wv_option* opts, int n)
@@ -141,7 +142,7 @@ static void format_option(const struct wv_option* opt, int left_col_width,
 
 void option_parser_print_options(struct option_parser* self, FILE* stream)
 {
-	fprintf(stream, "Options:\n");
+	fprintf(stream, "%s:\n", self->name);
 	int left_col_width = get_left_col_width(self->options, self->n_opts);
 
 	for (int i = 0; i < self->n_opts; ++i) {
@@ -306,8 +307,10 @@ int option_parser_parse(struct option_parser* self, int argc,
 	while (i < argc) {
 		if (argv[i][0] == '-') {
 			if (argv[i][1] == '-') {
-				if (argv[i][2] == '\0')
+				if (argv[i][2] == '\0') {
-					return 0;
+					i++;
+					break;
+				}
 
 				int rc = parse_long_arg(self, argc, argv, i);
 				if (rc < 0)
@@ -328,7 +331,9 @@ int option_parser_parse(struct option_parser* self, int argc,
 			i += rc;
 		}
 	}
-	self->endpos = i;
+	self->remaining_argc = argc - i;
+	if (self->remaining_argc)
+		self->remaining_argv = argv + i;
 	return 0;
 }
 

src/wayvncctl.c

@@ -114,16 +114,13 @@ int main(int argc, char* argv[])
 	if (!option_parser_get_value(&option_parser, "command"))
 		return 0;
 
-	argc -= option_parser.endpos;
-	argv += option_parser.endpos;
-
 	ctl_client_debug_log(verbose);
 
 	self.ctl = ctl_client_new(socket_path, &self);
 	if (!self.ctl)
 		goto ctl_client_failure;
 
-	int result = ctl_client_run_command(self.ctl, argc, argv, flags);
+	int result = ctl_client_run_command(self.ctl, &option_parser, flags);
 
 	ctl_client_destroy(self.ctl);
 

test/option-parser-test.c

@@ -36,10 +36,40 @@ static int test_simple(void)
 	ASSERT_TRUE(option_parser_get_value(&parser, "a"));
 	ASSERT_TRUE(option_parser_get_value(&parser, "option-b"));
 	ASSERT_FALSE(option_parser_get_value(&parser, "value-option"));
+	ASSERT_INT_EQ(0, parser.remaining_argc);
+	ASSERT_FALSE(parser.remaining_argv);
 
 	return 0;
 }
 
+static int test_extra_positional_args(void)
+{
+	struct option_parser parser;
+	option_parser_init(&parser, options);
+
+	const char* argv[] = {
+		"executable",
+		"pos 1",
+		"pos 2",
+		"-a",
+		"pos 3",
+		"-b",
+		"pos 4",
+	};
+
+	ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv));
+
+	ASSERT_STR_EQ("pos 1", option_parser_get_value(&parser, "first"));
+	ASSERT_STR_EQ("pos 2", option_parser_get_value(&parser, "second"));
+	ASSERT_STR_EQ("pos 3", option_parser_get_value(&parser, "third"));
+	ASSERT_TRUE(option_parser_get_value(&parser, "a"));
+	ASSERT_TRUE(option_parser_get_value(&parser, "option-b"));
+	ASSERT_FALSE(option_parser_get_value(&parser, "value-option"));
+	ASSERT_INT_EQ(1, parser.remaining_argc);
+	ASSERT_STR_EQ("pos 4", parser.remaining_argv[0]);
+
+	return 0;
+}
 static int test_short_value_option_with_space(void)
 {
 	struct option_parser parser;
@@ -129,6 +159,8 @@ static int test_stop(void)
 
 	ASSERT_TRUE(option_parser_get_value(&parser, "a"));
 	ASSERT_FALSE(option_parser_get_value(&parser, "b"));
+	ASSERT_INT_EQ(1, parser.remaining_argc);
+	ASSERT_STR_EQ("-b", parser.remaining_argv[0]);
 	return 0;
 }
 
@@ -175,8 +207,9 @@ static int test_subcommand_without_arguments(void)
 	const char* argv[] = { "executable", "-ab", "first", "second", "third",
 		"do-stuff" };
 	ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv));
-	ASSERT_INT_EQ(5, parser.endpos);
 	ASSERT_STR_EQ("do-stuff", option_parser_get_value(&parser, "command"));
+	ASSERT_INT_EQ(1, parser.remaining_argc);
+	ASSERT_STR_EQ("do-stuff", parser.remaining_argv[0]);
 	return 0;
 }
 
@@ -187,8 +220,10 @@ static int test_subcommand_with_arguments(void)
 	const char* argv[] = { "executable", "-ab", "first", "second", "third",
 		"do-stuff", "--some-option", "another-argument"};
 	ASSERT_INT_EQ(0, option_parser_parse(&parser, ARRAY_SIZE(argv), argv));
-	ASSERT_INT_EQ(5, parser.endpos);
 	ASSERT_STR_EQ("do-stuff", option_parser_get_value(&parser, "command"));
+	ASSERT_INT_EQ(3, parser.remaining_argc);
+	ASSERT_STR_EQ("do-stuff", parser.remaining_argv[0]);
+	ASSERT_STR_EQ("another-argument", parser.remaining_argv[2]);
 	return 0;
 }
 
@@ -196,6 +231,7 @@ int main()
 {
 	int r = 0;
 	RUN_TEST(test_simple);
+	RUN_TEST(test_extra_positional_args);
 	RUN_TEST(test_short_value_option_with_space);
 	RUN_TEST(test_short_value_option_without_space);
 	RUN_TEST(test_short_value_option_with_eq);