Locate Plugin Dir URL Fix#13
Conversation
* Updating the locate_plugin method to use the plugin directory name as the base for the dir_url * This is required since when running tests, the WP_CONTENT_DIR is set to a different WordPress install (wordpress-develop on VVV) by WP-Dev-Lib's bootstrap than the current plugin's install and thus the tests throw an exception before they can be run
|
@MattGeri the mess that is the |
|
@westonruter Ahh! Ok. The problem is that for normal plugins (based on the WP-Dev-Lib) config, the tests fail - actually they don't run because it throws an exception. Will see if I can come up with a solution that accommodates themes as well. |
| } | ||
| $content_sub_path = substr( $plugin_dir, strlen( $content_dir ) ); | ||
| $dir_url = content_url( trailingslashit( $content_sub_path ) ); | ||
| $dir_url = content_url( trailingslashit( 'plugins/' . basename( $plugin_dir ) ) ); |
There was a problem hiding this comment.
@MattGeri how about adding an if/else in here to check whether or not defined( 'WPCOM_IS_VIP_ENV' ). And if so, run the code you deleted, but if not VIP, then use the new line you added. Nevertheless, in that case, should we just use plugin_dir_url()?
* Takes in to account that a plugin can reside inside a theme * Uses a new method called relative path to determine where the plugin resides * Added tests for new relative path method
|
@westonruter I decided not to use the "if is VIP" switch as per your comment as if we were developing a VIP plugin in a theme locally and didn't have the ENV variable set, the code would not return the correct path and would effectively break the plugin if that dir_url is ever used. I've instead implemented a new method to determine the relative path of the plugin from the wp-content directory. Let me know what you think and if you see any potential issues. |
|
@MattGeri actually it would be OK because these VIP plugins would be only running unit tests in VIP Quickstart where the |
|
@westonruter Ah I see! I've not worked with VIP QuickStart before, so thought the ENV var was only on production. I still think the current solution is the better solution though as regardless of the environment VIP/Non-VIP /Who knows what else, it will work. I also find per environment switches in code to be a little bit smelly ;) Thoughts? |
|
Nesting plugins inside of themes is also smelly 😃
|
| * | ||
| * @see Plugin_Base::relative_path() | ||
| */ | ||
| public function test_relative_path() { |
There was a problem hiding this comment.
The '/srv/www/wordpress-develop/src path wouldn't work in a non-VVV context or if the unit tests aren't being run in wordpress-develop.
There was a problem hiding this comment.
This test is just checking that the relative_path method strips out the path before the starting point, so it's not dependant on VVV for it to work. I just used the VVV path for the test since it was the one that I use. Want me to add additional paths?
|
@westonruter If you feel it's better to do the if statement, then I can implement that |
|
@MattGeri I think what you have is good. The previous code for VIP Quickstart support wasn't very pretty, so if what you have is more elegant, I'm for it. |
* Removed a rogue space
@westonruter This removes what appears to be some significant functionality to figure out the url of the plugin. It also removes a check that the plugin resides in the wp-content directory. There were a few ways to go about fixing this bug and this is the way I chose. Happy to discuss alternatives and add additional checks.