Skip to content
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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

robbysoerya
Copy link

@robbysoerya robbysoerya commented Nov 12, 2024

Hi,
currently the package doesn't support for RichText or Text.Rich so I propose to add the function called trSpan, which is it will return TextSpan as result

this example to use this function

 {
    "msg_named":"Easy localization is written in the {lang} language"
 }
Text.rich(trSpan('msg_named',namedArgs: {'lang': TextSpan(text: 'Dart')}))

Summary by CodeRabbit

  • New Features

    • Introduced trSpan method for translating localization keys into TextSpan objects, enhancing text formatting capabilities.
    • Added support for named arguments in localization strings through new methods in the Localization class.
    • Updated BuildContextEasyLocalizationExtension to include trSpan for improved localization handling.
  • Documentation

    • Enhanced comments and documentation for existing methods to clarify usage and functionality.

Copy link

coderabbitai bot commented Nov 12, 2024

Walkthrough

The changes involve enhancements to the localization functionality within the Localization class and its related components. A new method trSpan has been introduced to facilitate the translation of localization keys into TextSpan objects, allowing for more complex text formatting. Additionally, helper methods for handling named arguments in TextSpan have been added. The public.dart and public_ext.dart files have also been updated to incorporate this new functionality, while existing methods remain intact with minor formatting improvements.

Changes

File Change Summary
lib/src/localization.dart - Added method: TextSpan trSpan(String key, {Map<String, TextSpan>? namedArgs})
- Added method: List<String> _splitTextWithNamedArg(String text)
- Added method: TextSpan _replaceSpanNamedArgs(String res, Map<String, TextSpan>? args)
- Modified plural method for clarity.
- Adjusted _resolve method for consistent logging and fallback handling.
lib/src/public.dart - Added method: TextSpan trSpan(String key, {BuildContext? context, Map<String, TextSpan>? namedArgs})
- Minor formatting adjustments to trExists function.
lib/src/public_ext.dart - Added method: TextSpan trSpan(String key, {Map<String, TextSpan>? namedArgs}) in BuildContextEasyLocalizationExtension
- Updated comments for the tr method with detailed usage examples.

Poem

In the land of text and span,
A rabbit hops with a clever plan.
Named args dance in a joyful way,
Localization shines bright as day.
With trSpan now, we weave our tale,
Hopping through strings, we shall not fail! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 enhancement

The documentation could be improved in several ways:

  1. Add a more descriptive introduction mentioning rich text support
  2. Document the return type and behavior
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0e88c1c and c5416da.

📒 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:

  1. Input validation for the key parameter
  2. 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

Comment on lines +72 to +86
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;
}
Copy link

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:

  1. Adding documentation with usage examples
  2. Supporting positional arguments and gender like the tr method
  3. 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;
   }

Comment on lines +156 to +163
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);
}
Copy link

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:

  1. Validate that all required named arguments are provided
  2. 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.

@bw-flagship
Copy link
Collaborator

I like the idea! Extensive Unittests will be required for sure

@robbysoerya
Copy link
Author

I like the idea! Extensive Unittests will be required for sure

Yeah, sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants