output/background: fix config ignoring fallback color #8558
Conversation
currently, the output background command handler prematurely returns with an error if the background file cannot be accessed. It should only error if user did not provide fallback color. closes swaywm#8556
|
cc @mstoeckl |
There was a problem hiding this comment.
I've tested this a bit; while it works when the file does not exist, I find that it fails to parse the color argument when the file is accessible; swaymsg output '*' background ~/test.png center '#ff00ff' gives an "Error: invalid output subcommand: #ff00ff".
Also, there's a preexisting issue in sway/commands/output/background.c that, while I don't think you need to fix it entirely, should not be exacerbated: when updating variables like output->background_option, the previous value should be free()d to avoid leaking it, and if strdup() fails the function should return with an error message (like return cmd_results_new(CMD_FAILURE, "Unable to allocate resource");.)
| @@ -117,23 +117,13 @@ struct cmd_results *output_cmd_background(int argc, char **argv) { | |||
| } | |||
|
|
|||
| bool can_access = access(src, F_OK) != -1; | |||
| if (!can_access) { | |||
| sway_log_errno(SWAY_ERROR, "Unable to access background file '%s'", | |||
There was a problem hiding this comment.
This error message, and maybe also the swaynag warning, would be useful to keep in the case where the file cannot be accessed and there is a fallback color.
There was a problem hiding this comment.
I agree it would be useful to log this to SWAY_DEBUG or SWAY_INFO, but it might be obstructive to swaynag it. Like if I provided a fallback color and then it ended up using that fallback color because it couldn't access the file, then I would not want to be bothered about it because it's working as I instructed it to.
1. to avoid uneccessary writing on output members 2. log a debug message when fallback is being used over inaccessible file 3. always parse the background color and swaynag warn if it is incorrect
|
To outline what I was going for, I present 4 cases:
|
As suggested by @mstoeckl 1. when updating output member variables, free previous values 2. add cleanup label and goto it if `strdup` fails
currently, the output background command handler prematurely returns with an error if the background file cannot be accessed. It should only error if user did not provide fallback color.
closes #8556