Implement conditional container

[sclu1034]

This container allows showing or hiding a child widget based on a boolean property.

I'm not quite sure about the names here. wibox.container.if and a property .condition could work as well.

[codecov]

Codecov Report

Merging #3648 (3b1a80a) into master (02fa372) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3648      +/-   ##
==========================================
+ Coverage   90.65%   90.67%   +0.02%     
==========================================
  Files         854      857       +3     
  Lines       54772    54884     +112     
==========================================
+ Hits        49651    49766     +115     
+ Misses       5121     5118       -3     

Flag	Coverage Δ
gcov	90.67% <100.00%> (+0.02%)	⬆️
luacov	93.47% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/wibox/container/conditional.lua	100.00% <100.00%> (ø)
lib/wibox/container/init.lua	100.00% <100.00%> (ø)
spec/wibox/container/conditional_spec.lua	100.00% <100.00%> (ø)
...xamples/wibox/container/conditional/comparison.lua	100.00% <100.00%> (ø)
spawn.c	86.16% <0.00%> (+0.06%)	⬆️
lib/wibox/container/constraint.lua	89.06% <0.00%> (+1.56%)	⬆️
lib/awful/layout/suit/corner.lua	92.03% <0.00%> (+1.76%)	⬆️

[actionless]

what the difference between setting .state bool of this new container vs setting .visible bool on already existing containers?

[actionless]

(i reopened to unstuck ci)

[Elv13]

Thanks for this.

I gave this a shot a few years ago in #2244 . Then a couple years later tried to convince people a more QML-like approach was the way forward in #3217 . I still think this is generally the way forward, but it will require me (or someone) to find the time to refactor luacheck enough to statically detect the variables defined inside of the object DOM. It's a more flexible design in the long run, because, for example, it can flip between multiple widgets on a complex condition and can directly integrate with signals.

That being said, we already have awful.widget.only_on_screen and this is a more generic version of this. So for the name, I would suggest something narrower like wibox.container.visible_when? Also, is it possible to refactor only_on_screen to be rebased on top of this module?

[sclu1034]

what the difference between setting .state bool of this new container vs setting .visible bool on already existing containers?

The same difference as setting .visible vs calling :remove()/:insert(). The container isn't just visibly empty, the child widget is removed from the hierarchy completely.

And as outlined in #3531, the handling of .visible isn't all that great currently.

Because of that, I'm also not entirely comfortable naming it wibox.container.visible_when. wibox.container.when would be fine, though.

Also, is it possible to refactor only_on_screen to be rebased on top of this module?

After a quick skim, it seems to be doing the same thing as this one, so I don't see anything that would prevent that refactor.

[actionless]

The container isn't just visibly empty, the child widget is removed from the hierarchy completely.

ok, i see.

now since it makes sense to discuss it further - i see the major difference between your PR and Elv's previous approaches:
you having it as a just static bool value, while Elv's code allows to set a function there

[sclu1034]

What's still needed here? As far as I can tell:

1. A decision/vote on the name:

• wibox.container.conditional: Name clash with Elv's variant
• wibox.container.if: Should make the functionality pretty clear. Could synergize with naming Elv's variant wibox.container.switch
• wibox.container.when: Same as if, in my eyes.
• wibox.container.visible_when: Incorrect on a technical level, since .visible is not what we're changing.

2. A decision/vote whether we want

• only this variant
• only Elv's variant
• both

[Elv13]

Names:

• wibox.container.if: Clash with reversed keyword and syntax highlight
• wibox.container.switch: Clash with some syntax highlight and potentially Lua might use it in the future.
• wibox.container.when: No objection.

A decision/vote whether we want:

Both IMHO, my version is complex and hard to merge. Your version is simple and fix a real problem.

Other question:

• Is state the best property name. Given it's a boolean, maybe enabled would make more sense? Or can we use strings states and have more than 2 (more than 1 type of child widget).
• Does it make sense to merge this and [RFC] Implement wibox.widget.template: An abstract widget that handles a preset of concrete widget. #3421 ? They kind of serve different use case, but overlap. Lazy initialization and widget_template compatibility makes it more useful. It makes this a better base class and there is some overlap in "future additions". Then again, it might make sense to keep them separated.

[sclu1034]

wibox.container.when: No objection.

Both IMHO, my version is complex and hard to merge. Your version is simple and fix a real problem.

maybe enabled would make more sense?

Fine with me.

Does it make sense to merge this and wibox.widget.template?

I don't think so. I don't see any major use case where it would be worth having both functionalities in one widget.

And, as you said, currently this container is quite simple to use. If they were merged, you'd need to bring the entire templating logic, unused, when you only wanted the simple if functionality.