Fix and test graph parsing

[bi4k8]

Fixes #1428.

Checklist

• I have described the changes
• I have linked to any relevant GitHub issues, if applicable
• Documentation in doc/ has been updated
• All new code is licensed under GPLv3

Description

First I did a couple cleanups (adding comments and moving an enveloping if into an early-return) to scan_graph to make it easier to read and edit. Then I reorganized the cases to correct the misparsing that was introduced in 5e98c49. But I still wasn't happy with how the code looked, so I wanted to simplify it further. I added a test case that exercises the parser on its own, then separated out parsing of a quoted command prefix from scan_graph, as it's a separate concern and not all callers even accept a command prefix (and some consider one optional, such as the net graphs). This cuts the number of cases that scan_graph has to handle in half.

I tested against a few conkyrc files from #1428 to verify that this PR corrects the regression, but more testing would be appreciated as it seems this impacted quite a few people (sorry!). This PR shouldn't change any behavior other than fixing the bug, but because it does touch all users of graphs, it would be good to give it some fairly thorough testing.

[netlify]

✅ Deploy Preview for conkyweb canceled.

Name	Link
🔨 Latest commit	b3ad577
🔍 Latest deploy log	https://app.netlify.com/sites/conkyweb/deploys/64052d957454ac000886b06c

[miblodelcarpio]

Nice work, @bi4k8. This lets me omit the width / height from my ${upspeedgraph} and ${downspeedgraph} declarations and have those values picked up from default_graph_height and default_graph_width as they were before.

I do, however, find that variables nested within those declarations fail to parse. Is this intended? Here are example declarations that fail:

${downspeedgraph enp0s31f6 000000 ${color4} 10000 -t -l}
${downspeedgraph enp0s31f6 000000 $color4 10000 -t -l}

The new behaviour is actually better than 1.17.0-1 in that it draws the graph in a default red, rather than throwing an X Error: Display 55db4fdabe20 and displaying garbage.

[bi4k8]

Nice work, @bi4k8. This lets me omit the width / height from my ${upspeedgraph} and ${downspeedgraph} declarations and have those values picked up from default_graph_height and default_graph_width as they were before.

I do, however, find that variables nested within those declarations fail to parse. Is this intended? Here are example declarations that fail:

${downspeedgraph enp0s31f6 000000 ${color4} 10000 -t -l} ${downspeedgraph enp0s31f6 000000 $color4 10000 -t -l}

The new behaviour is actually better than 1.17.0-1 in that it draws the graph in a default red, rather than throwing an X Error: Display 55db4fdabe20 and displaying garbage.

Unfortunately, you can't arbitrarily substitute Conky "variables" the way you might expect from the name. ${color1} is more of a command than a variable, so it doesn't work to place it where another command expects a color name. It certainly isn't a bad idea to support this, as it would be intuitive, but I don't think this functionality existed so far.

A workaround for what you're doing is to use templates, which are substituted the way you would expect:

conky.config = {
...
  template1 = "00ff00",
}
conky.text = [[
${downspeedgraph wlan0 50,200 000000 ${template1} 1000 -t}
]]

[miblodelcarpio]

Wonderful, cheers for the info, @bi4k8. And this template workaround is golden! I guess pretty much all of us from #1428 may as well benefit from this at least for the interface name.