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

Cli.php: Altering detectCmdLocationInPaths() to check $bin variable as non-empty string instead of non-null value. #125

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
2 participants
@egifford
Contributor

egifford commented Nov 6, 2017

The isExecutable() method returns a string and in almost all of the places it's used, its return value is checked as empty/nonempty string. But the detectCmdLocationInPaths() usage checks for a non-null value.

This discrepancy is to causing my installs to not find system tools like mysqldump and gzip. The for() loop in detectCmdLocationInPaths() is ending too early (after only one iteration) and doesn't check all locations in the system path.

I've changed the detectCmdLocationInPaths() method to check the return value of isExecutable() as a string.

Cli.php: Altering check for $bin from null to empty().
The isExecutable() method returns a string and almost all uses of the method check the return value with the empty() method. This commit changes detectCmdLocationInPaths() to also test the returned value as a string.

@sebastianfeldmann sebastianfeldmann merged commit 32da9f1 into sebastianfeldmann:master Nov 6, 2017

2 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sebastianfeldmann

This comment has been minimized.

Show comment
Hide comment
@sebastianfeldmann

sebastianfeldmann Nov 6, 2017

Owner

Thank you for the fix.

Owner

sebastianfeldmann commented Nov 6, 2017

Thank you for the fix.

@sebastianfeldmann

This comment has been minimized.

Show comment
Hide comment
@sebastianfeldmann

sebastianfeldmann Nov 7, 2017

Owner

I’ll release a new version with this fix today.
Thanks again :)

Owner

sebastianfeldmann commented Nov 7, 2017

I’ll release a new version with this fix today.
Thanks again :)

@sebastianfeldmann sebastianfeldmann self-assigned this Nov 7, 2017

@egifford egifford deleted the egifford:cli_detectCmdLocationInPaths branch Nov 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment