kanshi: Add support for setting output name variables v2 NEEDS REVISION

Hi!

This patch adds support for variables for output names to kanshi. This
allows setting short readable names for long output names that would
otherwise be pretty unreadable and makes longer configs much more
readable.

This is accomplished by adding a new directive `set $variable "variable
value"` and a list of variables in the `kanshi_config` struct. Each time
an output name is parsed in `parse_profile_output()`, if the name starts
with a dollar sign, the value of the variable with that name is used
instead.

There is currently no way to escape the dollar sign for outputs whose
name starts with a dollar sign, but I have never actually seen that in
practice. If support for escaping the dollar sign is required for you to
accept this feature, I will add it in V3 of this patch series.

Changes since V1: adjusted to use the libscfg parser

This patch set implements https://todo.sr.ht/~emersion/kanshi/106

Ferdinand "yrlf" Bachmann

Ferdinand Bachmann (1):
  Add support for setting output name variables

 doc/kanshi.5.scd |  3 +++
 include/config.h |  7 +++++
 parser.c         | 68 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 73 insertions(+), 5 deletions(-)

-- 
2.38.5

kanshi/patches/.build.yml: FAILED in 22s

[Add support for setting output name variables][0] v2 from [~yrlf][1]

[0]: https://lists.sr.ht/~emersion/public-inbox/patches/49136
[1]: mailto:me@yrlf.at

✗ #1142222 FAILED kanshi/patches/.build.yml https://builds.sr.ht/~emersion/job/1142222

Sorry for the delay.

I think finding a solution to this use-case is definitely a good idea,
but I'm wondering what the best solution is.

A generic "set" command makes it sound like this will work just like
regular variable substitution, with features we don't really need such
as replacement of a part of the output name, and replacement throughout
the whole config file (not just for the output name). One may expect the
following to "work", but it won't, and won't print any obvious error:

    set $foo bar
    profile {
        output "$foo 4242" mode 1920x1080
        exec "echo $foo"
    }

Maybe a less surprising syntax would be to have a dedicated way to setup
an output alias?

    output DP-1 alias main
    profile {
        output main mode 1920x1080
    }

Something surprising would be that the "output" command syntax would be
different inside and outside the "profile" directive. (That said, it may
be desirable to make the syntax outside of the "profile" directive a
superset of the syntax inside, so that defaults can be set -- but this
requires more thought.)

yrlf <me@yrlf.at>

6 months ago

I can try implementing the 'output X alias Y' syntax and see how that 
works out. The output alias directive should IMO only be recognized on 
the top level, outside any profiles.

Simon Ser <contact@emersion.fr>

6 months ago

Agreed!

Next

The output defaults on the top level is an interesting idea. This could 
be useful in the future for things like setting a default mode and scale 
for specific outputs so that profiles don't need to repeat themselves.

Next

Any other solution ideas?

yrlf <me@yrlf.at>

6 months ago

An alternative to overloading the "output" syntax would be to do it the 
other way round and have a dedicated alias command, though I'm not sure 
if that would be better. I think I'll go forward with implementing 
'output X alias Y' on the top level first.

Hi!

(sorry for the duplicate; new mail since I forgot to CC the mailing list 
and accidently top-posted)

You're correct, a generic set command is gonna lead users to believe it 
would be usable elsewhere as well. I like the "alias" terminology.

[PATCH kanshi v2 1/1] Add support for setting output name variables Export this patch

From: Ferdinand Bachmann <me@yrlf.at>

Fixes: https://todo.sr.ht/~emersion/kanshi/106
---
 doc/kanshi.5.scd |  3 +++
 include/config.h |  7 +++++
 parser.c         | 68 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/doc/kanshi.5.scd b/doc/kanshi.5.scd
index 88d16b3..1b0ac25 100644
--- a/doc/kanshi.5.scd
+++ b/doc/kanshi.5.scd
@@ -37,6 +37,9 @@ profile nomad {
 	Include as another file from _path_. Expands shell syntax (see *wordexp*(3)
 	for details).
 
+*set* $<name> <value>
+	Defines a new variable that can be used in output names.
+
 # PROFILE DIRECTIVES
 
 Profile directives are followed by space-separated arguments. Arguments can be
diff --git a/include/config.h b/include/config.h
index 047f05b..b49a4eb 100644
--- a/include/config.h
+++ b/include/config.h
@@ -45,7 +45,14 @@ struct kanshi_profile {
 	struct wl_list commands;
 };
 
+struct kanshi_variable {
+	struct wl_list link;
+	char *name;
+	char *value;
+};
+
 struct kanshi_config {
+	struct wl_list variables;
 	struct wl_list profiles;
 };
 
diff --git a/parser.c b/parser.c
index 2cccc08..a26833f 100644
--- a/parser.c
+++ b/parser.c
@@ -194,14 +194,31 @@ static ssize_t parse_profile_output_param(struct kanshi_profile_output *output,
 }
 
 static struct kanshi_profile_output *parse_profile_output(
-		struct scfg_directive *dir) {
+		struct scfg_directive *dir, struct kanshi_config *config) {
 	if (dir->params_len == 0) {
 		fprintf(stderr, "directive 'output': expected at least one param\n");
 		return NULL;
 	}
 
 	struct kanshi_profile_output *output = calloc(1, sizeof(*output));
-	output->name = strdup(dir->params[0]);
+
+	if (dir->params[0][0] == '$') {
+		struct kanshi_variable *variable;
+		wl_list_for_each(variable, &config->variables, link) {
+			if (strcmp(dir->params[0], variable->name) == 0) {
+				output->name = strdup(variable->value);
+				break;
+			}
+		}
+
+		if (output->name == NULL) {
+			fprintf(stderr, "Undefined variable '%s'\n", dir->params[0]);
+			free(output);
+			return NULL;
+		}
+	} else {
+		output->name = strdup(dir->params[0]);
+	}
 
 	size_t i = 1;
 	while (i < dir->params_len) {
@@ -266,7 +283,8 @@ static struct kanshi_profile_command *parse_profile_exec(
 	return command;
 }
 
-static struct kanshi_profile *parse_profile(struct scfg_directive *dir) {
+static struct kanshi_profile *parse_profile(struct scfg_directive *dir,
+		struct kanshi_config *config) {
 	struct kanshi_profile *profile = calloc(1, sizeof(*profile));
 	wl_list_init(&profile->outputs);
 	wl_list_init(&profile->commands);
@@ -292,7 +310,7 @@ static struct kanshi_profile *parse_profile(struct scfg_directive *dir) {
 
 		if (strcmp(child->name, "output") == 0) {
 			struct kanshi_profile_output *output =
-				parse_profile_output(child);
+				parse_profile_output(child, config);
 			if (output == NULL) {
 				return NULL;
 			}
@@ -345,12 +363,39 @@ static bool parse_include_command(struct scfg_directive *dir, struct kanshi_conf
 	return true;
 }
 
+static bool parse_set_command(struct scfg_directive *dir, struct kanshi_config *config) {
+	if (dir->params_len != 2) {
+		fprintf(stderr, "directive 'set': expected exactly two parameters\n");
+		return false;
+	}
+
+	if (dir->params[0][0] != '$') {
+		fprintf(stderr, "Invalid variable name '%s', must start with $\n", dir->params[0]);
+		return false;
+	}
+
+	// Check for duplicate variable name
+	struct kanshi_variable *variable;
+	wl_list_for_each(variable, &config->variables, link) {
+		if (strcmp(dir->params[0], variable->name) == 0) {
+			fprintf(stderr, "Redefinition of variable '%s'\n", variable->name);
+			return false;
+		}
+	}
+
+	variable = calloc(1, sizeof(*variable));
+	variable->name = strdup(dir->params[0]);
+	variable->value = strdup(dir->params[1]);
+	wl_list_insert(&config->variables, &variable->link);
+	return true;
+}
+
 static bool _parse_config(struct scfg_block *block, struct kanshi_config *config) {
 	for (size_t i = 0; i < block->directives_len; i++) {
 		struct scfg_directive *dir = &block->directives[i];
 
 		if (strcmp(dir->name, "profile") == 0) {
-			struct kanshi_profile *profile = parse_profile(dir);
+			struct kanshi_profile *profile = parse_profile(dir, config);
 			if (!profile) {
 				return false;
 			}
@@ -359,6 +404,10 @@ static bool _parse_config(struct scfg_block *block, struct kanshi_config *config
 			if (!parse_include_command(dir, config)) {
 				return false;
 			}
+		} else if (strcmp(dir->name, "set") == 0) {
+			if (!parse_set_command(dir, config)) {
+				return false;
+			}
 		} else {
 			fprintf(stderr, "unknown directive '%s'\n", dir->name);
 			return false;
@@ -389,6 +438,7 @@ struct kanshi_config *parse_config(const char *path) {
 	if (config == NULL) {
 		return NULL;
 	}
+	wl_list_init(&config->variables);
 	wl_list_init(&config->profiles);
 
 	if (!parse_config_file(path, config)) {
@@ -400,6 +450,14 @@ struct kanshi_config *parse_config(const char *path) {
 }
 
 void destroy_config(struct kanshi_config *config) {
+	struct kanshi_variable *variable, *tmp_variable;
+	wl_list_for_each_safe(variable, tmp_variable, &config->variables, link) {
+		free(variable->name);
+		free(variable->value);
+		wl_list_remove(&variable->link);
+		free(variable);
+	}
+
 	struct kanshi_profile *profile, *profile_tmp;
 	wl_list_for_each_safe(profile, profile_tmp, &config->profiles, link) {
 		struct kanshi_profile_output *output, *output_tmp;
-- 
2.38.5

builds.sr.ht <builds@sr.ht>

7 months ago

kanshi/patches/.build.yml: FAILED in 22s

[Add support for setting output name variables][0] v2 from [~yrlf][1]

[0]: https://lists.sr.ht/~emersion/public-inbox/patches/49136
[1]: mailto:me@yrlf.at

✗ #1142222 FAILED kanshi/patches/.build.yml https://builds.sr.ht/~emersion/job/1142222

Simon Ser <contact@emersion.fr>

6 months ago

Sorry for the delay.

I think finding a solution to this use-case is definitely a good idea,
but I'm wondering what the best solution is.

A generic "set" command makes it sound like this will work just like
regular variable substitution, with features we don't really need such
as replacement of a part of the output name, and replacement throughout
the whole config file (not just for the output name). One may expect the
following to "work", but it won't, and won't print any obvious error:

    set $foo bar
    profile {
        output "$foo 4242" mode 1920x1080
        exec "echo $foo"
    }

Maybe a less surprising syntax would be to have a dedicated way to setup
an output alias?

    output DP-1 alias main
    profile {
        output main mode 1920x1080
    }

Something surprising would be that the "output" command syntax would be
different inside and outside the "profile" directive. (That said, it may
be desirable to make the syntax outside of the "profile" directive a
superset of the syntax inside, so that defaults can be set -- but this
requires more thought.)

Any other solution ideas?