cmds:output: fix quoted defs not stripped

[paul-ri]

Someone found the issue on here: https://www.reddit.com/r/swaywm/comments/kr2b42/use_variable_in_swaymsg/
After investigating:

With the following config:

def $abc "wayland wayland "

The following command would fail to be executed:

swaymsg -- output '$abc' scale 2

The definition $abc is replaced with it content, including the quotes,
which fails to match real output names.

We now strip quotes early in the output command.

[emersion]

Doesn't the issue also apply to commands that aren't output commands? Like this for instance:

swaymsg -- input '$abc' events disabled

[rpigott]

By def $abc do you mean set $abc?

The following works for me:

$ swaymsg -s $TESTSOCK set '$screen' '"wayland wayland "'
$ swaymsg -s $TESTSOCK output '$screen' scale 2

and records in the debug output:

[sway/commands.c:255] Handling command 'set $screen "wayland wayland "'
[...]
[sway/commands.c:255] Handling command 'output $screen scale 2'
[...]
[sway/config/output.c:227] Config stored for output wayland wayland  (enabled: -1) ...

Similarly, the following works in the config because the quotes are handled later in config_command:

set $screen "wayland wayland "
output $screen scale 2

What doesn't work: set $screen "wayland wayland " in the config and

$ swaymsg output '$screen' scale 2 # matches literal output '"wayland wayland "'

because then the quotes are never stripped.

We would want to specify the value with a trailing space and no extra quotes, but in this example the value of $screen should be "wayland wayland " it is "wayland wayland" instead

# the following line has a trailing space that sway discards and i3 does not
set $screen wayland wayland 

I'd say the real bug is:

1. sway's quote handling is inconsistent between swaymsg and the config config
2. sway's parser is incompatible with i3's definition of the set command. i3 interprets every character between the first space following the identifier and the final newline as the variable value, whereas sway parses the line and stitches tokens back together to form the variable value, possibly discarding some whitespace.

Stripping quotes around device identifiers in input/output doesn't technically address either of these issues, but it might still be a decent idea to change those commands under the assumption a real device identifier will never contain quotes.

[paul-ri]

Thanks! I was hoping exactly for this kind of insight from someone in the know.

By def $abc do you mean set $abc?

Indeed! My bad.

the assumption a real device identifier will never contain quotes

I'll try and investigate if there's a way to validate that assumption somehow if I can.

That or strip quotes from the output name too.

I won't delve into addressing the real bugs you're mentioning though.

[paul-ri]

@rpigott

The latest changes don't address the "real bug" you have mentioned.

I have updated the commit message for better details.

It's an attempt at keeping a consistent behaviour for cmd_output & cmd_input whether or not the command is from the config loading or execute_command.

Thanks for the pointer about config_command. I found out that it was stripping the quotes anyways for output/input commands. So it definitely felt like it was a good idea to stop that and leave it to cmd_output & cmd_input to handle the formatting of their inputs.

Let me know if you'd do it any differently.

@emersion : stripping quotes for cmd_input. The command was broken there too. Cheers!

[emersion]

stripping quotes for cmd_input. The command was broken there too

Well, input was just an example. This patch doesn't fix the underlying issue, other commands are still broken.

[emersion]

In other words: I'd prefer to see a patch that fixes the underlying issue rather than add a workaround for each command argument that might contain spaces.

[rpigott]

Yes, we should address the swaymsg issue and i3 compatibility first.

Basically, input/output commands don't exist in i3, so I figure there is a chance that using strict i3-style parsing may make them cumbersome to use, or render previously reasonably quoted commands in users' configs unusable. That is undesirable and we have the freedom to change the command to avoid that, if needed/wanted. But we should fix these issues first.

The "wayland wayland " display is a potential example. Requiring the user to keep a trailing space on their set line seems pretty lame, but that's how set works in i3 so that's how set should work in sway. Changing the definition of an input/output identifier to strip quotes is just one possible remedy to consider, if we want.