Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conditional exec always executed #334

Closed
bobemoe opened this issue Oct 16, 2016 · 11 comments
Closed

conditional exec always executed #334

bobemoe opened this issue Oct 16, 2016 · 11 comments
Labels
bug related to incorrect existing implementation of some functionality Stale requires attention because it hasn't been updated in a long time

Comments

@bobemoe
Copy link

bobemoe commented Oct 16, 2016

I have a conditional execi, but since updating from 1.9.0 to 1.10.5 the execi is ALWAYS executed, even when the condition is FALSE.

${if_match ${battery_percent BAT1}<10}
    ${execi 10 aplay ~/.i3/alert.wav}
${endif}

I have tried exec too and the exhibits the same behaviour.

I have put some text inside the conditional block and it is not displayed, confirming the condition evaluates false, however the exec is still executed.

@bobemoe bobemoe changed the title conditional execi always executed conditional exec always executed Oct 16, 2016
@plikhari
Copy link

Quote the values like

${if_match "${battery_percent BAT1}" < "10"}
    ${execi 10 aplay ~/.i3/alert.wav}
${endif}

See if this works.

@bobemoe
Copy link
Author

bobemoe commented Oct 18, 2016

Hi thanks for your reply. Unfortunately that doesn't seem to make any difference. I don't think there is a problem with the condition, as I have also put some test text inside conditional block and it is not displayed, like so:

${if_match "${battery_percent BAT1}" < "10"}
    TEST
    ${execi 10 aplay ~/.i3/alert.wav}
    TEST
${endif}

I am not seeing TEST TEST displayed but the execi is still being executed. This confirms the condition is evaluating false. If I change the condition so it evaluates true (change 10 to 1000) then TEST TEST is displayed and the execi is still executed.

I have also tested using an if_existing as follows:

${if_existing /etc/}
        ${execi 10 aplay ~/.i3/alert.wav}
        TEST
${endif}

TEST is displayed, and execi is executed.

${if_existing /404/}
        ${execi 10 aplay ~/.i3/alert.wav}
        TEST
${endif}

TEST is not displayed, but execi is executed.

@bobemoe bobemoe closed this as completed Oct 18, 2016
@bobemoe bobemoe reopened this Oct 18, 2016
@plikhari
Copy link

Fantastic have a look at the issue I raised #318

All of the if_ conky variables will only print or not print within the condition - depending on true or false. These if_ conditions do not stop any execution of a command with exec, execi etc.

So to work around this - you saw my work around there - using shell to figure things out.
${texecpi 10 if [ -s ${template1} ]; then do something here; fi } I use texecpi as I need to execute an external process. I do not know any way where a conky variable can be captured and used in this work around.

From a common sense point of view - if the condition is false then the following code should not be evaluated at all.

@marcpayne
Copy link
Contributor

Thanks for pointing to a workaround @plikhari

All this stems from the way callbacks are handled now. Visible output from conky objects respects ${if...} expressions and works as expected. However, the behind-the-scenes work needed to update those objects (e.g. running a command and storing its output, or getting the latest info for ${top...}) is performed in one go, each update interval, before any ${if...} expressions are handled.

I know this explanation doesn't fix the situation, but hopefully it sheds some light as to why it happens.

@plikhari
Copy link

Hey @marcpayne - Many thanks for that information.

I figured this by going through the source code.

But at some point in time - we need to find a way to handle these issues - as my workaround will only work with tangible things that shell can look at - like in my case - check if a file exists.

@dbriba
Copy link
Contributor

dbriba commented Oct 19, 2016

And if you chain the command with anti-slash, like this :
${if_match ${battery_percent BAT1}<10}
${execi 10 aplay ~/.i3/alert.wav}
${endif}
That's work ?

@bobemoe
Copy link
Author

bobemoe commented Oct 19, 2016

@dbriba sorry to report that adding the \ has not had any affect.

@plikhari thanks for the idea, I now use a bash scrip to do the test and play the sound. I may as well use ACPI events at this point, or a battery monitor. I guess conky isn't really the right place to be doing this sort of stuff.

I'm happy now anyway. Shall I leave this issue open for your reference? Sounds like you may want to change this behaviour as it kind of defies logic, and also breaks backward compatibility of some configs, in a kind of hard to debug way. Anyway, up to you all now, feel free to close if you are want.

Thanks :)

@dgiglio
Copy link

dgiglio commented Mar 16, 2017

@plikhari

So to work around this - you saw my work around there - using shell to figure things out.
${texecpi 10 if [ -s ${template1} ]; then do something here; fi }

Excuse me, why do you use a template inside a condition?

@plikhari
Copy link

You can use the actual file name here - it is just that - when you are handling 20+ template files - it helps to know which external sources I am checking or using. I use a script to feed the template variables used for each template file into the help docs. I am referring to the documentation for conkywx - my weather program.

@lasers lasers added the bug related to incorrect existing implementation of some functionality label Aug 6, 2018
@lasers lasers added the major label Aug 7, 2018
This was referenced Aug 11, 2018
Copy link

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 github-actions bot added the Stale requires attention because it hasn't been updated in a long time label Nov 20, 2023
Copy link

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug related to incorrect existing implementation of some functionality Stale requires attention because it hasn't been updated in a long time
Projects
None yet
Development

No branches or pull requests

6 participants