A -theme-str with two quoted strings reads them as one string

[rokokol]

When a -theme-str holds two quoted strings, rofi reads them as one string. The string runs from the first quote to the last quote on the line, and the second property is lost.

Steps to reproduce

rofi -no-config \
  -theme-str 'element-text { font: "Sans 10"; } textbox { font: "Mono 12"; }' \
  -dump-theme | grep 'font:'

What I see

element-text gets one long font string, and textbox gets no font:

    font:             "Sans 10"; } textbox { font: "Mono 12";

The same two rules in a file, one on each line, give two fonts:

printf 'element-text { font: "Sans 10"; }\ntextbox { font: "Mono 12"; }\n' >two.rasi
rofi -no-config -theme two.rasi -dump-theme | grep 'font:'

    font: "Sans 10";
    font: "Mono 12";

With two url("…") values in one -theme-str, rofi tries to load an image with that whole span as its path and fails.

What I expected

element-text gets "Sans 10" and textbox gets "Mono 12", as when the same text is in a theme file with one property on each line.

Why it happens

STRING in lexer/theme-lexer.l is \"{UANYN}*\", and UANYN excludes only the newline, not the quote. flex takes the longest match, so a string ends at the last quote on its line. A theme file usually has one string on each line, so this does not show there. A -theme-str is one line.

This looks deliberate: test_properties_string_escape in test/theme-parser-test.c expects "aap" noot" mies " to be one string, with the inner quotes kept. So I am not sure whether this is a bug or the intended trade-off, and I did not want to send a patch that breaks that test.

Some options I can see:

• keep the rule, and say in rofi-theme(5) that a string ends at the last quote on its line, so -theme-str needs \n between strings
• end a string at the first unescaped quote, as STRING_LIST already does, and require \" for a quote inside a string. This changes test_properties_string_escape and themes that rely on bare inner quotes

Which one would you prefer? I can send a pull request for either.

Version: rofi 2.0.0 (also the current next, where the rule is the same).

[DaveDavenport]

Hmm that indeed does not behave as expected on the commandline.
I remember that there was a reason to make it greedy (not sure what anymore).

I need to think about a solution that causes the least breakage for users.

[rokokol]

Thank you. I think I found the reason for the greedy match.

-dump-config and -dump-theme write a string without escapes. xrmoptions.c has a TODO should this be escaped? on that line. Thus a command with quotes in it dumps like this:

$ rofi -no-config -run-shell-command '{terminal} -e sh -c "{cmd}; read"' -dump-config | grep run-shell
	run-shell-command: "{terminal} -e sh -c "{cmd}; read"";

Only a greedy string reads this line back. -dump-config came on 2017-06-21 (77a4d97), and test_properties_string_escape on 2017-06-26 (b9c624d). So I think the test protects this round trip. Many config.rasi files came from -dump-config, so a strict non-greedy string can break them.

I tried a fix with less breakage on a local branch:

1. The dumps escape \ and " in a string. Each string then reads back as the same value, also with a stricter lexer.
2. The lexer keeps the greedy STRING rule. In the action, the string ends at the first unescaped quote that has optional blanks and then ;, , or ). yyless() gives the remainder back to the lexer.

What I checked:

• the example from my post gives "Sans 10" and "Mono 12"
• two url("…") values in one -theme-str parse as two values
• test_properties_string_escape passes without a change, so "aap" noot" mies " is still one string
• an old unescaped dump, such as "{terminal} -e sh -c "{cmd}; read"", reads as before
• a new dump reads back as the same value
• meson test passes, and a new test for strings on one line fails without the lexer change

One case changes: a value with a bare quote and then ; inside it, such as "sh -c "a"; b". It now ends after "a", so this value needs \". I can add one sentence about this rule to rofi-theme(5).

I also saw a small separate bug. -dump-theme prints url ("(null)") for an image, because theme.c prints p->value.s and not p->value.image.url.

Do you want one pull request, or two (the dump escape, then the lexer change)?

[DaveDavenport]

Thanks for diving into this, I've did not have time yet.

One PR is good.

Thanks!