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

Fix yad version check if statement #239

Closed

Conversation

alisonjenkins
Copy link

I was having stl report that my version of yad was '0'. Having checked
the code I found that the $YAD variable was just set to 'yad' which
meant that it would be checking for a file called yad in the current
directory. Swapping to command -v which will use the PATH variable to
determine if yad is present.

I was having stl report that my version of yad was '0'. Having checked
the code I found that the $YAD variable was just set to 'yad' which
meant that it would be checking for a file called yad in the current
directory. Swapping to command -v which will use the PATH variable to
determine if yad is present.
@frostworx
Copy link
Collaborator

Hello Alan,
nice to hear from you! Thanks for reporting, this is definitely a problem, but the fix is not 100% correct, as the initial idea is to allow to choose from appimage and conty yad versions (see here)
so YAD should be defined in global.conf.
Maybe the best solution would be to remove the global YAD="yad" completely and when adding an early function which checks if YAD is configured in global.conf. If yes do nothing, if no set the default
YAD="$(which yad)"
What do you think?

frostworx pushed a commit that referenced this pull request May 20, 2021
@frostworx
Copy link
Collaborator

Just pushed a possible fix. Please check if this fixes it for you as well.

@alisonjenkins
Copy link
Author

Hello Alan,
nice to hear from you! Thanks for reporting, this is definitely a problem, but the fix is not 100% correct, as the initial idea is to allow to choose from appimage and conty yad versions (see here)
so YAD should be defined in global.conf.
Maybe the best solution would be to remove the global YAD="yad" completely and when adding an early function which checks if YAD is configured in global.conf. If yes do nothing, if no set the default
YAD="$(which yad)"
What do you think?

Hey mate,

Yeah that sounds sane. I will take a look at the code and have a go at implementing this shortly.

Thanks,

Alan

@frostworx
Copy link
Collaborator

Oh, wait, don't miss my previous comment! It might be already fixed in current master!
See here:
031c832

@alisonjenkins
Copy link
Author

Ah yes that looks good. I will install the latest and retest my end. If it is all good I will report back and close this PR.

Thanks for all your good work mate!

@frostworx
Copy link
Collaborator

Thanks for your friendly feedback, mate! :)

@alisonjenkins
Copy link
Author

OK I can confirm that the latest version works fine again so no changes are required. Closing this PR.

@alisonjenkins alisonjenkins deleted the fix-yad-version-check branch May 20, 2021 19:09
@frostworx
Copy link
Collaborator

Cool, thanks for testing! :)

@frostworx frostworx mentioned this pull request May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants