fixes Non-static method #34
Conversation
Non-static method FriendsOfRedaxo\VidStack\Video::getVideoInfo() cannot be called statically
|
Caution Review failedThe pull request is closed. WalkthroughThe changes involve significant modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
lib/video.php (3)
Line range hint
173-199: Consider cachinggetVideoInforesults for efficiencySince
getVideoInfois called multiple times with the same$source, consider caching its result in an instance variable to avoid redundant processing, especially if the method's complexity increases in the future.You can implement caching as follows:
class Video { private readonly string $source; + private ?array $videoInfo = null; // ... public function generateFull(): string { - $videoInfo = self::getVideoInfo($this->source); + $videoInfo = $this->getVideoInfoCached(); $isAudio = self::isAudio($this->source); // ... } public function generate(): string { // ... - $videoInfo = self::getVideoInfo($this->source); + $videoInfo = $this->getVideoInfoCached(); // ... } + private function getVideoInfoCached(): array + { + if ($this->videoInfo === null) { + $this->videoInfo = self::getVideoInfo($this->source); + } + return $this->videoInfo; + } }This approach enhances performance by ensuring
getVideoInfois called only once per instance.
Line range hint
44-47: Handle missing translation keys gracefullyIn the
getTextmethod, if a translation key is missing, the method returns"[[$key]]". While this indicates a missing translation, it might not be user-friendly. Consider providing a default message or logging the missing keys for easier debugging.You could modify the method as follows:
- return self::$translations[$this->lang][$key] ?? "[[{$key}]]"; + return self::$translations[$this->lang][$key] ?? '';Alternatively, log the missing keys:
if (!isset(self::$translations[$this->lang][$key])) { // Log the missing translation key error_log("Missing translation for key '{$key}' in language '{$this->lang}'"); } return self::$translations[$this->lang][$key] ?? '';
Line range hint
18-23: Avoid exposing file paths in exception messagesIn
loadTranslations, the exception message reveals the file path when the translations file is not found. Exposing file paths can be a security risk. Consider modifying the exception message to avoid disclosing sensitive information.Update the exception message as follows:
- throw new \RuntimeException("Translations file not found: $file"); + throw new \RuntimeException("Translations file not found.");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- lib/video.php (3 hunks)
🧰 Additional context used
🔇 Additional comments (6)
lib/video.php (6)
141-149: Appropriate refactoring ofgetVideoInfoto a static methodChanging
getVideoInfoto a static method with thestring $sourceparameter is a suitable fix for the issue where a non-static method was being called statically. This refactoring enhances the method's reusability and ensures that it no longer depends on instance properties.
208-209: Confirm update ofgetVideoInfocall ingeneratemethodIn the
generatemethod, the call togetVideoInfohas been correctly updated toself::getVideoInfo($this->source), aligning with the new static method signature. This change ensures consistency across the class methods.
Line range hint
208-221: Confirm that subtitles are correctly nested within<media-provider>The subtitles are now moved inside the
<media-provider>tag in thegeneratemethod. Ensure that this change conforms to the expected HTML structure and that subtitle functionality remains unaffected in the media player.Testing the media player with subtitles will help confirm that they display correctly after this structural change.
Line range hint
173-199: Verify consent placeholder functionality for external platformsIn
generateFull, the consent placeholder is generated for videos from external platforms like YouTube or Vimeo. Ensure that the consent mechanism works as intended and that the user experience is smooth when loading external content.Consider testing with actual YouTube and Vimeo videos to confirm the consent process and video playback.
148-149: Verify the Vimeo URL pattern matches all formatsThe regular expression for Vimeo URLs in
getVideoInfomay not cover all possible Vimeo URL formats. Ensure that the pattern accurately captures video IDs from various Vimeo URL structures, including those with additional path segments or query parameters.You can test the regex pattern with different Vimeo URLs using the following script:
#!/bin/bash # Description: Test the Vimeo regex pattern with various Vimeo URLs. # Expected: The pattern should match and extract the video ID from each URL. # Sample Vimeo URLs to test urls=( "https://vimeo.com/123456789" "https://player.vimeo.com/video/123456789" "https://vimeo.com/channels/staffpicks/123456789" "https://vimeo.com/album/1234567/video/123456789" "https://vimeo.com/groups/name/videos/123456789" ) pattern='~(?:<iframe [^>]*src=")?(?:https?:\/\/(?:[\w]+\.)*vimeo\.com(?:[\/\w]*\/(progressive_redirect\/playback|external|videos?))?\/([0-9]+)[^\s]*)"?(?:[^>]*><\/iframe>)?(?:<p>.*<\/p>)?~ix' for url in "${urls[@]}"; do if [[ $url =~ $pattern ]]; then echo "Matched URL: $url" echo "Extracted Video ID: ${BASH_REMATCH[2]}" echo else echo "No match for URL: $url" echo fi done
173-176: Ensure all calls togetVideoInfoare updatedIn the
generateFullmethod, the call togetVideoInfohas been correctly updated toself::getVideoInfo($this->source). Verify that all other calls togetVideoInfowithin the class have been updated to match the new static method signature.You can run the following script to confirm that all calls to
getVideoInfopass the$sourceparameter:
Non-static method FriendsOfRedaxo\VidStack\Video::getVideoInfo() cannot be called statically
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Refactor