-
Notifications
You must be signed in to change notification settings - Fork 7
Feat fallback locale #17
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces a fallback locale mechanism in Locale, adds setFallback(), updates constructor and validation to honor exceptions flag, and modifies getText() to consult a fallback locale for default text. Tests are updated and expanded to cover fallback behavior and exception handling. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant L as Locale
C->>L: setFallback("en-US")
L-->>C: Locale (chained)
C->>L: getText(key, params)
alt key exists in default locale
L-->>C: translated value
else key missing in default locale
alt key exists in fallback locale
L-->>C: fallback translation (as default text)
else exceptions disabled
L-->>C: placeholder or computed default
else exceptions enabled
L-->>C: throw Exception
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Locale/Locale.php (2)
28-34: Consider making the fallback property protected for better encapsulation.The
fallbackproperty is declared as public, which allows direct modification bypassing the validation logic insetFallback(). Consider making it protected to ensure proper validation.- /** - * Fallback locale. Used when specific or default locale is missing translation. - * Should always be set to locale that includes all translations. - * - * @var string - */ - public $fallback; + /** + * Fallback locale. Used when specific or default locale is missing translation. + * Should always be set to locale that includes all translations. + * + * @var string + */ + protected $fallback;
73-89: Consider adding null safety check and improving parameter documentation.The method implementation is sound, but there are minor improvements that could enhance robustness and documentation.
/** * Change fallback Locale * - * @param $name + * @param string $name The name of the locale to set as fallback * * @throws Exception */ public function setFallback(string $name): self { if (! \array_key_exists($name, self::$language) && self::$exceptions) { throw new Exception('Locale not found'); } $this->fallback = $name; return $this; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Locale/Locale.php(5 hunks)tests/Locale/LocaleTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Locale/LocaleTest.php (1)
src/Locale/Locale.php (1)
setFallback(80-89)
🔇 Additional comments (4)
src/Locale/Locale.php (3)
55-57: LGTM: Exception handling now respects the global flag.The change to conditionally throw exceptions based on
self::$exceptionsimproves consistency with the class's exception handling strategy.
66-68: LGTM: Constructor now respects the exception flag.The constructor now properly respects the global
$exceptionsflag, maintaining consistency with other methods in the class.
100-102: LGTM: setDefault now respects the exception flag.The change maintains consistency with the new exception handling pattern across the class.
tests/Locale/LocaleTest.php (1)
81-92: Excellent test coverage for the fallback functionality.The test effectively validates the fallback behavior:
- Confirms primary locale translations take precedence
- Verifies fallback locale provides missing translations
- Tests the transition from placeholder to fallback translation
The test structure is clear and comprehensive.
Summary by CodeRabbit
New Features
Refactor
Tests