search

drush-ops / drush-launcher

A small wrapper around Drush for your global $PATH.
GNU General Public License v2.0
237 stars 50 forks source link

Fix fallback docs and logic #42

Closed juampynr closed 6 years ago

juampynr commented 6 years ago

I tried today the Drush launcher with a Drupal 7 project and, after finding https://github.com/drush-ops/drush-launcher/pull/39 and updating it, I did the following:

[juampy@carboncete ~]$ cd /path/to/drupal7
[juampy@carboncete /path/to/drupal7 (master)]$ DRUSH_LAUNCHER_FALLBACK=/usr/share/drush/drush
[juampy@carboncete /path/to/drupal7 (master)]$ drush st
The Drush launcher could not find a Drupal site to operate on. Please do *one* of the following:
  - Navigate to any where within your Drupal project and try again.
  - Add --root=/path/to/drupal so Drush knows where your site is located.

After debugging the launcher, I found out that getenv() was returning false. I fixed this by exporting the variable instead of just setting it.

This pull request does two things:

  1. Adjust the docs so the fallback variable is exported.
  2. Move the fallback via environment variable logic out of the arg loop as otherwise it would be unnecessarily evaluated more than once if there are several unexpected arguments. The new logic also allows having an environment variable but overriding it via an option, which is a practice that I have seen in other projects.
webflo commented 6 years ago

Looks good. Thanks!