output/background: parse background mode into enum

[mstoeckl]

This change parses the background mode in the output ... background ... command instead of validating the mode string and then copying it around. Doing so avoids a number of strdup() calls and associated free()s, making the logic and failure handling paths in output_cmd_background() somewhat easier to follow.

It turns out this change also fixes a mismatch between the way the background subcommand parsed modes (case insensitively) and the way swaybg does (exact match). Previously both

swaymsg output '*' background ~/Sway_Tree.png center
swaymsg output '*' background ~/Sway_Tree.png CENTER

would be accepted by sway, but swaybg would only use the centering mode on the first image, and would print a warning on the second.

Notes:

• The commit keeps the case insensitive parsing behavior, even though it did not work properly before, because many Sway commands already accept keywords case-insensitively. (Some do, some don't; I'm not sure which should be preferred.)
• enum background_mode is AFAIK the first enum without a real _DEFAULT behavior, so I've named the null case _UNSET. Is there an existing convention for this?
• The background_mode_names table is used in sway/commands/output/background.c (for parsing) and sway/config/output.c (debug output and spawn_swaybg()); I've put it in the former file, but am not certain about the right place for it.
• A similar "parse-instead-of-validating" change may be worth doing for the background color, but most approaches would complicate spawn_swaybg(), which currently allocates an array of pointers to strings with a longer lifespan -- parsing and then reprinting colors would require temporary allocations or per-output_config scratch space to hold the colors. There are other solutions, but none I've found to be clean and simple so far.

Edit: looks like there are compile werrors because execvp takes char *const argv[], not const char *const argv[]; I may need to rethink this, depending on what execvp does. Edit 2: fixed, execvp does not modify its input and its signature is constrained by history per Rationale section of man 3p execvp.

[emersion]

Overall looks good to me!

The commit keeps the case insensitive parsing behavior, even though it did not work properly before, because many Sway commands already accept keywords case-insensitively. (Some do, some don't; I'm not sure which should be preferred.)

Sounds safer to keep previous behavior (even though if I had to start from scratch I'd probably lean on rejecting strings with a different case).

enum background_mode is AFAIK the first enum without a real _DEFAULT behavior, so I've named the null case _UNSET. Is there an existing convention for this?

I think we use NONE elsewhere, not sure it's exactly the same case? UNSET sounds fine to me regardless.

The background_mode_names table is used in sway/commands/output/background.c (for parsing) and sway/config/output.c (debug output and spawn_swaybg()); I've put it in the former file, but am not certain about the right place for it.

That sounds fine to me.

A similar "parse-instead-of-validating" change may be worth doing for the background color, but most approaches would complicate spawn_swaybg(), which currently allocates an array of pointers to strings with a longer lifespan -- parsing and then reprinting colors would require temporary allocations or per-output_config scratch space to hold the colors. There are other solutions, but none I've found to be clean and simple so far.

We could maybe use a buffer on the stack for the color?

[mstoeckl]

We could maybe use a buffer on the stack for the color?

There can be many output_configs and many colors that get passed to swaybg, so storing them all on the stack can potentially overflow it.

[emersion]

Oh, right, didn't notice the loop! Maybe not worth the trouble then…