use WP_oEmbed singleton when working with providers #84
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| And STDOUT should be empty | ||
|
|
||
| @require-wp-4.0 | ||
| Scenario: Only match an oEmbed provider if discover |
There was a problem hiding this comment.
How is this scenario related to the new code? It doesn't test anything with example.com
| Then STDOUT should match /^(?:(?:1|0)\n)+$/ | ||
|
|
||
| @require-wp-4.0 | ||
| Scenario: Match an oEmbed provider |
There was a problem hiding this comment.
Ditto here, how does this scenario test the new code? audio.com and YouTube aren't really new providers, no?
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #64 where the list_providers command was directly instantiating the WP_oEmbed class instead of using the WordPress singleton, causing it to miss all dynamically registered oEmbed providers. The fix changes the instantiation to use _wp_oembed_get_object() which returns the singleton instance containing all registered providers.
Key changes:
- Updated
Provider_Command::list_providers()to use_wp_oembed_get_object()instead ofnew \WP_oEmbed() - Added comprehensive test suite to verify custom providers registered via
wp_oembed_add_provider()are properly listed
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Provider_Command.php | Changed from direct WP_oEmbed instantiation to singleton access via _wp_oembed_get_object() with explanatory comment |
| features/add-oembed-provider.feature | Added comprehensive test scenarios verifying custom provider registration and listing functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| And STDOUT should contain: | ||
| """ | ||
| audio.com/ |
There was a problem hiding this comment.
Inconsistent indentation: this line has an extra space before audio.com/. It should align with the other STDOUT expectations (line 263 has the correct indentation).
| audio.com/ | |
| audio.com/ |
Fixes #64.
The command was directly instantiating the WP_oEmbed class, which meant none of the registered providers were in the providers array.