Skip to content

Conversation

@terencehonles
Copy link
Contributor

Remove --path=/dev/null so if the current directory is a wordpress directory it will load the plugins and correctly prefix "wp" when needed. The second if is changed to elif because the conditions are exclusive and if the first test matched the second doesn't need to run.

@tianon
Copy link
Member

tianon commented Mar 8, 2019

Does this mean to imply that WordPress plugins can also provide additional WP-CLI behavior? Are there any documentation links on that you can share? (My searching isn't coming up with anything useful except the wp plugin subcommand, which is good but not related. 😅)

@terencehonles
Copy link
Contributor Author

I found a reference here: https://wp-cli.org/#extending and also here https://make.wordpress.org/cli/handbook/internal-api/wp-cli-add-command/ but I came across this when trying to install the ilabs mediacloud wordpress plugin.

Not that you're probably looking for this, but here's the code that it triggers https://github.com/Interfacelab/ilab-media-tools/blob/eb3ad995724d5c6355a9016870257eb80c20d91f/classes/CLI/Storage/StorageCommands.php#L161 and it is registered here https://github.com/Interfacelab/ilab-media-tools/blob/eb3ad995724d5c6355a9016870257eb80c20d91f/ilab-media-tools.php#L113

TBH: I'm not super familiar with wordpress and am still putting things together.

@terencehonles
Copy link
Contributor Author

ping :)

@tianon
Copy link
Member

tianon commented Jun 24, 2020

Man, sorry for the delay. 🙇

I really appreciate the patience and the excellent links. ❤️

My concern with removing this is that we added for a reason, but I can't recall what that reason was (I'm sure there was some specific use case that broke without it). Do you have any ideas?

Does wp help foo possibly fail if WordPress is not installed?

@terencehonles
Copy link
Contributor Author

No worries, I think I package my own docker image which includes this, but I actually haven't used the cli that much. I'm really not too familiar with wordpress. However this is what I'm thinking:

  1. If wordpress isn't installed wp itself will fail (so the value of --path doesn't matter) and the container will try to execute the command as given.
  2. If wordpress is installed wp will run and help will try to lookup a command that has been registered (via the core commands or plugins via WP_CLI::add_command(...)).

If help itself fails it would be because something has broken the CLI from loading. I'm not sure how defensive the CLI is but the --path argument looks like it affects this code: https://github.com/wp-cli/wp-cli/blob/d2a002214df900bba7303310f42c68b996d873ab/php/WP_CLI/Runner.php#L199-L233 which looks like it goes looking for wp-load.php or index.php up the FS. I'm guessing this was written this way to just try to be as fast as possible, but that's the limit to what I can suggest.

combine ifs in the cli entrypoint
@yosifkit
Copy link
Member

yosifkit commented Sep 4, 2020

Rebased and coalesced the two if statements to one since they were doing the same thing.

LGTM cc @tianon

@tianon tianon merged commit 598769a into docker-library:master Sep 4, 2020
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Sep 4, 2020
Changes:

- docker-library/wordpress@598769a: Merge pull request docker-library/wordpress#387 from terencehonles/remove-cli-entrypoint-path-argument-to-load-plugins
- docker-library/wordpress@c0d11ed: remove --path argument so plugins are loaded
@terencehonles
Copy link
Contributor Author

Thanks all! I look forward to updating my container and not having to mount over the entrypoint / reducing the extra config 😅

@terencehonles terencehonles deleted the remove-cli-entrypoint-path-argument-to-load-plugins branch September 15, 2020 18:00
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.

3 participants