Conversation
WalkthroughIntroduces a runtime sentinel DEFAULT_DYNAMIC_KEY and extends Locale::getText to accept an optional string|null $default parameter. Changes initialize translation differently when the sentinel is used, add a null-return when resolved translation is null, update docblocks, and add tests covering dynamic default, custom default, null default, and placeholder composition. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Locale
Caller->>Locale: getText(key, default=?, placeholders?)
alt default === DEFAULT_DYNAMIC_KEY
Locale->>Locale: translation = "{{" + key + "}}"
else default provided (string|null)
Locale->>Locale: translation = default
end
Locale->>Locale: resolve translation (fallbacks / lookups)
alt resolved translation === null
Locale-->>Caller: null
else
Locale->>Locale: apply placeholders if any
Locale-->>Caller: final string
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Locale/Locale.php (2)
126-126: Fix parameter order in docblock.The docblock parameter order doesn't match the actual function signature. The
$defaultparameter comes before$placeholdersin the function signature but appears after it in the docblock.- * @param string $key - * @param array<string, string|int> $placeholders - * @param string|null $default + * @param string $key + * @param string|null $default + * @param array<string, string|int> $placeholders
131-131: Consider using a more robust sentinel value.The current sentinel value
'[[defaultDynamciKey]]'(aside from the typo) could theoretically conflict with actual user input. Consider using a more unique approach like a private static object or a more distinctive string pattern.- const string DEFAULT_DYNAMIC_KEY = '[[defaultDynamicKey]]'; // Replaced at runtime by $key wrapped in {{ and }} + private static ?object $DEFAULT_DYNAMIC_KEY = null; + + private static function getDefaultDynamicKey(): object + { + if (self::$DEFAULT_DYNAMIC_KEY === null) { + self::$DEFAULT_DYNAMIC_KEY = new \stdClass(); + } + return self::$DEFAULT_DYNAMIC_KEY; + }And update the function signature and comparison:
- public function getText(string $key, string|null $default = self::DEFAULT_DYNAMIC_KEY, array $placeholders = []) + public function getText(string $key, string|null|object $default = null, array $placeholders = []) { + if ($default === null) { + $default = self::getDefaultDynamicKey(); + } // ... - $translation = $default === self::DEFAULT_DYNAMIC_KEY ? '{{'.$key.'}}' : $default; + $translation = $default === self::getDefaultDynamicKey() ? '{{'.$key.'}}' : $default;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Locale/Locale.php(3 hunks)tests/Locale/LocaleTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Locale/LocaleTest.php (1)
src/Locale/Locale.php (2)
getText(131-159)Locale(7-170)
🔇 Additional comments (4)
src/Locale/Locale.php (2)
136-136: LGTM - Clean default value initialization.The logic correctly initializes the translation based on whether the sentinel value is used, providing a dynamic placeholder when no explicit default is given.
150-152: LGTM - Proper null handling.The early return for null translations prevents unnecessary placeholder processing and maintains expected behavior when an explicit null default is provided.
tests/Locale/LocaleTest.php (2)
68-71: LGTM - Improved test readability with named arguments.Using named arguments for the
placeholdersparameter significantly improves test readability and makes the intent clearer. This is a good practice for optional parameters.Also applies to: 72-74, 77-79, 81-83
124-133: LGTM - Comprehensive test coverage for default parameter.The test thoroughly covers all the new default parameter behaviors:
- Existing translations return correctly
- Missing translations with dynamic default return placeholder
- Custom default values are respected
- Null default returns null
- Placeholder substitution works with custom defaults
This provides excellent coverage for the new functionality.
2 participants
Summary by CodeRabbit
New Features
Tests