When I've set the site with site-set and e.g. try variable-get, autocomplete that usually provides a list of variables doesn't work. When I specify the alias directly, it does work however.

Comments

owen barton’s picture

Tricky - the ppid is different, because completion is running in a subshell. Hence, it is looking for a /tmp/drush-env file that doesn't exist and doesn't detect any site.

I am not sure how to best resolve this yet - perhaps drush.complete.sh needs to pass in the ppid of itself (which will be the interactive shell) via the environment? Any better ideas?

For future reference, the call path looks like:
-> complete.inc calls drush_complete_process_argv();
--> this calls drush_sitealias_check_arg(); and drush_sitealias_check_site_env();
---> the latter function further calls drush_sitealias_site_get(), which appears to set (if empty) the 'DRUSH_TARGET_SITE_ALIAS' context to the "site-set" site using the drush_sitealias_site_get() and drush_sitealias_get_envar_filename() functions.
-> back in complete.inc we then fetch the detected site alias with $set_sitealias = drush_sitealias_get_record('@self');

greg.1.anderson’s picture

I haven't tried it yet, but I think it would work right to favor DRUSH_TARGET_SITE_ALIAS if it is set, and fall back to drush_sitealias_get_record('@self') if it is not.

owen barton’s picture

@greg.1.anderson - I don't see how that would work - I don't think the issue is with the DRUSH_TARGET_SITE_ALIAS context or @self. I think the issue is with the ppid and the fact that completion starts a sub-shell, so the ppid Drush sees is that of the completion shell, rather than the interactive shell pid (which is the pid for the file that contains the active site alias). As far as I can tell, if the ppid were correct then the rest of the code already correctly determines the correct alias (for @self).

greg.1.anderson’s picture

Right, I was thinking about the case that already works.. Your comments in #1 are appropriate here. Rather than pass in the ppid to Drush, I think we could pass in the site alias from drush-env.

__drush_ps1() currently gets the current site via f="${TMPDIR:-/tmp/}/drush-env/drush-drupal-site-$$". We could do the same thing in _drush_completion(), and then add $f to the call to Drush. However, $(drush $f --early=includes/complete.inc ... isn't quite right, because $f might actually be a remote site. So, I guess we need to pass in the site name some other way (e.g. via an option) that drush_complete_process_argv() could pick up.

owen barton’s picture

Yes - I think that is roughly the same approach I was suggesting. I was thinking a shell environment variable, since adding stuff to the command line is perhaps more likely to interfere with the argument parsing we do. I guess the question is, do we want to make passing an alias via the environment a "public" facility, or should we keep it specific to complete.inc only?

greg.1.anderson’s picture

I don't know, I wanted to store the current site in an environment variable instead of a file all along. I'd just switch the whole implementation of the site-set command over to use env variables. Short of that, any implementation is okay with me.

daften’s picture

Partly understand some of it, but the inner workings are still a big mystery to me, so don't think I can help solve this. Sorry :s

owen barton’s picture

Version: 7.x-5.7 » 8.x-6.x-dev
Status: Active » Closed (fixed)

Committed in d764827 - it turns out using DRUSH_TARGET_SITE_ALIAS does work (for both arg and env declared aliases).