eww icon indicating copy to clipboard operation
eww copied to clipboard

[BUG] run-while on defpoll variable does not respect the initial value.

Open Animeshz opened this issue 3 years ago • 2 comments

Checklist before submitting an issue

  • [x] I have searched through the existing closed and open issues for eww and made sure this is not a duplicate
  • [x] I have specifically verified that this bug is not a common user error
  • [x] I am providing as much relevant information as I am able to in this bug report (Minimal config to reproduce the issue for example, if applicable)

Describe the bug

  1. The defpoll vars are added without the first evaluation of run-while property (I completely forget about that). As a result they always run unless the vars they reference changes the eww-expr to false later at runtime (they disrespect initial value such as literal false).
  2. The var refs are not verified when loading the config, they're just applied. Current behavior will silently ignore it, since they're refreshed async (the var refs update re-evaluates the s-expr and pushes changes in running state of defpoll var, if var ref is invalid, the update will never happen due to it), instead it should produce an error when loading the config. Should be superseded by fix of point 1.
  3. Cross reference of defpoll variable run-while referencing each other or by a chain of 3 (defpoll a :run-while b ...) (defpoll b :run-while c ...) (defpoll c :run-while a ...) or 4, etc. need special treatment while adding them, probably disallow any reference to defpoll var if :inital is not present in the reference, or just evaluate their first value regardless and then only evaluate :run-while property later. Later approach seem to be better, right next to loading them in EwwConfig (before adding them to the script_var_handler) we can evaluate their initial_value using this, populate the EwwState and then evaluate :run-while then probably add it to the script_var_handler. This should help in fixing 1.

Fixing/handling 3 should fix 1 & automatically 2.

Reproducing the issue

-

Expected behaviour

-

Additional context

-

Animeshz avatar Oct 10 '21 09:10 Animeshz

https://github.com/Animeshz/eww/blob/275c67d74d84b7a8cbe397ec83abe599a2330188/crates/eww/src/app.rs#L219-L220

These (flagging when any property change) breaks the counting, there has to be some better way to take account of the referenced variables within variable parameters somewhat like there is when they're used by widgets, which is parsed into referenced by the window-names...

Maybe somewhat unify the working of variables property like widget ones, to collect them as used by the windows. In addition also unify the working of widget property references to update only when required (when var they reference changes) compared to every time any one of the property changes all listeners are notified.

This current PR seems fuzzy as well (less quality), so I'm closing it.

Animeshz avatar Oct 20 '21 14:10 Animeshz

I feel like the state management structure is currently definitely the big ugly part of eww. It's relatively inflexible, and still causes a good bit of issues and confusion. I've been brainstorming about how to rewrite the structure for a couple months now, but haven't gotten to actually implementing a rework yet. Depending on how critical these edge-case bugs are, it might be reasonable to just wait until I (or someone else, wink-wink) found the energy and brainpower to come up with a cleaner general state architecture - as these fixes would need a rewrite later on anyways, or might not be necessary at all, depending on the implementation

elkowar avatar Oct 23 '21 12:10 elkowar