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

Command parameters that start with config values interpreted as set parameters #4325

Closed
richardbporter opened this issue Jan 26, 2021 · 2 comments · Fixed by #4340
Closed
Labels
Bug Something isn't working

Comments

@richardbporter
Copy link
Contributor

Describe the bug
When specifying command parameters, if the parameter starts with a BLT config value, that config value is interpreted as being set.

To Reproduce

  1. Run blt sync --site siteone.com
  2. See that the --uri option used in Drush is set to one.com instead of siteone.com
  3. This would error if that site did not exist or sync the wrong site if it did.

This specific example can be worked around by using an equals sign (--site=siteone.com) or by wrapping it in quotes (--site "siteone.com"). However, in other situations this doesn't work. For example, say you have a custom BLT command that takes an argument. Passing an argument like blt my:command environmentalsciences.edu will set the environment option in ConfigInitializer and I'm not sure how to escape it.

Expected behavior
I would expect BLT config values to only be set when only that config value is passed via a parameter and not when it is included as part of another word.

System information
DrupalVM

  • Operating system type: Ubuntu
  • Operating system version: 18.04
  • BLT version: 11.6.0

Additional context
Maybe this is expected behavior and just a documentation issue on clearly stating how to escape arguments/options. Apologies if I missed such documentation.

@richardbporter richardbporter added the Bug Something isn't working label Jan 26, 2021
danepowell added a commit to danepowell/blt that referenced this issue Feb 5, 2021
@danepowell
Copy link
Contributor

Alright, I think I nailed this, would you mind helping to test? #4340

I'm pretty nervous because this modifies bootstrap code that's pretty complex and already caused some grief lately. But a fair amount of hammering hasn't revealed any regressions.

danepowell added a commit that referenced this issue Feb 8, 2021
* DX-3283: Fix #4325: Parameters interpreted incorrectly

* Code style fix

* Fix notice
@richardbporter
Copy link
Contributor Author

Sorry for not getting back to this sooner - we were not on BLT 12 yet. I just tested this with 12.8.0 and it works great. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants