Zren / plasma-applet-commandoutput

https://store.kde.org/p/1166510/
GNU General Public License v2.0
87 stars 18 forks source link

waitForCompletion is disabled? #27

Closed trmdi closed 3 years ago

trmdi commented 3 years ago

Is this a typo?

https://github.com/Zren/plasma-applet-commandoutput/blob/9003024463e9e7cceaa07200da7ca0f10af27602/package/contents/ui/config/ConfigGeneral.qml#L45

I mean the option to make the timer runs only one time, in case the command does not need to update.

Zren commented 3 years ago

Kinda sorta. The executable data engine can't run the same command twice at the same time, so I disabled it. If the interval is shorter than the command takes to run, then I think it'll skip the 2nd call. But tbh that's nothing to worry about. I guess we can enable it and make the config default waitForCompletion=false.

trmdi commented 3 years ago

Hmm, when Interval = 0, it still runs only one time, even when Timer.repeat == true. So, waitForCompletion is redundant for this purpose.

Though I don't know if it is still needed for other usages. Can we remove waitForCompletion? Instead, we stop the timer right before executing the command, and restart it after the command exits?

Zren commented 3 years ago

Might as well keep waitForCompletion. I may make limit the interval to 1ms if waitForCompletion=false though since it seems QML Timer's will stop. Even timer.restart() like below doesn't seem to be working for some reason.

if (config.waitForCompletion || config.interval === 0) {
    timer.restart()
}
trmdi commented 3 years ago

Maybe this works?

test.patch.txt

Zren commented 3 years ago

Your code is if interval=0 then the timer doesn't repeat, nor does it "restart" when the command is finished. So the command will never be run more than once...

trmdi commented 3 years ago

Yes, we use interval=0 for making it run only one time intendedly.

Zren commented 3 years ago

Oh, you want a runOnce option?

trmdi commented 3 years ago

Yes, an example use case is just to write some texts on the desktop.

Zren commented 3 years ago

Do you really need it to be a "command", or do you basically want a "Note" widget?

trmdi commented 3 years ago

Do you really need it to be a "command", or do you basically want a "Note" widget?

Hmm... Sometimes I want to write some texts on the dektop just to decorate it. :P Like this: image

The "Note" widget can't change the font family and size like this one. Anyways, I can use a very large interval... Though the timer is still a bit wasted.

Zren commented 3 years ago

I can use a very large interval.

Yeah, that's what I was about to suggest.

Edit: Also that is adorable. xD

trmdi commented 3 years ago

Because when removing this, we could make the config window a bit simpler, but I don't know if there is any cons. Could you give an example in that waitForCompletion == false is useful?

Zren commented 3 years ago

A "clock" needs to update every 1000ms exactly. A clock running every 1010ms will skip a second every once and awhile.

I'm more likely to remove waitForCompletion == true than == false.

trmdi commented 3 years ago

I think you can't make a clock update at 0ms 1000ms 2000ms... (the real millisecond of the clock e.g. 10:45:02.000 AM -> 0ms...) exactly, it will update at e.g. 5ms 1235ms. 2001ms... randomly depending on when it starts, how long the command takes, the event loop... there will be always a delay between this clock and another. So I don't really understand your example. :/

Zren commented 3 years ago

There will always be a delay from X000ms, however I assume it should still start the logic around at X000ms. If there's a 100ms delay caused by the onTriggered logic before restarting the timer, then you'll skip from 9s to 11s.

Notice how it'll skip 10sec.

With a repeating timer, I assume the math when the timer is triggered would be the following to attempt to trigger with the same interval between calls:

sleepFor(interval - ((now - timerStarted) % interval))
trmdi commented 3 years ago

Do you know why the output sometimes (1 time per minute) disappears then reappears when I use this command? The latest git version. [[ $((date +%S% 2)) -eq 0 ]] && echo "openSUSE KDE 😗" || echo "openSUSE KDE 😘"

image Ah, actually that is an error command: https://stackoverflow.com/questions/36238961/script-error-value-too-great-for-base-error-token-is-09 So, there should be some warning when the command is not successfully exited.