-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
feat: add support for rich text #724
base: develop
Are you sure you want to change the base?
feat: add support for rich text #724
Conversation
WalkthroughThe changes involve enhancements to the localization functionality within 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 (
|
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: 2
🧹 Outside diff range and nitpick comments (3)
lib/src/public.dart (1)
48-64
: Documentation needs enhancementThe documentation could be improved in several ways:
- Add a more descriptive introduction mentioning rich text support
- Document the return type and behavior
- Add more examples showing different use cases
Consider updating the documentation like this:
/// {@template trSpan} -/// function for translate your language keys +/// Main function for translating language keys into rich text spans. +/// Returns a [TextSpan] that can be used with [Text.rich] for advanced text formatting. /// [key] Localization key /// [BuildContext] The location in the tree where this widget builds /// [namedArgs] Map of localized strings. Replaces the name keys {key_name} with [TextSpan] +/// +/// If [context] is null, uses the global [Localization.instance]. +/// If [namedArgs] is null, returns a simple [TextSpan] with the translated text. /// /// Example: /// /// ```json /// { -/// "msg_named":"Easy localization is written in the {lang} language" +/// "msg_named": "Easy localization is written in the {lang} language", +/// "welcome": "Welcome {name} to {app}" /// } /// ``` /// ```dart - /// Text.rich(trSpan('msg_named',namedArgs: {'lang': TextSpan(text: 'Dart')})), + /// Text.rich(trSpan('msg_named', namedArgs: {'lang': TextSpan(text: 'Dart')})), + /// Text.rich(trSpan('welcome', namedArgs: { + /// 'name': TextSpan(text: 'John', style: TextStyle(fontWeight: FontWeight.bold)), + /// 'app': TextSpan(text: 'MyApp', style: TextStyle(color: Colors.blue)), + /// })), /// ``` /// {@endtemplate}lib/src/public_ext.dart (1)
222-236
: Add documentation for the new trSpan method.The new method lacks documentation comments explaining its purpose, parameters, and usage examples. This is inconsistent with other methods in the file that have detailed documentation.
Add documentation similar to the
tr
method above:+ /// Translates a key into a TextSpan, supporting rich text with named arguments. + /// + /// [key] Localization key + /// [namedArgs] Map of TextSpan values to replace named placeholders in the text + /// + /// Example: + /// ```json + /// { + /// "msg_named": "Easy localization is written in the {lang} language" + /// } + /// ``` + /// ```dart + /// Text.rich(context.trSpan('msg_named', namedArgs: {'lang': TextSpan(text: 'Dart')})) + /// ``` TextSpan trSpan( String key, { Map<String, TextSpan>? namedArgs, }) {lib/src/localization.dart (1)
123-139
: Add input validation to prevent null/empty text.The method handles text splitting well but could benefit from input validation.
Consider adding validation:
List<String> _splitTextWithNamedArg(String text) { + if (text.isEmpty) { + return [text]; + } final matches = _namedArgMatcher.allMatches(text); var lastIndex = 0; final result = <String>[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
lib/src/localization.dart
(6 hunks)lib/src/public.dart
(1 hunks)lib/src/public_ext.dart
(1 hunks)
🔇 Additional comments (6)
lib/src/public.dart (1)
66-74
: Consider adding input validation and annotations
The implementation is clean and follows the established pattern, but could be enhanced with:
- Input validation for the key parameter
- Return value annotation
Consider these improvements:
+import 'package:meta/meta.dart';
+@useResult
TextSpan trSpan(
String key, {
BuildContext? context,
Map<String, TextSpan>? namedArgs,
}) {
+ if (key.isEmpty) {
+ throw ArgumentError.notNull('key');
+ }
return context != null
? Localization.of(context)!.trSpan(key, namedArgs: namedArgs)
: Localization.instance.trSpan(key, namedArgs: namedArgs);
}
Let's verify if the meta package is already a dependency:
lib/src/public_ext.dart (2)
226-235
: LGTM! Error handling and integration look solid.
The implementation:
- Follows the established error handling pattern
- Properly delegates to the localization instance
- Maintains null safety through optional parameters
222-236
: Verify consistent usage of trSpan across the codebase.
The implementation aligns with the PR objectives for rich text support. Let's verify its usage pattern across the codebase.
✅ Verification successful
Implementation is consistent and properly integrated
The verification shows that the trSpan
implementation is well-integrated and follows a consistent pattern across the codebase:
- The implementation exists at all necessary levels (Localization, public API, and extensions)
- The method signature is consistent across all implementations
- The feature is properly documented with examples
- The implementation supports the existing translation patterns with named arguments
- No conflicting or inconsistent usage patterns were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for existing Text.rich usage to ensure consistent patterns
# and identify potential migration opportunities.
echo "Searching for Text.rich usage patterns..."
rg "Text\.rich" --type dart
echo "Searching for potential translation keys that might benefit from rich text..."
rg "[\"\'].*\{.*\}.*[\"\']" --type json
Length of output: 522
Script:
#!/bin/bash
# Let's try a more focused search approach
# Search for TextSpan usage
echo "Searching for TextSpan usage..."
rg "TextSpan" -A 2 --type dart
# Search for translation patterns with curly braces (potential rich text)
echo "Searching for translation patterns..."
rg "tr\(.*\{.*\}" --type dart
# Search for any existing trSpan calls
echo "Searching for trSpan usage..."
rg "trSpan\(" --type dart
Length of output: 3140
lib/src/localization.dart (3)
12-12
: LGTM: Well-defined regex pattern for named arguments.
The regex pattern correctly captures named arguments enclosed in curly braces, allowing alphanumeric characters and underscores.
197-198
: LGTM: Clean formatting improvements.
The formatting changes improve readability without affecting functionality.
Also applies to: 241-242
72-86
: Verify implementation with PR example.
Let's verify that the implementation works with the example from PR description.
✅ Verification successful
Let me gather more information to verify the implementation thoroughly.
Let me search for the implementation of the helper methods used by trSpan
to verify if it works correctly with the example.
Let me try a different search pattern to find the helper method implementations.
Based on the implementation found, I can now verify if the code works with the example. Let me check one more thing about the text splitting implementation.
Based on the code analysis, I can now provide a final verification response:
Implementation correctly handles the example use case
The implementation of trSpan
and its helper methods properly supports the example usage:
_replaceLinks
processes any linked translations_replaceSpanNamedArgs
correctly splits text and replaces named arguments with TextSpans_splitTextWithNamedArg
properly handles the pattern{lang}
from the example- The implementation matches the documented usage:
Text.rich(trSpan('msg_named',namedArgs: {'lang': TextSpan(text: 'Dart')}))
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation works with the example from PR description
# Test: Search for test files that might contain the example usage
rg -l "msg_named.*TextSpan.*lang.*Dart" --type dart
# Test: Search for translation files that might contain the msg_named key
fd -e json -x jq -r '.msg_named // empty'
Length of output: 333
Script:
#!/bin/bash
# Search for test files containing trSpan usage
rg "trSpan\(" --type dart -A 5
# Search for the _replaceLinks and _replaceSpanNamedArgs implementations
ast-grep --pattern 'TextSpan _replaceSpanNamedArgs($_, $_) {
$$$
}'
# Look for any test cases
fd "test.*dart$" -x cat {}
Length of output: 83486
Script:
#!/bin/bash
# Search for _replaceLinks and _replaceSpanNamedArgs implementations
ast-grep --pattern 'String _replaceLinks(String $text) {
$$$
}'
ast-grep --pattern 'TextSpan _replaceSpanNamedArgs(String $text, $_) {
$$$
}'
# Search for any test files containing trSpan tests
fd "test.*dart$" -x rg "trSpan.*test" {}
Length of output: 194
Script:
#!/bin/bash
# Search for _replaceLinks and _replaceSpanNamedArgs implementations with more flexible pattern
rg "_replaceLinks|_replaceSpanNamedArgs" -A 10
# Search for any test files containing TextSpan tests
rg "test.*TextSpan" --type dart -A 5
# Look for example usage in documentation or comments
rg "msg_named.*TextSpan" --type dart -A 5
Length of output: 2655
Script:
#!/bin/bash
# Search for _splitTextWithNamedArg implementation
rg "_splitTextWithNamedArg" -A 10
# Look for any related tests that verify named argument replacement
rg "namedArgs.*TextSpan" --type dart -A 5
Length of output: 1888
TextSpan trSpan( | ||
String key, { | ||
Map<String, TextSpan>? namedArgs, | ||
}) { | ||
late String res; | ||
late TextSpan span; | ||
|
||
res = _resolve(key); | ||
|
||
res = _replaceLinks(res); | ||
|
||
span = _replaceSpanNamedArgs(res, namedArgs); | ||
|
||
return span; | ||
} |
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.
🛠️ Refactor suggestion
Add documentation and consider feature parity with tr
method.
The new trSpan
method lacks documentation and doesn't support all features available in the tr
method. Consider:
- Adding documentation with usage examples
- Supporting positional arguments and gender like the
tr
method - Adding error handling for invalid keys
Add documentation and consider implementing feature parity:
+ /// Translates a string key into a [TextSpan] with support for named arguments.
+ ///
+ /// Example:
+ /// ```dart
+ /// Text.rich(trSpan('msg_named', namedArgs: {'lang': TextSpan(text: 'Dart')}))
+ /// ```
+ ///
+ /// The translation string should contain named placeholders in curly braces:
+ /// ```json
+ /// {
+ /// "msg_named": "I love coding in {lang}"
+ /// }
+ /// ```
TextSpan trSpan(
String key, {
+ List<TextSpan>? args,
Map<String, TextSpan>? namedArgs,
+ String? gender,
}) {
late String res;
late TextSpan span;
+ if (gender != null) {
+ res = _gender(key, gender: gender);
+ } else {
+ res = _resolve(key);
+ }
- res = _resolve(key);
res = _replaceLinks(res);
span = _replaceSpanNamedArgs(res, namedArgs);
+
+ if (args != null) {
+ // TODO: Implement positional args support
+ }
return span;
}
TextSpan _replaceSpanNamedArgs(String res, Map<String, TextSpan>? args) { | ||
if (args == null || args.isEmpty) return TextSpan(text: res); | ||
final spans = _splitTextWithNamedArg(res).map((part) { | ||
final key = part.replaceAll(RegExp(r'^\{|\}$'), ''); | ||
return args[key] ?? TextSpan(text: part); | ||
}).toList(); | ||
return TextSpan(children: spans); | ||
} |
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.
🛠️ Refactor suggestion
Add validation and optimize performance.
The method could benefit from validation and performance improvements:
- Validate that all required named arguments are provided
- Pre-check if text contains any placeholders before processing
Consider these improvements:
TextSpan _replaceSpanNamedArgs(String res, Map<String, TextSpan>? args) {
if (args == null || args.isEmpty) return TextSpan(text: res);
+
+ // Quick check if text contains any placeholders
+ if (!res.contains('{')) return TextSpan(text: res);
+
+ // Extract all required argument names
+ final required = _namedArgMatcher
+ .allMatches(res)
+ .map((m) => m.group(2))
+ .where((name) => name != null)
+ .toSet();
+
+ // Validate all required arguments are provided
+ final missing = required.where((name) => !args.containsKey(name));
+ if (missing.isNotEmpty) {
+ throw ArgumentError('Missing named arguments: ${missing.join(', ')}');
+ }
+
final spans = _splitTextWithNamedArg(res).map((part) {
final key = part.replaceAll(RegExp(r'^\{|\}$'), '');
return args[key] ?? TextSpan(text: part);
}).toList();
return TextSpan(children: spans);
}
Committable suggestion skipped: line range outside the PR's diff.
I like the idea! Extensive Unittests will be required for sure |
Yeah, sure |
Hi,
currently the package doesn't support for
RichText
orText.Rich
so I propose to add the function calledtrSpan
, which is it will returnTextSpan
as resultthis example to use this function
Summary by CodeRabbit
New Features
trSpan
method for translating localization keys intoTextSpan
objects, enhancing text formatting capabilities.Localization
class.BuildContextEasyLocalizationExtension
to includetrSpan
for improved localization handling.Documentation