Inconsistent API accross *bar, *graph and *gauge

[MattSturgeon]

Proper syntax (seems) to be variable [variable specific attributes] [dimensions] [graph settings] however there are a number of exceptions to this...

The one exception may be the lua_* variables, who take an undefined number of arguments. execgraph solves this problem by wrapping multiword commands in quotes.

Having just searched through the docs:

• apcupsd_loadbar, mixerbar, mixerlbar and mixerrbar do not have (documented) dimensions attributes at all(!)
• battery_bar uses (height),(width) (num) instead of (num) (height),(width)
• fs_bar and fs_bar_free use (height),(width) fs
• wireless_link_bar uses (height),(width) (net)
• execbar and execgauge use (height),(width) command instead of command (height),(width)
  • execgraph is correct with command (height),(width) (gradient color 1) (gradient color 2) (scale) (-t) (-l)
• execibar and execigauge use interval (height),(width) command, so it's half right!
  • execigraph has no problems
• lua_bar and lua_gauge use _(height, width) functionname (function parameters)
  • `lus_graph uses _functionname (height),(width) (gradient colour 1) (gradient colour 2) (scale) (-t) (-l) note no function parameters...

For what it's worth, my currently proposed nvidiabar/gauge/graphs follow the exec* example and use (height),(width) command for bar and gauge, and command (height),(width) [graph-stuff] for the graph

[marcpayne]

:+1:

I'm glad you brought this up. On one hand, I like your example of proper syntax, where variable specific attributes come first. This makes good intuitive sense. On the other hand, parsing an arbitrary number of attributes (or a single attribute with spaces) is more difficult this way. As you noted, execgraph solves this with double quotes [1]. The *bar and *gauge variables are nice in that we look for height or height,width and take whatever is left as variable specific attributes. No quotes needed.

One possible long-term solution is named arguments like you might see in Python. This would eliminate the problem of an inconsistent API since users could choose whatever attributes they want in whatever order. It would also make parsing much more straightforward. For example:

lua_graph function="foo arg1 arg2 ...", width=200, height=50, scale=60, log=true

This graph can take a function with any number of arguments and it is very clear what each value means. A couple downsides to this approach are the extra verbosity and the fact that this would require a HUGE backward-incompatible rewrite. To be totally consistent, we would have to make every conky variable accept named arguments. I would consider this a good reason for a 2.0 version bump.

These are my thoughts on the matter for now. I look forward to seeing what other solutions people come up with! :)

[1] It's worth noting that execgraph is a special case. The code that handles quoted commands is part of the general-purpose scan_graph() function. IMO, scan_graph() shouldn't even be aware that execgraph exists. It should handle graph attributes, and nothing else.

[MattSturgeon]

I agree key=value arguments is a good idea.

Theoretically this could be as simple as parsing the generic(recognised) keys as normal and leaving unrecognised keys in something like an array of key=value pairs for the object's own functions to deal with.

Specific object implementations could then use a generic getter functions to avoid duplication things like throwing errors for required arguments, checking if a key has a value/is defined, getting the value, etc.

This may even simplify the code if I'm not mistaken... disclaimer: I have a very limited understanding of conky's code/architecture and haven't looked at this idea properly at all!

You could potentially even think about offering a "legacy parse" function for backwards compatibility with the old syntax, though I'm unsure how feasible, maintainable or even desirable this would be...

[mxmlnkn]

This may even simplify the code

I think so too. Another thing which should be simplified is, using std::list instead of C [linked list] arrays. I just saw them in fs.cc when trying to solve another issue. Also an overview for the architecture would be nice to make it easier for people like me to solve issues. It's hard to follow the logic with all those callbacks, ... About legacy support. I personally found the new LUA-like config-syntax already rather confusing... Instead of supporting old legacy arguments, I find an automatic conversion script like it exists for old-to-lua-syntax better. It reduces the complexity of the 'real' code. The key=value sounds rather cool, but also very verbose. Maybe like python a default order should be allowed anyway, e.g. I often use: plot(x,y,label='test'), I wouldn't want to write plot( x=x, y=y, label='test')

[ghost]

Python like syntax: I agree with marcpayne this is very disruptive and is a good reason for a version bump (and probably not release it any time soon), people are still digesting 1.10.

As mxmlnkn said, this would require a convetor script to make it easier for the end users. I would indeed also avoid legacy support. This might complicate things and people will use it as long as you support it (people in general don't like change).

The verbosity would indeed be a problem, the suggestion by mxmlnkn would help. But we have to be careful since, in my opinion, the conky files are now already not that easy to read (certainly if they become big).

I'm also wondering if it's worth it (python like syntax), the original problem is inconsistent syntax (for the mentioned variables). Python like sytnax would bring flexibility and cleaner code. With the cost of a disruptive change and a lot of work. I would only shift the balance to python like systnax if the code becomes significant cleaner or allows for new (now impossible) features (which I don't see at the moment). Otherwise I doubt it would be worth it.

I also assume this would mean you apply the new syntax to all the variables, since it would make little sense to make it available only for bar, graph and *gauge.

---

I disagree that "specific variables first" is more intuitive. I find mandatory first more consistent (and intuitive). For example:

plotA(x, y, option1, option2)
plotB(x, y, option1)
plotC(x, y, option1, option2, option3)

If you know the plot function you already know it needs x and y so you can write them first. It allows you to glance over calls quickly; if you only care about the options you can easly skip to them or vice versa for the mandatory options you can stop look after the last one. With the specific options in front of them you always need to "search" for the default ones since they potentially could appear "everywhere". Whether or not you fix the order of the options, is with this approach, less of a concern for me.

---

An alternative method would be to use the syntax of ${image ...}, image <path to image> (-p x,y) (-s WxH) (-n) (-f interval). This is less verbose as python like syntax (also less flexible) but allows parsing the optional arguments easiliy if your want them in front of the mandatory arguments. This also would make the purpose of the parameters more clear in the conky config.

---

Architecturally I agree with the comment on scan_graph() from marcpayne.

Sorry for the long post.

[ghost]

New suggestion!

Make a single bar, graph and gauge variable with syntax:

Doc: bar [text] (-h height) (-w width)
${bar nvidia gpuutil -h 30 -w 100}
${bar swapperc -w 120 }
${bar nvidia memfrq}
(analogue for graph and grauge)

because not all variable output is in range of 0-100 (and users will probably try invalid ones) we could enforce a specialized syntax; i would suggest adding the suffix '_perc'. So for example nvidia_perc gpuutil would be valid to use but nvidia memfreqmin would result in an error.

Advantages:

• Reduce work for newly added features and modules, not every module (eg. music players, nvidia, cpu, mem, swap, ....) must implement their own keyword (e.g. cpubar, ...) -> less work for devs.
• Unified syntax. Users will only use 'bar' and due to the '_perc' suffix they know what variables they may use (of course be very precise in the documentation about this)
• Solves the syntax inconsistency and simplifies things.
• Way easier to implement new visualisations (e.g. an arc bar)
• Way more concise documentation

Disadvantages:

• The '_perc' suffix might result is some unnecessary syntax/variables. For example you would have: ${audacious_volume} and ${audacious_volume_perc}. Although you could drop the first one since technically audacious_volume could return the db values; adding the '_perc' is just more explicit.
• No backwards compatibility (deprecating nvidiabar, cpubar, membar, .... all of them)
• A lot of work

---

A the moment i'm in the process of updating the documentation (see my branch) and the amount of inconsistent syntax, for variables, is staggering:

• module [option] (e.g. nvidia gpuutil)
• module_option (e.g. audacious_volume)
• moduleoption (e.g. swapperc)

I personally favour the first one with snake case for variants e.g. nvidia and nvidia_perc (for the suggested new syntax). Also snake case for the options; e.g. ${nvidia_perc gpu_util}

If we would unify the syntax across the board (completely breaking backwards compatibility) this might be worth a major version bump. (in the process we could eliminate old style code )

Thoughts?

[github-actions[bot]]

This issue is stale because it has been open 365 days with no activity. Remove stale label or comment, or this issue will be closed in 30 days.

[github-actions[bot]]

This issue was closed because it has been stalled for 30 days with no activity.