Better commandline interface, support multiple operations

wlopm.1

@@ -1,43 +1,46 @@
-.TH wlopm 1 2021-03-14 wlopm-0.0.1
+.TH wlopm 1 2021-03-17 wlopm-0.0.1
 
 .SH NAME
 wlopm - Wayland output power management
 
 .SH DESCRIPTION
-wlopm is a simple client implementing zwlr-output-power-management-v1. It can be
+wlopm is a simple client implementing zwlr-output-power-management-v1. If no
-used to query and set the output power mode of Wayland outputs.
+operations are defined, wlopm will list all outputs and their current power
+modes.
 
 .SH USAGE
-\fBwlopm\fR
+\fB-h\fR, \fB--help\fR
 .RS 4
-List outputs and their power modes.
+Print help and exit.
 .P
 .RE
 
-\fBwlopm --json\fR
+\fB-j\fR, \fB--json\fR
 .RS 4
-List outputs and their power modes, formatted in JSON.
+Enable JSON formatting for listing outputs and errors encountered while trying
+to set their power modes.
 .P
 .RE
 
-\fBwlopm on <output-name>\fR
+\fB--on\fR <output-name>
 .RS 4
-Set output power mode to on.
+Set the power mode of the output to on.
 .P
 .RE
 
-\fBwlopm off <output-name>\fR
+\fB--off\fR <output-name>
 .RS 4
-Set output power mode to off.
+Set the power mode of the output to off.
 .P
 .RE
 
-\fBwlopm toggle <output-name>\fR
+\fB--toggle\fR <output-name>
 .RS 4
-Toggle the output power mode.
+Toggle the power mode of the output.
 .P
 .RE
 
+
 .SH AUTHOR
 Leon Henrik Plickat
 

wlopm.c

@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
+#include <errno.h>
 
 #include <wayland-client.h>
 
@@ -31,26 +32,14 @@
 #define VERSION "0.0.1"
 
 const char usage[] =
-	"Usage:\n"
+	"Usage: wlopm [options...]\n"
-	"\twlopm                        List outputs and their power modes.\n"
+	"  -j, --json                  Use JSON format.\n"
-	"\twlopm --json                 Format the list as JSON.\n"
+	"  -h, --help                  Print this help text and exit.\n"
-	"\twlopm on     <output-name>   Set output power mode to on.\n"
+	"      --on     <output-name>  Set the power mode of the specified output to on.\n"
-	"\twlopm off    <output-name>   Set output power mode to off.\n"
+	"      --off    <output-name>  Set the power mode of the specified output to off.\n"
-	"\twlopm toggle <output-name>   Toggle output power mode.\n"
+	"      --toggle <output-name>  Toggle the power mode of the specified output.\n"
 	"\n";
 
-enum Action
-{
-	LIST,
-	ON,
-	OFF,
-	TOGGLE,
-};
-
-enum Action action = LIST;
-bool json = false;
-char *name = NULL;
-
 struct Output
 {
 	struct wl_list link;
@@ -59,14 +48,39 @@ struct Output
 	struct zwlr_output_power_v1 *wlr_output_power;
 	enum zwlr_output_power_v1_mode mode;
 	char *name;
+	bool operation_failed;
 	uint32_t global_name;
 };
 
+enum Action
+{
+	LIST,
+	OPERATIONS,
+};
+
+enum Power_mode
+{
+	ON,
+	OFF,
+	TOGGLE,
+};
+
+struct Operation
+{
+	struct wl_list link;
+	char *name;
+	enum Power_mode power_mode;
+};
+
+bool json = false;
+bool json_prev = false;
+
 struct wl_display *wl_display = NULL;
 struct wl_registry *wl_registry = NULL;
 struct wl_callback *sync_callback = NULL;
 
 struct wl_list outputs;
+struct wl_list operations;
 
 struct zxdg_output_manager_v1 *xdg_output_manager = NULL;
 struct zwlr_output_power_manager_v1 *wlr_output_power_manager = NULL;
@@ -86,9 +100,7 @@ static void wlr_output_power_handle_mode (void *data, struct zwlr_output_power_v
 static void wlr_output_power_handle_failed (void *data, struct zwlr_output_power_v1 *wlr_output_power)
 {
 	struct Output *output = (struct Output *)data;
-	fprintf(stderr, "ERROR: Setting mode for output \"%s\" failed.\n", output->name);
+	output->operation_failed = true;
-	loop = false;
-	ret = EXIT_FAILURE;
 }
 
 static const struct zwlr_output_power_v1_listener wlr_output_power_listener = {
@@ -156,7 +168,7 @@ static struct Output *output_from_name (const char *str)
 {
 	struct Output *output;
 	wl_list_for_each(output, &outputs, link)
-		if ( strcmp(output->name, name) == 0 )
+		if ( strcmp(output->name, str) == 0 )
 			return output;
 	return NULL;
 }
@@ -209,22 +221,28 @@ static void sync_handle_done (void *data, struct wl_callback *wl_callback, uint3
 	}
 	else if ( sync == 1 )
 	{
-		if ( action == LIST )
+		if (wl_list_empty(&operations))
 		{
+			/* The operations list is empty, so let's just list all
+			 * outputs and their current power mode.
+			 */
 			struct Output *output;
 			if (json)
 			{
-				fputs("[\n", stdout);
+				fputs("[", stdout);
-				uint32_t i = 0, len = (uint32_t)wl_list_length(&outputs);
 				wl_list_for_each(output, &outputs, link)
 				{
-					fprintf(stdout, "  {\n    \"output\": \"%s\",\n    \"power-mode\": \"%s\"\n  }%s\n",
+					fprintf(stdout,
+							"%s\n  {\n"
+							"    \"output\": \"%s\",\n"
+							"    \"power-mode\": \"%s\"\n"
+							"  }",
+							json_prev ? "," : "",
 							output->name,
-							power_mode_to_string(output->mode),
+							power_mode_to_string(output->mode));
-							i < (len - 1) ? "," : "");
+					json_prev = true;
-					i++;
 				}
-				fputs("]\n", stdout);
+				fputs("\n]\n", stdout);
 			}
 			else
 				wl_list_for_each(output, &outputs, link)
@@ -234,17 +252,41 @@ static void sync_handle_done (void *data, struct wl_callback *wl_callback, uint3
 		}
 		else
 		{
-			const struct Output *output = output_from_name(name);
+			/* There are operations in the operations list. We have
+			 * things to do!
+			 */
+
+			if (json)
+				fputs(
+						"{\n"
+						"  \"errors\": [",
+						stdout);
+
+			struct Operation *operation;
+			wl_list_for_each(operation, &operations, link)
+			{
+				const struct Output *output = output_from_name(operation->name);
 				if ( output == NULL )
 				{
-				fprintf(stdout, "ERROR: No output with name \"%s\".\n", name);
+					if (json)
-				ret = EXIT_FAILURE;
+					{
-				loop = false;
+						fprintf(stdout,
-				return;
+								"%s\n    {\n"
+								"      \"output\": \"%s\",\n"
+								"      \"error\": \"output does not exist\"\n"
+								"    }",
+								json_prev ? "," : "",
+								operation->name);
+						json_prev = true;
+					}
+					else
+						fprintf(stderr, "ERROR: Output '%s' does not exist.\n",
+								operation->name);
+					continue;
 				}
 
 				enum zwlr_output_power_v1_mode new_mode;
-			switch (action)
+				switch (operation->power_mode)
 				{
 					case ON:
 						new_mode = ZWLR_OUTPUT_POWER_V1_MODE_ON;
@@ -260,62 +302,168 @@ static void sync_handle_done (void *data, struct wl_callback *wl_callback, uint3
 						else
 							new_mode = ZWLR_OUTPUT_POWER_V1_MODE_ON;
 						break;
-
-				case LIST:
-					/* unreachable */
-					break;
 				}
 
 				zwlr_output_power_v1_set_mode(output->wlr_output_power, new_mode);
+			}
+
 
 			/* We need to sync yet another time because setting the
-			 * power mode might fail.
+			 * power mode might fail and we want to display those
+			 * error messages.
 			 */
 			sync_callback = wl_display_sync(wl_display);
 			wl_callback_add_listener(sync_callback, &sync_callback_listener, NULL);
 		}
 	}
 	else
+	{
+
+		struct Output *output;
+		wl_list_for_each(output, &outputs, link)
+			if (output->operation_failed)
+			{
+				if (json)
+				{
+					fprintf(stdout,
+							"%s\n    {\n"
+							"      \"output\": \"%s\","
+							"      \"error\": \"setting power mode failed\"\n"
+							"    }",
+							json_prev ? "," : "",
+							output->name);
+					json_prev = true;
+				}
+				else
+					fprintf(stderr, "ERROR: Setting power mode for output '%s' failed.\n",
+							output->name);
+			}
+
+		if (json)
+			fputs(
+					"\n  ]\n"
+					"}\n",
+					stdout);
 		loop = false;
+	}
 	sync++;
 }
 
+static void destroy_all_outputs (void)
+{
+	struct Output *output, *tmp;
+	wl_list_for_each_safe(output, tmp, &outputs, link)
+	{
+		if ( output->wlr_output_power != NULL )
+			zwlr_output_power_v1_destroy(output->wlr_output_power);
+		if ( output->xdg_output != NULL )
+			zxdg_output_v1_destroy(output->xdg_output);
+		wl_output_destroy(output->wl_output);
+		wl_list_remove(&output->link);
+		free(output->name);
+		free(output);
+	}
+}
+
+static void destroy_all_operations (void)
+{
+	struct Operation *operation, *tmp;
+	wl_list_for_each_safe(operation, tmp, &operations, link)
+	{
+		wl_list_remove(&operation->link);
+		free(operation->name);
+		free(operation);
+	}
+}
+
+static bool create_operation (const char *name, enum Power_mode power_mode)
+{
+	struct Operation *operation = calloc(1, sizeof(struct Operation));
+	if ( operation == NULL )
+	{
+		fprintf(stderr, "ERROR: calloc: %s\n", strerror(errno));
+		return false;
+	}
+
+	operation->name = strdup(name);
+	if ( operation->name == NULL )
+	{
+		fprintf(stderr, "ERROR: calloc: %s\n", strerror(errno));
+		free(operation);
+		return false;
+	}
+
+	operation->power_mode = power_mode;
+
+	wl_list_insert(&operations, &operation->link);
+	return true;
+}
+
 int main(int argc, char *argv[])
 {
-	switch (argc)
+	wl_list_init(&operations);
+	for (int i = 1; i < argc; i++)
 	{
-		case 1:
+		if ( strcmp(argv[i], "-h") == 0 || strcmp(argv[i], "--help") == 0 )
-			break;
+		{
-
+			destroy_all_operations();
-		case 2:
+			fputs(usage, stderr);
-			if ( strcmp(argv[1], "--json") == 0 )
+			return EXIT_SUCCESS;
+		}
+		else if ( strcmp(argv[i], "-j") == 0 || strcmp(argv[i], "--json") == 0 )
 			json = true;
-			else
+		else if ( strcmp(argv[i], "--on") == 0 )
 		{
-				fputs(usage, stderr);
+			if ( i == argc - 1 )
+			{
+				fputs("ERROR: '--on' needs an output name.\n", stderr);
+				destroy_all_operations();
 				return EXIT_FAILURE;
 			}
-			break;
+			if (! create_operation(argv[i+1], ON))
-
-		case 3:
-			if ( strcmp(argv[1], "on") == 0 )
-				action = ON;
-			else if ( strcmp(argv[1], "off") == 0 )
-				action = OFF;
-			else if ( strcmp(argv[1], "toggle") == 0 )
-				action = TOGGLE;
-			else
 			{
-				fputs(usage, stderr);
+				destroy_all_operations();
 				return EXIT_FAILURE;
 			}
-			name = strdup(argv[2]);
+			i++;
-			break;
+		}
-
+		else if ( strcmp(argv[i], "--off") == 0 )
-		default:
+		{
-			fputs(usage, stderr);
+			if ( i == argc - 1 )
+			{
+				fputs("ERROR: '--off' needs an output name.\n", stderr);
+				destroy_all_operations();
 				return EXIT_FAILURE;
 			}
+			if (! create_operation(argv[i+1], OFF))
+			{
+				destroy_all_operations();
+				return EXIT_FAILURE;
+			}
+			i++;
+		}
+		else if ( strcmp(argv[i], "--toggle") == 0 )
+		{
+			if ( i == argc - 1 )
+			{
+				fputs("ERROR: '--toggle' needs an output name.\n", stderr);
+				destroy_all_operations();
+				return EXIT_FAILURE;
+			}
+			if (! create_operation(argv[i+1], TOGGLE))
+			{
+				destroy_all_operations();
+				return EXIT_FAILURE;
+			}
+			i++;
+		}
+		else
+		{
+			fprintf(stderr, "ERROR: Unknown option '%s'\n", argv[i]);
+			destroy_all_operations();
+			return EXIT_FAILURE;
+		}
+	}
 
 	/* We query the display name here instead of letting wl_display_connect()
 	 * figure it out itself, because libwayland (for legacy reasons) falls
@@ -326,8 +474,6 @@ int main(int argc, char *argv[])
 	if ( display_name == NULL )
 	{
 		fputs("ERROR: WAYLAND_DISPLAY is not set.\n", stderr);
-		if ( name != NULL )
-			free(name);
 		return EXIT_FAILURE;
 	}
 
@@ -335,8 +481,6 @@ int main(int argc, char *argv[])
 	if ( wl_display == NULL )
 	{
 		fputs("ERROR: Can not connect to wayland display.\n", stderr);
-		if ( name != NULL )
-			free(name);
 		return EXIT_FAILURE;
 	}
 
@@ -350,21 +494,9 @@ int main(int argc, char *argv[])
 
 	while ( loop && wl_display_dispatch(wl_display) > 0 );
 
-	struct Output *output, *tmp;
+	destroy_all_operations();
-	wl_list_for_each_safe(output, tmp, &outputs, link)
+	destroy_all_outputs();
-	{
-		if ( output->wlr_output_power != NULL )
-			zwlr_output_power_v1_destroy(output->wlr_output_power);
-		if ( output->xdg_output != NULL )
-			zxdg_output_v1_destroy(output->xdg_output);
-		wl_output_destroy(output->wl_output);
-		wl_list_remove(&output->link);
-		free(output->name);
-		free(output);
-	}
 
-	if ( name != NULL )
-		free(name);
 	if ( sync_callback != NULL )
 		wl_callback_destroy(sync_callback);
 	if ( wlr_output_power_manager != NULL )